Skip to content

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

@jakkku jakkku force-pushed the fix/useScrollRestoration branch from 0a47956 to 6f44e5e Compare October 30, 2022 06:45
@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! 🥳

@jakkku jakkku force-pushed the fix/useScrollRestoration branch from 6f44e5e to 0536431 Compare October 30, 2022 06:54
@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.

3 participants