feat: add data-sveltekit-replacestate and -keepfocus options to links#9019
Conversation
| details: { | ||
| state: {}, | ||
| replaceState: url.href === location.href | ||
| replaceState: options.replace_state || url.href === location.href |
There was a problem hiding this comment.
I didn't make the corresponding form changes because I haven't used SvelteKit forms yet - so I don't think I'm best placed to make/verify any changes in that area!
|
Linting is failing because of a TS error in the following: <a id="one" href="/data-sveltekit/replacestate/target" data-sveltekit-replacestate>one</a>I couldn't find where I would make the appropriate updates to fix this. I tried looking for corresponding |
|
Just had another little look at this and realised the corresponding types are within sveltejs/language-tools. I can make a PR there too but I'm not sure what the process is for ensuring the changes to both would be released in sync? Update: PR within language-tools here: sveltejs/language-tools#1865 |
|
@dummdidumm what's the process for merging in changes across the 3 repos this affects? Will updates to sveltejs/language-tools and sveltejs/svelte get published first and then we can move forward with this PR? Is there anything else I can do to help on this PR here? |
|
Yes we will likely merge Svelte/language-tools first. That said - since you now nicely laid out all the needed PRs across the repos, would you be open to also adding |
|
Sure, I'll try and have a look in the next few days. |
|
I've added |
| <a data-sveltekit-keepfocus href="/path">Path</a> | ||
| ``` | ||
|
|
||
| ...will cause the currently focused element to retain focus after navigation. Otherwise, focus will be reset to the body. |
There was a problem hiding this comment.
Note to self to try out: I'm wondering if this really keeps focus on the element prior to clicking the a tag, or if the a tag itself is then the focus. Probably the latter, which is why this mainly makes sense for <form> elements, which we probably should note here
There was a problem hiding this comment.
I'll give this a closer look later, but I think we should have some sort of note saying this is primarily intended for forms. I think it's usually an antipattern for links, unless you know what you're doing.
There was a problem hiding this comment.
Confirmed the currently-focused element is the link itself. I think the language is fine as-is, not sure there's a better way to phrase it that doesn't exclude the form case. I added a suggested paragraph below discussing the a11y considerations.
dummdidumm
left a comment
There was a problem hiding this comment.
Thank you for your PRs across all three needed repositories to get this in!
An additional
data-sveltekit-replacestateoption on top of the existing options.Closes #9014
Closes #7895
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:.