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: replace beforeunload to pagehide for safafi on ios #9522

Merged

Conversation

jakkku
Copy link
Contributor

@jakkku jakkku commented Oct 30, 2022

Fixes #9463

here is the codesandbox
please check it in safari on ios

beforeunload event does not occur in safari on ios
Apple's documentation says to use pagehide instead of unload.

unload
Deprecated, use pagehide instead.

the mdn documentation says

When the user clicks the browser's back button,
the page hide event is fired on the current page before the previous page is displayed.

So I think it would be okay to use pagehide instead of beforeunload.

This is my first time working in react-router's codebase. Please let me know if there are any additional considerations

@changeset-bot
Copy link

changeset-bot bot commented Oct 30, 2022

🦋 Changeset detected

Latest commit: 29077f4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
react-router-dom Patch
react-router Patch
react-router-dom-v5-compat Patch
react-router-native Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Oct 30, 2022

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@brophdawg11 brophdawg11 changed the base branch from dev to brophdawg11/pagehide January 19, 2023 18:45
@brophdawg11
Copy link
Contributor

Thanks @jakkku! We can't change useBeforeUnload directly since that would be a breaking change, but we can look into using the pagehide event inside <ScrollRestoration>. I'm going to merge this into a branch of mine to make those changes and do some testing.

@brophdawg11 brophdawg11 merged commit fcc3018 into remix-run:brophdawg11/pagehide Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants