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

Fix position fixed causing layout shifts on standalone sites (pwa) #277

Merged

Conversation

Cuzart
Copy link
Contributor

@Cuzart Cuzart commented Feb 21, 2024

I simply added a new condition to only call setPositionFixed when display mode is set to standalone like in most PWA's.

Solves #199 where you can find more details about the problem. I personally don't think this needs an extra prop for more control over this behaviour but maybe there is a good reason for including one?

Copy link

vercel bot commented Feb 21, 2024

@Cuzart is attempting to deploy a commit to the emil Team on Vercel.

A member of the Team first needs to authorize it.

@Dearest
Copy link

Dearest commented Feb 22, 2024

Disappointed, this PR didn't solve the issue #199 . In the iOS app's webview, the return value of executing window.matchMedia('(display-mode: standalone)').matches is false.

@Cuzart
Copy link
Contributor Author

Cuzart commented Feb 22, 2024

Well it solves the issue for pwa's. For native app's with webviews the condition is not met you're right I somehow missed that point of the discussion tbh as my main goal was to fix it for pwa which affected my own app.

I tried to add another condition to avoid setting position fixed on webview's
const isWebView = /(iPhone|iPod|iPad).*AppleWebKit(?!.*Safari)/i.test(navigator.userAgent); found here

but the safe inset on the bottom is still there. Not sure if this is because of the preview in Xcode or the view needs some styling or configuration but the bottom inset is always visible independent from the drawer. (I am no swift dev)

If wanted I could add this second condition to the PR :)

Bildschirmaufnahme.2024-02-22.um.12.05.11.mov

This is a xcode simulator preview with a WKWebView rendering the localhost dev server from running npm run dev:test (I add a absolute positioned element to the body when the condition is met to see if it catches determining whether the user agent runs in a webview)

@emilkowalski emilkowalski merged commit 67163bf into emilkowalski:main Apr 6, 2024
1 check failed
Comment on lines +99 to +101
// avoid for standalone mode (PWA)
const isStandalone = window.matchMedia('(display-mode: standalone)').matches;
!isStandalone && setPositionFixed();
Copy link

@AntoLepejian AntoLepejian Jun 30, 2024

Choose a reason for hiding this comment

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

I think this needs an early return.

I tracked down a bug I was having only on iOS PWAs to vaul release 0.9.1, and this commit is most likely the culprit.

The content below my drawer is jumping up and down, and the set-timeout in this if statement is most likely why. (other issues are happening too, like any time I click anything in the drawer, the contents jumping up and down without my touch events being registered)

If we're no longer fixing the position for PWAs, we don't need to restore it after 500ms either, right?

@emilkowalski @Cuzart

Suggested change
// avoid for standalone mode (PWA)
const isStandalone = window.matchMedia('(display-mode: standalone)').matches;
!isStandalone && setPositionFixed();
// avoid for standalone mode (PWA)
const isStandalone = window.matchMedia('(display-mode: standalone)').matches;
if (isStandalone) return;
setPositionFixed();

Copy link
Contributor Author

@Cuzart Cuzart left a comment

Choose a reason for hiding this comment

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

Looks good to me. I did not experience any similar issues but I agree it makes sense to return early

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants