Skip to content
This repository has been archived by the owner on Jan 15, 2019. It is now read-only.

fix for opacity featured image #320

Closed
wants to merge 6 commits into from

Conversation

LittleBigThing
Copy link

Proposed fix for the opacity of featured image layers for browser not supporting mix-blend-mode
Looks good on Chrome and Safari, need to test on Edge and IE.

proposed fix for the opacity of featured image layers for browser not supporting mix-blend-mode
@LittleBigThing
Copy link
Author

Should fix #121

@LittleBigThing
Copy link
Author

Will add some comments later

@allancole allancole added this to the RC1 milestone Oct 25, 2018
Csaba added 2 commits October 25, 2018 21:27
adding comments in SASS (only) to clarify the styling inside @supports
@LittleBigThing
Copy link
Author

Not sure about the conflicts, but the PR works as expected in IE and Edge (#121). Of course, the opacity can always be adjusted as needed. Accessibility testing is welcome. Also, some opinions about the effect.

Personally, I like the single post version with the image as a background. The featured image in the post feed is trickier: there, the image is not meant to be a background image, so one could expect to see it more clearly. This is of course general comment, but is more pronounced for this fallback version as the effect is less clear and unique.

Note: filter is not supported in IE11 so the result is different there than is Edge.

@kjellr kjellr self-requested a review October 26, 2018 01:51
@kjellr
Copy link
Collaborator

kjellr commented Oct 26, 2018

This is looking great, thanks @LittleBigThing!

Here are some screenshots of my testing:

Safari 12.0:

screen shot 2018-10-25 at 10 34 42 pm

screen shot 2018-10-25 at 10 34 09 pm

Edge

screen shot 2018-10-25 at 10 37 43 pm

screen shot 2018-10-25 at 10 37 07 pm

Internet Explorer

screen shot 2018-10-25 at 10 39 22 pm

screen shot 2018-10-25 at 10 39 07 pm


Any idea why the post-feed versions for IE/Edge are so much lighter than the ones on the single post page? Are those missing a layer or something?

The featured image in the post feed is trickier: there, the image is not meant to be a background image, so one could expect to see it more clearly. This is of course general comment, but is more pronounced for this fallback version as the effect is less clear and unique.

Yeah, I agree. It's a little weird, but it doesn't bother me too much. It's also worth noting that site authors will have the option to opt-out of these overlays entirely in case they consider it a problem (#284).

As per the notes in #6, this should be fairly solid on the a11y front. The lightest color possible with these alternate overlays is actually darker than that of our usual overlays (which already passes WCAG AA). 👍

Once we sort out those merge conflicts (I'm happy to take a stab at it if you'd like — just let me know), and figure out why there's a lightness difference between the in-feed images and the single-post images, this should be good to go.

Thanks again for seeing this issue through! 🙌

@LittleBigThing
Copy link
Author

I am not sure why it looks the same on browsers that support mix-blend-mode, probably something to do with the effect it generates, but indeed, the post-feed version could use the additional dark layer on IE and Edge (the fourth layer of the featured image styles on single posts). Something like this:

.post-thumbnail-inner:after {
    background: rgba(0,0,0,.35);
    content: "";
    display: block;
    height: 100%;
    opacity: .5;
    position: absolute;
    top: 0;
    width: 100%;

    @supports (mix-blend-mode) {
        display: none;
    }
}

adding a dark layer for the featured images in the post feed for IE and Edge
@LittleBigThing
Copy link
Author

LittleBigThing commented Oct 26, 2018

Far from being a Github expert, I suppose that I better start all over to fix those conflicts over after the option of turning off the image filter will be merged (#327)? Or is there a better order to merge? :-)

Csaba added 2 commits October 26, 2018 14:45
fixing the positioning of the site logo in IE
This reverts commit 43fdb79.
@kjellr
Copy link
Collaborator

kjellr commented Oct 26, 2018

Far from being a Github expert, I suppose that I better start all over to fix those conflicts over after the option of turning off the image filter will be merged (#327)? Or is there a better order to merge? :-)

I don't think there will be issues, but I do think that #327 will be ready to go shortly, so let's hold off just to double check. 👍

@kjellr
Copy link
Collaborator

kjellr commented Oct 28, 2018

Closing in favor of #375.

@kjellr kjellr closed this Oct 28, 2018
@kjellr kjellr removed their request for review October 28, 2018 17:42
@LittleBigThing LittleBigThing deleted the fix-opacity branch October 29, 2018 06:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants