SvelteKit should not set tabIndex=-1 #3501
Comments
|
The line in question: It looks like this was added in #309 to implement #307 @geoffrich explained the motivation for this here: #3381 (comment) This isn't the section of code I'm most familiar with, but I hope that helps describe the original goals / motivations. Happy to consider PRs to improve this! |
|
I think we should take another look at this behavior. It seems to be causing some obscure issues (see also #1699, where the root cause is a Chrome bug, but one that only happens when A mitigation might be only setting tabindex on the body when it's about to be focused, and then removing it after the focus has occurred. This will require testing with various browsers and screen readers to make sure focus is set appropriately. However, I think a more long term solution will be rethinking our focus management solution to be more robust (and customizable by the user). |
This comment has been minimized.
This comment has been minimized.
I've found that this particular behavior causes problems with mixed mouse/keyboard navigation. If you click on a scrollable region and then try to use the arrow keys to scroll within the region, the body scrolls instead (or nothing happens if body isn't scrollable). If anyone else is running into issues like this, it can be worked around by also adding |
|
Since there are a few issues caused by const doc = document.documentElement;
doc.setAttribute('tabindex', '-1');
doc.focus();
doc.removeAttribute('tabindex');I did some tests and, informally, it seems to work just as well. It didn't seem to trip Narrator or NVDA, and worked across Chromium and Firefox (don't know about Safari). I'll PR this change soon, but we should probably look for adverse effects. |
Describe the bug
SvelteKit adds tabIndex=-1 to the document body in order to be able to manage focus upon navigation: #307
This is not a "safe" change, in that it may impact the behavior of components on the page. In particular, this change means that some actions that would normally not trigger a
focusevent (like mousedown on some text) will now fire afocusevent on the document body. These extrafocusevents may confuse code that does its own focus management (for example, a focus trap). They were the source of two different user-reported bugs in my component library: rgossiaux/svelte-headlessui#36 and rgossiaux/svelte-headlessui#39Looking at the motivation for introducing this originally, I don't see why it can't be replaced with something better. Can SvelteKit clean up after itself after navigation by removing the
tabIndex? Alternatively, can it do something that doesn't require setting this at all? For example, Headless UI has a function to focus on the first focusable element within a container: https://github.com/tailwindlabs/headlessui/blob/cae976a18b7955670cd28fb1067a49917bda2c09/packages/%40headlessui-react/src/utils/focus-management.ts#L105 This works by using a CSS selector to identify potentially-focusable elements, then attempting to focus them one by one until a success.Reproduction
Not sure this needs a special test repro since this happens on every SvelteKit app, but I can create one if you disagree.
The bugs that this caused can be reproduced at this commit: https://github.com/rgossiaux/svelte-headlessui/tree/ac1f86ac156a94c86e81647c55d8bb56c035edc6
Logs
No response
System Info
System: OS: macOS 12.1 CPU: (10) arm64 Apple M1 Max Memory: 4.11 GB / 64.00 GB Shell: 3.3.1 - /opt/homebrew/bin/fish Binaries: Node: 16.8.0 - /var/folders/rn/2kdgb0jj25q2v656j3qc_ywr0000gn/T/fnm_multishells/23841_1640400023703/bin/node Yarn: 1.22.17 - /opt/homebrew/bin/yarn npm: 7.21.0 - /var/folders/rn/2kdgb0jj25q2v656j3qc_ywr0000gn/T/fnm_multishells/23841_1640400023703/bin/npm Browsers: Chrome: 97.0.4692.99 Safari: 15.2Severity
serious, but I can work around it
Additional Information
No response
The text was updated successfully, but these errors were encountered: