Open felskov opened 3 years ago
Hey! Thanks for opening an issue. Any chance you'd be willing to provide a codesandbox.io ?
If I'm reading this correctly, there's a more root problem to the issue you're having, which is that FocusScopes don't nest well and the contain restoreFocus
are ignored. Is there a reason we shouldn't start there?
Thanks for the fast reply!
No reason not to start with the nested FocusScopes at all, I just figured that it might have been by design :) I've created a code pen highlighting the issue here:
https://codesandbox.io/s/funny-violet-kurvg
It shouldn't be possible to tab out side of the primary container on top due to contain, but that's not the case. I am easily able to navigate to the buttons outside that scope.
Thanks so much for that! Seems like a bug to me. Will bring to the rest of the team. We're in the middle of a few things, so it may be a few days.
No rush, thanks for looking into you.
If you fix the issue with nested FocusScope we wont need the original feature request. But I still think you might want to consider exposing the option - could be useful in other situations, and seems easy to add an option to prevent scroll when using the focusManager.
Note to the team/whoever picks this up: the issue to be investigated/fixed here is the focus containment behavior of a parent FocusScope breaks when there is a FocusScope nested within w/ autoFocus
applied to it, e.g.
<FocusScope contain>
// FOCUSABLE ELEMENTS HERE
<FocusScope autoFocus>
// FOCUSABLE ELEMENTS HERE
</FocusScope>
</FocusScope>
🙋 Feature Request
I'd like to be able to prevent scroll when using the focus manager imperatively. Currently the FocusManagerOptions don't include this option, and scrolling is a hardcoded setting - e.g. here in focusFirst:
https://github.com/adobe/react-spectrum/blob/b4a3821acce160a0013587b6891e864390a33efe/packages/%40react-aria/focus/src/FocusScope.tsx#L187
🤔 Expected Behavior
You have the option to prevent scroll when focussing elements through the focusManager.
😯 Current Behavior
It's impossible to disable scrolling when focusing elements using focusManager.
💁 Possible Solution
If we could simply pass an option { preventScroll: true } to focusFirst/-Last/-Next/-Prev this would provide us with the added flexibility to support our use case without needing too many workarounds. I'm guessing it would be relatively easy to add this option to the focusManager interface looking at the code?
🔦 Context
Our experience is that FocusScopes don't nest well. We have an overlay that opens up a sort of wizard containing multiple steps. The overlay has a FocusScope with
contain restoreFocus autoFocus
which works well when the overlay opens. But when navigating between steps, we'd like to autoFocus first input inside the next step.We initially tried using a nested
<FocusScope autoFocus>
which technically provided the autoFocus, but meant that thecontain restoreFocus
settings from the parent scope ended up being ignored.Instead we've tried implementing the autoFocus behaviour ourselves, using a mount effect and imperatively applying focus using useFocusManager(). This worked well until we implemented a transition between the steps - this transition is killed because the browser now tries to perform automatic scrolling while we're transitioning the UI.
💻 Examples
Simplified version of the code used to implement the auto focus behaviour when changing steps: