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

Update Cover Block background-attachment: fixed iOS bug handling to properly work with iOS 13+ #40087

Conversation

tomasztunik
Copy link
Contributor

@tomasztunik tomasztunik commented Apr 6, 2022

What?

Update @supportscheck disabling background-attachment: fixed for iOS devices to support latest versions of iPadOS and desktop mode.

Fixes #17718

Why?

The current solution was disabled when iPad Safari was rendering pages in desktop mode. Support for -webkit-overflow-scrolling: touch was removed in desktop mode in iOS 13 as mentioned in #17718

How?

As suggested in #17718 we change the @supports check from -webkit-overflow-scrolling: touch to -webkit-touch-callout: inherit

Testing Instructions

  1. Create a new page with a Cover Block, set a background image and toggle the fixed background option.
  2. View the page on an iPad (device or simulator).
  3. Toggle between desktop and mobile views.

The block should render the same.

Currently, the block in desktop mode would render oversized as it would try to use backgorund-position: fixed but the parallax effect would not work.

(a smaller image might make the issue more apparent)

Screenshots or screencast

how to toggle before after
Screenshot 2022-04-06 at 09 48 22 Screenshot 2022-04-06 at 09 37 59 Screenshot 2022-04-06 at 09 38 34

@tomasztunik tomasztunik requested a review from ajitbohra as a code owner April 6, 2022 07:58
@paaljoachim
Copy link
Contributor

Thanks for working on this @tomasztunik

I am not testing this correctly.
I have a physical Ipad beside me.
Went to this web site: https://opsalgard.no/
Uses the theme estar. Has various plugins activated.
Cover block with an image that has minimum height of cover 850.
Fixed background toggle is on.

It looks good on desktop view, but I am still having the problem at the iPad view.
I need to know how to cancel out the none working CSS code and instead add in the code that should work. I can then retest it out with the physical iPad.
Thanks!

@tomasztunik
Copy link
Contributor Author

tomasztunik commented Apr 7, 2022

If you add this custom SCSS/CSS it should work.
For the test purpose added !important to make sure it overrides the current value.

scss

.wp-block-cover-image,
.wp-block-cover {
    &.has-parallax {
        @supports (-webkit-touch-callout: inherit) {
           background-attachment: scroll !important;
       }
    }
}

css

@supports (-webkit-touch-callout: inherit) {
   .wp-block-cover.has-parallax,
   .wp-block-cover-image.has-parallax {
      background-attachment: scroll !important;
   }
}

PS.
Also looks like an awesome place!

@paaljoachim
Copy link
Contributor

Thank you for the code!

Before adding the CSS code

Before turning on "Fixed background"

None-fixed-background

After turning on "Fixed background"
Fixed-background

After adding the CSS code to the customizer.

(Ignore the placement of the text. I was testing things with the Grid (Change content position)

After-adding-CSS-fix

The code works really well!
Thank you very much @tomasztunik

@paaljoachim paaljoachim self-requested a review April 7, 2022 08:14
Copy link
Contributor

@paaljoachim paaljoachim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code works really well!
Thank you!

@paaljoachim paaljoachim added [Type] Bug An existing feature does not function as intended Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) labels Apr 12, 2022
@paaljoachim
Copy link
Contributor

@tellthemachines @talldan @ndiego
It would be nice to get this small CSS fix PR into WP 6.0.
Thanks!

@ndiego ndiego added Needs Testing Needs further testing to be confirmed. and removed Needs Testing Needs further testing to be confirmed. labels Apr 19, 2022
@ndiego
Copy link
Member

ndiego commented Apr 19, 2022

I do not have an iPad, but if we are confident in this fix, lets get it merged and we can consider for Beta 3

@tomasztunik
Copy link
Contributor Author

For the issue here this is the only way to tackle it, it doesn't change the fix itself just changes how we detect the platform which is backwards compatible with older iOS versions. I think as it was around since iOS 13 it would be worth getting it in.

@Mamaduka Mamaduka merged commit fe7644c into WordPress:trunk Apr 26, 2022
@Mamaduka Mamaduka added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Apr 26, 2022
@github-actions github-actions bot added this to the Gutenberg 13.2 milestone Apr 26, 2022
adamziel pushed a commit that referenced this pull request Apr 29, 2022
@adamziel
Copy link
Contributor

I cherry picked this change into wp/6.0 branch to be included in WordPress 6.0 Beta 4 on Monday: 01df2a5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Bug An existing feature does not function as intended
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Cover Block: Fixed background problem in iPadOS 13.x Safari
5 participants