Skip to content

Commit

Permalink
fix: defer when clicking hash links with data-sveltekit-reload (#12866)
Browse files Browse the repository at this point in the history
fixes #12582
fixes #12188

This PR ensures that clicking on a hash link with the reload option avoids the usual behaviour of thinking the page will reload and instead allows it to be handled as if the option wasn't enabled (hash links on the same page never reload the page).

It also fixes a behaviour where an element doesn't get focused when clicking on the hash link more than once.

---------

Co-authored-by: Simon H <[email protected]>
  • Loading branch information
eltigerchino and dummdidumm authored Oct 28, 2024
1 parent bcb30cd commit 5006435
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilled-cats-hang.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: ensure element is focused after subsequent clicks of the same hash link
5 changes: 5 additions & 0 deletions .changeset/strange-buckets-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: avoid reloading behaviour for hash links with data-sveltekit-reload if the hash is on the same page
14 changes: 10 additions & 4 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -2089,8 +2089,11 @@ function _start_router() {

if (download) return;

const [nonhash, hash] = url.href.split('#');
const same_pathname = nonhash === strip_hash(location);

// Ignore the following but fire beforeNavigate
if (external || options.reload) {
if (external || (options.reload && (!same_pathname || !hash))) {
if (_before_navigate({ url, type: 'link' })) {
// set `navigating` to `true` to prevent `beforeNavigate` callbacks
// being called when the page unloads
Expand All @@ -2105,8 +2108,7 @@ function _start_router() {
// Check if new url only differs by hash and use the browser default behavior in that case
// This will ensure the `hashchange` event is fired
// Removing the hash does a full page navigation in the browser, so make sure a hash is present
const [nonhash, hash] = url.href.split('#');
if (hash !== undefined && nonhash === strip_hash(location)) {
if (hash !== undefined && same_pathname) {
// If we are trying to navigate to the same hash, we should only
// attempt to scroll to that element and avoid any history changes.
// Otherwise, this can cause Firefox to incorrectly assign a null
Expand All @@ -2121,7 +2123,11 @@ function _start_router() {
if (hash === '' || (hash === 'top' && a.ownerDocument.getElementById('top') === null)) {
window.scrollTo({ top: 0 });
} else {
a.ownerDocument.getElementById(decodeURIComponent(hash))?.scrollIntoView();
const element = a.ownerDocument.getElementById(decodeURIComponent(hash));
if (element) {
element.scrollIntoView();
element.focus();
}
}

return;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<a href="#example" data-sveltekit-reload>focus</a>
<input id="example" />
<a href="/data-sveltekit/reload/hash/new">new page</a>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>hello world</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<a href="#example">focus</a>
<input id="example" />
24 changes: 24 additions & 0 deletions packages/kit/test/apps/basics/test/cross-platform/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,30 @@ test.describe('Routing', () => {
expect(await page.textContent('#page-url-hash')).toBe('#target');
});

test('clicking on a hash link focuses the associated element', async ({ page }) => {
await page.goto('/routing/hashes/focus');
await page.locator('a[href="#example"]').click();
await expect(page.getByRole('textbox')).toBeFocused();
// check it still works when the hash is already present in the URL
await page.locator('a[href="#example"]').click();
await expect(page.getByRole('textbox')).toBeFocused();
});

test('backwards navigation works after clicking a hash link with data-sveltekit-reload', async ({
page,
clicknav,
baseURL
}) => {
await page.goto('/data-sveltekit/reload/hash');
await page.locator('a[href="#example"]').click();
expect(page.url()).toBe(`${baseURL}/data-sveltekit/reload/hash#example`);
await clicknav('a[href="/data-sveltekit/reload/hash/new"]');
expect(page.url()).toBe(`${baseURL}/data-sveltekit/reload/hash/new`);
await page.goBack();
expect(page.url()).toBe(`${baseURL}/data-sveltekit/reload/hash#example`);
await expect(page.getByRole('textbox')).toBeVisible();
});

test('back button returns to previous route when previous route has been navigated to via hash anchor', async ({
page,
clicknav
Expand Down

0 comments on commit 5006435

Please sign in to comment.