Skip to content

Conversation

@minetoblend
Copy link
Contributor

@minetoblend minetoblend commented Oct 24, 2025

Companion to ppy/osu#35439

Moves the focus movement logic into as an extension function to cover 2 use-cases:

  • implementing ITabbableContainer without inheriting from TabbableContainer
  • being able to control focus movement from the parent

Also adds the option to navigate to the first valid focus target if no child is currently focused through the requireFocusedChild parameter (naming kinda bad I know, but I couldn't think of anything better).

stack.Push(target);

// If we don't have a currently focused child we pretend we've already encountered our target child to move focus to the first valid target.
bool started = currentlyFocused == null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Highlighting this line because it's the only line that doesn't match the original implementation. Should act the same as before unless requireFocusedChild is false.

@smoogipoo
Copy link
Contributor

Couldn't you just put a TabbableContainer in the hierarchy somewhere?

@minetoblend
Copy link
Contributor Author

@smoogipoo While it's theoretically possible to use that class, I can't extend TabbableContainer in DrawableTernaryButton itself since it already extends from OsuButton so I'd have to put it as an intermediary drawable somewhere and having the focused drawable be another element than the one being interacted with seems like a code smell to me.

Another reason is that I'm hoping to eventually have this logic be handled by the TabbableContentContainer. There's a bunch of caveats with having the logic for this inside TabbableContainer. For exampe, if you navigate to a TabbableContainer without a TabbableContentContainer set you cannot navigate away from it with tab anymore, which is an easy thing to overlook.

Copy link
Contributor

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

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

I suppose this is fairly harmless in isolation

@smoogipoo smoogipoo merged commit 0c8bac9 into ppy:master Nov 5, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants