fix: respect scroll-margin when navigating to a url-supplied anchor#15246
fix: respect scroll-margin when navigating to a url-supplied anchor#15246Rich-Harris merged 15 commits intosveltejs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 6fb92b7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
LGTM! I made some suggestions to add comments that are pretty much what you've explained in the PR description. Although, I have a bit of uneasiness because I can't remember if all browsers (on different platforms) have the same behaviour for location.replace although I tested this many years ago. Also, sorry this took so long to get around to 😓
Maybe @Rich-Harris would want to take a second look to see if this would regress #14569 ? That one doesn't have a test and the PR description mentions it's difficult to reproduce (but it makes sense)
Co-authored-by: Tee Ming <chewteeming01@gmail.com>
Co-authored-by: Tee Ming <chewteeming01@gmail.com>
|
IMO this should also fix sveltejs/svelte.dev#1693 but it still seems to be broken in the preview svelte-docs-bot posted.
Wonder why. But it fixes #15192 - see https://www.sveltelab.dev/qk496863qm9cro3 |
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
Does the Kit version need to be updated manually? I think only the docs are updated from the bot
This one's really strange because I couldn't reproduce it from the issue or from the sveltelab link. But I can reproduce it from the test scenario in this PR |
Ahh, makes sense. Maybe it shouldn't create a preview when the PR does not touch any docs then? :D
The sveltelab link I posted uses the fix from this PR. |
You'll have to ask @elliott-with-the-longest-name-on-github if that's possible 😆 |
|
very excited to apply this fix to svelte.dev 🎉 |
…veltejs#15246) * test * fix * changeset * revert import sorting * Update packages/kit/src/runtime/client/client.js Co-authored-by: Tee Ming <chewteeming01@gmail.com> * Update packages/kit/src/runtime/client/client.js Co-authored-by: Tee Ming <chewteeming01@gmail.com> * Update packages/kit/src/runtime/client/client.js Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> --------- Co-authored-by: Tee Ming <chewteeming01@gmail.com> Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
fixes #15192
When we navigate to a url-supplied anchor, we don't need to scroll to it in
reset_focus. Thelocation.replace()call already scrolls to the anchor and respectsscroll-margin-top/scroll-padding-top& smooth scrolling.The previous solution (implemented in #14569) used
getBoundingClientRect()which does not takescroll-margin-top/scroll-padding-topinto account.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits