Skip to content

Conversation

@ReverseGem7
Copy link
Contributor

focus-manager.mp4

@ReverseGem7 ReverseGem7 marked this pull request as ready for review September 4, 2025 17:24
@kommander
Copy link
Collaborator

What about having a tabIndex on Renderables to control what should be "tabable" and what not? Would then also work with custom Renderables.

@ReverseGem7
Copy link
Contributor Author

ReverseGem7 commented Sep 7, 2025

Right now, a Renderable is considered tabbable if it has focusable: true and visible: true. If a custom Renderable has children (also Renderables) with these flags, they should be traversed by the focus manager too.

If we wanted to change this and allow Tab navigation inside a Renderable, we could add something like a toggleTabIndex function on the RenderContext to override the current behavior. We could also consider adding a separate tabbable property to Renderables instead of using focusable.

What’s your opinion?

@kommander
Copy link
Collaborator

I am a bit concerned about tree traversal for finding tabables, as that might also traverse long lists with potentially thousands of nodes and block the process. I would make that a built-in collector, on the RenderContext add methods to collect the focusable when it is added to the tree and remove it from the list when removed from the tree. Then it's just fast iteration on a small list, compared to tree traversal.

@ReverseGem7
Copy link
Contributor Author

something like creating a secondary tree?

@kommander
Copy link
Collaborator

Just a list of Renderables, sorted by tabIndex. Doesn't matter where in the tree they are then, can still check if visible or not and take the next/prev available visible renderable to focus.

@ReverseGem7 ReverseGem7 marked this pull request as draft September 9, 2025 20:07
@ReverseGem7 ReverseGem7 marked this pull request as ready for review September 10, 2025 15:16
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.

2 participants