New simplified "click to focus" logic for core widgets. #19736
Conversation
Click to focus is now a global observer.
alice-i-cecile
added
C-Bug
There was a problem hiding this comment.
I like this design, but two non-blocking comments:
- I think we should roll all of those supporting plugins into
DefaultPlugins. Perhaps not for 0.17, but once it's ready. Users who don't want this will want to disable this code via features anyways, and can also opt-out by disabling plugins from there. - We should consider making this the single, blessed path to set input focus. I don't see a clear argument for continuing to support other methods, and it's less confusing and buggy to only have one path.
One thing to note is that "click to focus" currently only works with If we wanted to make this universal, we would need to have a |
|
That being said, I suppose we could merge the |
|
Also, another open question: should I use |
Yeah, I like that idea. |
| @@ -207,43 +206,18 @@ pub(crate) fn slider_on_pointer_down( | |||
| )>, | |||
| q_thumb: Query<&ComputedNode, With<CoreSliderThumb>>, | |||
| q_children: Query<&Children>, | |||
| q_parents: Query<&ChildOf>, | |||
| focus: Option<ResMut<InputFocus>>, | |||
| focus_visible: Option<ResMut<InputFocusVisible>>, | |||
| mut commands: Commands, | |||
| ui_scale: Res<UiScale>, | |||
| ) { | |||
| if q_thumb.contains(trigger.target()) { | |||
There was a problem hiding this comment.
Is there no mechanism to block click propagation without an observer?
| focus.0 = Some(ev.target()); | ||
| } | ||
| } else if windows.contains(ev.target()) { | ||
| // Stop and focus it |
There was a problem hiding this comment.
| // Stop and focus it | |
| // Stop and remove current focus |
| ev.propagate(false); | ||
| // Don't mutate unless we need to, for change detection | ||
| if focus.0.is_some() { | ||
| focus.0 = None; |
There was a problem hiding this comment.
| focus.0 = None; | |
| focus.clear(); |
| if let Ok(window) = windows.single() { | ||
| commands | ||
| .entity(ev.target()) | ||
| .trigger(AcquireFocus { window }); | ||
| } |
There was a problem hiding this comment.
I'm confused by why the PrimaryWindow is returned if a focusable target isn't found, what if the clicked target isn't on the primary window?
Click to focus is now a global observer.
Objective
Previously, the "click to focus" behavior was implemented in each individual headless widget, producing redundant logic.
Solution
The new scheme is to have a global observer which looks for pointer down events and triggers an
AcquireFocusevent on the target. This event bubbles until it finds an entity withTabIndex, and then focuses it.Testing
Tested the changes using the various examples that have focusable widgets. (This will become easier to test when I add focus ring support to the examples, but that's for another day. For now you just have to know which keys to press.)
Migration
This change is backwards-compatible. People who want the new behavior will need to install the new plugin.