Skip to content

Conversation

@grnd-alt
Copy link
Contributor

The Focus-Trap library will throw an error if a trap does not have any elements it can focus. Therefore we need something to set focus on if there's nothing else to use. In the documentation it is recommended to give the container a tab-index of -1 to make it focusable but not appear in the tab flow, and then set that as the fallBack to focus on when theres nothing else in the container.

}
const el = this.getPopoverContentElement()
el.tabIndex = -1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we only set it if there is no existing tab index?

Copy link
Contributor

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense looking at the focus-trap documentation for fallBackFocus, but would appreciate some additional feedback from @ShGKme or @susnux here :)

@susnux susnux added bug Something isn't working 3. to review Waiting for reviews labels Apr 1, 2025
@susnux susnux added this to the 8.24.0 milestone Apr 1, 2025
@susnux
Copy link
Contributor

susnux commented Apr 1, 2025

/backport to next

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense also from documentation of focus trap

@susnux susnux merged commit 4fa2be4 into master Apr 1, 2025
@susnux susnux deleted the fix/no-fallback-focus branch April 1, 2025 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants