Skip to content

Focus outlines#23628

Merged
alice-i-cecile merged 5 commits intobevyengine:mainfrom
viridia:focus_outlines
Apr 3, 2026
Merged

Focus outlines#23628
alice-i-cecile merged 5 commits intobevyengine:mainfrom
viridia:focus_outlines

Conversation

@viridia
Copy link
Copy Markdown
Contributor

@viridia viridia commented Apr 2, 2026

Objective

Part of #19236

Solution

Implements basic focus outlines for feathers widgets using Outline.

Testing

Manual testing

return;
}

for entity in q_indicators.iter() {
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.

Might be simpler if you just find the current InputFocus entity and set outlines on it and all its descendants with FocusIndicator and record those focus ringed nodes. Then afterwards, iterate through all the FocusIndicator nodes With<Outline> and remove all Outlines except those already visited.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Would that be faster? I'm concerned about the cost of allocating memory (although we know how many items there are so we could reserve the collection up front).

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.

Well the number of focus ringed entities wouldn't normally be very large, it could just use a Local smallvec or something with 5 capacity or so.

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.

It would be much faster I think otherwise, as you wouldn't need to walk back up the tree each time. And you would only visit nodes and queue a remove command for those that already have an outline.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-UI Graphical user interfaces, styles, layouts, and widgets X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 3, 2026
@github-project-automation github-project-automation bot moved this to Needs SME Triage in UI Apr 3, 2026
@alice-i-cecile alice-i-cecile requested a review from kfc35 April 3, 2026 00:31
Copy link
Copy Markdown
Contributor

@ickshonpe ickshonpe left a comment

Choose a reason for hiding this comment

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

I definitely think manage_focus_indicators can be restructured to be more efficient but it's not that vital, everything else looks like it works now.

viridia and others added 2 commits April 2, 2026 17:58
Co-authored-by: ickshonpe <david.curthoys@googlemail.com>
@kfc35 kfc35 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 3, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 3, 2026
Merged via the queue into bevyengine:main with commit f13d3f9 Apr 3, 2026
40 checks passed
@github-project-automation github-project-automation bot moved this from Needs SME Triage to Done in UI Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants