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

Revamp intercepted-navigation scroll handling #239

Merged
merged 4 commits into from Jul 13, 2022
Merged

Revamp intercepted-navigation scroll handling #239

merged 4 commits into from Jul 13, 2022

Conversation

domenic
Copy link
Collaborator

@domenic domenic commented Jun 15, 2022

This changes the default behavior of intercepted navigations with respect to scrolling in the following ways:

  • Scroll restoration is performed on reloads, not just traverses.
  • For pushes and replaces, we either scroll to the top or scroll to the fragment.

This change requires some updated API surface: the scrollRestoration option to intercept() has become scroll, and the navigateEvent.restoreScroll() method has become navigateEvent.scroll(). The latter method now has more capabilities, working to give browser-default-like behavior for "push", "replace", and "reload" in addition to "traverse".

Similar to before, all of this can be opted out of by using scroll: "manual".

Closes #237 by giving an easy default behavior for SPA navigations with hashes.

Closes #231 by making "push", "replace", and "reload" behave by default in an MPA-like manner with respect to scroll position, just like "traverse" does.


Preview | Diff

This changes the default behavior of intercepted navigations with respect to scrolling in the following ways:

* Scroll restoration is performed on reloads, not just traverses.
* For pushes and replaces, we either scroll to the top or scroll to the fragment.

This change requires some updated API surface: the scrollRestoration option to intercept() has become scroll, and the navigateEvent.restoreScroll() method has become navigateEvent.scroll(). The latter method now has more capabilities, working to give browser-default-like behavior for "push", "replace", and "reload" in addition to "traverse".

Similar to before, all of this can be opted out of by using scroll: "manual".

Closes #237 by giving an easy default behavior for SPA navigations with hashes.

Closes #231 by making "push", "replace", and "reload" behave by default in an MPA-like manner with respect to scroll position, just like "traverse" does.
Copy link

@jakearchibald jakearchibald left a comment

Choose a reason for hiding this comment

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

@domenic could I interest you in a { scroll: 'auto' } default, that acts like after-transition, unless navigateEvent.scroll() is called before the intercept promise settles?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@domenic
Copy link
Collaborator Author

domenic commented Jun 16, 2022

could I interest you in a { scroll: 'auto' } default, that acts like after-transition, unless navigateEvent.scroll() is called before the intercept promise settles?

Hmm, I wonder if we should just make that the behavior of "after-transition"...

@jakearchibald
Copy link

Yeah, that makes sense

@domenic domenic requested a review from natechapin June 29, 2022 09:02
Copy link
Collaborator

@natechapin natechapin left a comment

Choose a reason for hiding this comment

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

Just a couple nitpicks

README.md Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Show resolved Hide resolved
@domenic domenic merged commit cb425c0 into main Jul 13, 2022
@domenic domenic deleted the scroll-changes branch July 13, 2022 04:16
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 20, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 20, 2022
Follows WICG/navigation-api#239

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/DKeklNbLG5s

Bug: 1345507
Change-Id: Id5c1b9d77953388dbe287f6403b0a49c8359d51b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3758902
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026382}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 20, 2022
Follows WICG/navigation-api#239

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/DKeklNbLG5s

Bug: 1345507
Change-Id: Id5c1b9d77953388dbe287f6403b0a49c8359d51b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3758902
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026382}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 22, 2022
…ation scroll handling, a=testonly

Automatic update from web-platform-tests
Navigation API: Revamp intercepted-navigation scroll handling

Follows WICG/navigation-api#239

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/DKeklNbLG5s

Bug: 1345507
Change-Id: Id5c1b9d77953388dbe287f6403b0a49c8359d51b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3758902
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026382}

--

wpt-commits: 5fb15e04adca47cc972a2e4c665853aca6a1a5b9
wpt-pr: 34919
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Jul 27, 2022
…ation scroll handling, a=testonly

Automatic update from web-platform-tests
Navigation API: Revamp intercepted-navigation scroll handling

Follows WICG/navigation-api#239

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/DKeklNbLG5s

Bug: 1345507
Change-Id: Id5c1b9d77953388dbe287f6403b0a49c8359d51b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3758902
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026382}

--

wpt-commits: 5fb15e04adca47cc972a2e4c665853aca6a1a5b9
wpt-pr: 34919
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
Follows WICG/navigation-api#239

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/DKeklNbLG5s

Bug: 1345507
Change-Id: Id5c1b9d77953388dbe287f6403b0a49c8359d51b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3758902
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026382}
NOKEYCHECK=True
GitOrigin-RevId: d64518f92c71b650d1b5f83dbfae7d434bfc95bf
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.

Making SPA-navs-with-hashes work Mismatch between scroll and focus handling
3 participants