Skip to content

Adding interaction states for headless widgets. (ui only) #19349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

viridia
Copy link
Contributor

@viridia viridia commented May 23, 2025

Part of #19236

This PR contains struct definitions for the low-level interaction states used by the headless widgets.

Testing

Tests will be in subsequent PRs. This PR contains only the basic data structures and hooks.

@viridia
Copy link
Contributor Author

viridia commented May 23, 2025

@alice-i-cecile @cart This PR contains only the bevy_ui changes, not the changes to bevy_picking which are still in the old PR.

@viridia
Copy link
Contributor Author

viridia commented May 23, 2025

Also please note the release notes are incomplete, and will be revised in follow-on PRs.

@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 M-Needs-Release-Note Work that should be called out in the blog due to impact S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 26, 2025
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Can you explain why you think we should use immutable components for the accessibility updates, rather than ordinary systems?

@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label May 26, 2025
@viridia
Copy link
Contributor Author

viridia commented May 26, 2025

Can you explain why you think we should use immutable components for the accessibility updates, rather than ordinary systems?

I don't feel strongly about this, but my reasoning goes something like this:

  • Reducing latency is good.
  • Component hooks have very low latency. Using ordinary systems may entail a higher latency depending on what order they are run in the schedule. We can't necessarily predict where in the schedule the user will decide to mutate checked and disabled states, so we don't know where in the schedule is the optimal place to sync the accessibility updates.
  • Using component hooks implies immutable components, because of the difficulty of detecting mutations.

All that being said, there are some advantages to using mutable components here: it makes the observers for input events simpler, since we can just mutate the states directly and don't have to resort to commands.

It's a trade-off, and I am not expert enough to know which option is better.

@alice-i-cecile
Copy link
Member

Hmm. This departs from our designs elsewhere, but it's also a new set of tools. I really like being able to quickly respond to "a button was pressed" using the immutable components trick without having to iterate the whole set, and I would also very much like to avoid adding more callsites for RemovedComponents.

I'm alright with this design, but it should be swapped to use global observers added in the UiPlugin, not hooks. Hooks are intended for "true invariants" and cannot be overridden or disabled, and will be harder to configure and introspect in the future.

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 M-Needs-Release-Note Work that should be called out in the blog due to impact S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Controversial There is active debate or serious implications around merging this PR
Projects
Status: Widget-ready
Development

Successfully merging this pull request may close these issues.

3 participants