Fix paddings not taking unsafe area into account #528
Conversation
wojtekmaj
changed the title
| pageHeaderContainer.style.paddingLeft = 'max(1.5rem, env(safe-area-inset-left))'; | ||
| pageHeaderContainer.style.paddingRight = 'max(1.5rem, env(safe-area-inset-right))'; |
There was a problem hiding this comment.
This doesn't seem like it would work on IE11? Is it possible to have a graceful fallback?
There was a problem hiding this comment.
Graceful fallback is provided in L25 - see https://github.com/pmmmwh/react-refresh-webpack-plugin/pull/528/files#r748701391
There was a problem hiding this comment.
Would we need this as well? Thinking about it the answer seems to be no since we have a global wrapper outside of the page header?
pageHeaderContainer.style.paddingBottom = 'max(1rem, env(safe-area-inset-bottom))';
pageHeaderContainer.style.paddingTop = 'max(1rem, env(safe-area-inset-top))';There was a problem hiding this comment.
I had no intention of changing the behavior in any way other than expanding paddings when needed if they are smaller than env-area-inset-*.
So the answer is: If pageHeaderContainer.style.padding = '1rem 1.5rem'; is not needed, then these two lines also aren't. But if it's to stay, these overrides are also to stay to allow expading paddings beyond 1.5rem if needed.
There was a problem hiding this comment.
I've tested on a device with a notch and I just realised that the top bar actually never hit the unsafe area due to the browser address bar, so I think we won't need the top overrides 👍🏻
There was a problem hiding this comment.
Not if you display the webpage as PWA with fullscreen mode :)
| footer.style.bottom = '0'; | ||
| footer.style.bottom = 'env(safe-area-inset-bottom)'; |
There was a problem hiding this comment.
Values are ignored by the browsers if they don't understand the syntax, so previous value should be retained, much like in pure CSS:
bottom: 0; /* Fallback for older browsers */
bottom: env(safe-area-inset-bottom);Try this:
el.style.marginTop = '100px';
el.style.marginTop = 'pee pee poo poo';and you'll see that because the browser didn't understand the 2nd value, 100px is retained. And that's how IE11 should remain compatible!
| pageHeaderContainer.style.paddingLeft = 'max(1.5rem, env(safe-area-inset-left))'; | ||
| pageHeaderContainer.style.paddingRight = 'max(1.5rem, env(safe-area-inset-right))'; |
There was a problem hiding this comment.
Would we need this as well? Thinking about it the answer seems to be no since we have a global wrapper outside of the page header?
pageHeaderContainer.style.paddingBottom = 'max(1rem, env(safe-area-inset-bottom))';
pageHeaderContainer.style.paddingTop = 'max(1rem, env(safe-area-inset-top))';
| footer.style.paddingBottom = '0'; | ||
| footer.style.paddingBottom = 'env(safe-area-inset-bottom)'; |
There was a problem hiding this comment.
Instead of moving the fixed button panel up, I opted to add useless padding to the panel so at the background color would bleed into the unsafe area.
| iframe.style.minHeight = '100vh'; | ||
| iframe.style.minHeight = '-webkit-fill-available'; |
There was a problem hiding this comment.
Fixes an issue where the button panel would be hidden due to how 100vh is calculated on iOS Safari.
When website on which overlay is displayed uses
viewport-fit: cover, website declares it will handle unsafe areas on its own. This also applies to the overlay displayed on top of it.Before:
After:
