Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try fix for Cover Image parallax in Chrome #2872

Closed
wants to merge 1 commit into from

Conversation

jasmussen
Copy link
Contributor

Description

When anything but the body is the scrolling container, Chrome doesn't know how to handle background-attachment: fixed;, as such the recent change to this regressed the Cover Image block in Chrome.

This branch is an experiment to use a different approach to fix the block. It puts the image itself on a separate layer, and uses clip-path to "crop it". This makes it work in Chrome, Safari, etc. But it is kind of a hack, so we need to test this thoroughly.

One benefit, however, if we can get this to work, is that performance should be much better as the image layer is now rendered on the GPU.

How Has This Been Tested?

Chrome, Safari, still lots of testing to do, and tweaks to do.

Screenshots (jpeg or gifs if applicable):

cover image

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

When anything but the body is the scrolling container, Chrome doesn't know how to handle `background-attachment: fixed;`, as such the recent change to this regressed the Cover Image block in Chrome.

This branch is an experiment to use a different approach to fix the block. It puts the image itself on a separate layer, and uses `clip-path` to "crop it". This makes it work in Chrome, Safari, etc. But it is kind of a hack, so we need to test this thoroughly.

One benefit, however, if we can get this to work, is that performance should be much better as the image layer is now rendered on the GPU.
@jasmussen jasmussen added the [Status] In Progress Tracking issues with work in progress label Oct 4, 2017
@jasmussen jasmussen self-assigned this Oct 4, 2017
@jasmussen jasmussen requested review from mtias and aduth October 4, 2017 11:44
@jasmussen
Copy link
Contributor Author

This PR still feels glitchy. I'm hoping I can tune that. An alternative approach, and probably better overall, is to iframe the editor. #2420

@aduth
Copy link
Member

aduth commented Oct 4, 2017

I'm having trouble recreating the original issue that this is seeking to resolve.

@jasmussen
Copy link
Contributor Author

I am going to investigate this thoroughly tomorrow. I'm crossing my fingers that you have a newer version of Chrome than I developed this on, where the bug is fixed in Chrome.

@jasmussen
Copy link
Contributor Author

jasmussen commented Oct 5, 2017

Oh this is INTERESTING!

Since @aduth couldn't reproduce this I've investigated further. As it turns out, this works fine on 1x (non retina) screens. It does not work on my iMac screen. In fact I can reproduce it easily on one screen, drag it on to the connected 1x screen on the side, and not reproduce it there.

Can anyone try this on other retina screens? Like an MBP 15, or other iMac?

@jasmussen
Copy link
Contributor Author

Note, I'm on Sierra on this iMac. I'll try upgrading to High Sierra in the when the .1 comes, see if that fixes it. Could be a driver issue or something. Very very weird.

I also confirmed that this works fine, on the iMac, in Firefox (and Safari), but it still doesn't work in Chrome Canary.

@jasmussen
Copy link
Contributor Author

Confirmed. This is an issue with Mac Chrome and retina screens. Bug here: https://groups.google.com/a/chromium.org/forum/#!topic/chromium-bugs/zJcpzQY_aUY (please consider starring it). Here's a video showing the issue:

https://cloudup.com/crw0Cau7z_L

In that video I'm dragging from a non retina screen to a retina screen to reproduce.

@aduth
Copy link
Member

aduth commented Oct 6, 2017

Would that mean this would have always been an issue regardless of the change to the scroll container?

@jasmussen
Copy link
Contributor Author

Would that mean this would have always been an issue regardless of the change to the scroll container?

No, my apologies I should've been more clear. It's a combination of the aforementioned general scroll bug, and a Chrome specific retina bug. If I uncheck the "overflow-y: auto" on the main scrollable container and let the html element scroll instead, it works fine also on retina screens.

@jasmussen
Copy link
Contributor Author

Closing this for now. It will either be fixed upstream in Chrome, or we can revisit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants