focus-layers
focus-layers copied to clipboard
Fix focus loosing for last page element
There is a bug.
If the focus is on the last element of the page, by pressing Tab button, focus moves to the address bar of the browser. After then focus move to first focusable element of the page. Your hook tracks focus movement inside focus layer component. For this reason, focus trap stops working.
Also this example currently not works too.
@faultyserver I tried to update my project too, but found this regression in 6.1.
It was the reason why I was not able to use my focus-layers fix from #10
@faultyserver can we accept this small fix. I see warning from my #10 on every yarn upgrade-interactive and it bother me.
Hello! Sorry for such a delayed response.
I'm not sure this is the approach that we'd want to take here. The forceFirst option of wrapFocus is really a utility for when focus first moves into a new container, where there is no directionality. The use case is in these lines, where as the comment says, we need to automatically move focus if it hasn't already moved:
https://github.com/discord/focus-layers/blob/e36120d7aa1b4a7c9fc79cbbab525a968e08fa42/src/useFocusLock.tsx#L98-L108
An example of where this wouldn't be wanted is if the user was using shift+tab to move backwards through the document. When they go past the first element on the page and back into the browser's controls, this would just stop focus on that element instead of wrapping to the end as expected.
I think the brokenness that you're experiencing here is from not including FocusGuards around your app which create a fake tab stop to prevent exactly this issue of focus moving out of the document from the first or last elements on the page. The first example in the README needs to be updated to include these, but later on in the docs this is mentioned under "Edge Guards".
Additionally, changing attachTo to window for the event listeners limits the flexibility of the hooks, because it wouldn't be (easily) possible to have a component higher in the tree intercept focus events and pre-handle them. This isn't a normal use case, but is something we want to preserve as an option.
Hopefully that fixes the behavior you're seeing! If not, please let us know.