Skip to content

handle natively focusable, disabled, and hidden elements in focustrap#11245

Merged
riknoll merged 3 commits intomasterfrom
dev/riknoll/microbit-fix-focuslist
Apr 6, 2026
Merged

handle natively focusable, disabled, and hidden elements in focustrap#11245
riknoll merged 3 commits intomasterfrom
dev/riknoll/microbit-fix-focuslist

Conversation

@riknoll
Copy link
Copy Markdown
Member

@riknoll riknoll commented Apr 2, 2026

fixes microsoft/pxt-microbit#6744

this PR modifies our isFocusable logic to handle natively focusable elements (like anchor tags with hrefs), elements with the disabled attributes, and hidden elements.

also adds two helper functions: getFocusableDescendants and getTabbableDescendants which get the elements that are focusable or tabbable respectively. in general, i tried to write the code so that it calls isVisible as infrequently as possible since that is an expensive check.

also includes a fix for the focus indicator outline on social buttons which i noticed while testing the use case in the linked issue.

@microbit-robert @microbit-matt-hillsdon FYI

@riknoll riknoll requested a review from a team April 2, 2026 22:57
let currentNode: Node | null = walker.nextNode();
while (currentNode) {
elements.push(currentNode as Element);
currentNode = walker.nextNode();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are there any scenarios where the nextNode() will always return something and we get stuck in this loop?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i don't think it's possible, unless you have some really twisted recursive xml or something like that

@riknoll riknoll enabled auto-merge (squash) April 6, 2026 16:59
@riknoll riknoll merged commit 51f62fd into master Apr 6, 2026
20 checks passed
@riknoll riknoll deleted the dev/riknoll/microbit-fix-focuslist branch April 6, 2026 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FocusTrap.tsx has incomplete focusable element list

2 participants