Skip to content

Commit a7345bc

Browse files
PPakalnsalice-i-cecile
authored andcommitted
Make Radio Button behaviour modular and consistent with other widgets (bevyengine#21294)
# Objective Fixes bevyengine#21261 ## Solution Changes: - Detect events directly on radio buttons, - Make RadioGroup optional, - ValueChange events are triggered on checked radio button and RadioGroup. This makes radio button behavior: - similar to other widgets, where we can observe triggered change directly on widget, - radio button widget can function separately, - modular, users can decide if they want to use RadioGroup or want to roll out their own solution. Current behavior in examples doesn't change with this PR. ## Testing Tested using existing examples. See `feathers` example, behavior doesn't change. Additionally, tested in bevy_immediate where widget consistency is useful. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
1 parent 787f7ad commit a7345bc

File tree

2 files changed

+109
-59
lines changed

2 files changed

+109
-59
lines changed

crates/bevy_ui_widgets/src/radio.rs

Lines changed: 89 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,18 @@ use crate::ValueChange;
3939
#[require(AccessibilityNode(accesskit::Node::new(Role::RadioGroup)))]
4040
pub struct RadioGroup;
4141

42-
/// Headless widget implementation for radio buttons. These should be enclosed within a
43-
/// [`RadioGroup`] widget, which is responsible for the mutual exclusion logic.
42+
/// Headless widget implementation for radio buttons. They can be used independently,
43+
/// but enclosing them in a [`RadioGroup`] widget allows them to behave as a single,
44+
/// mutually exclusive unit.
4445
///
4546
/// According to the WAI-ARIA best practices document, radio buttons should not be focusable,
4647
/// but rather the enclosing group should be focusable.
4748
/// See <https://www.w3.org/WAI/ARIA/apg/patterns/radio>/
49+
///
50+
/// The widget emits a [`ValueChange<bool>`] event with the value `true` whenever it becomes checked,
51+
/// either through a mouse click or when a [`RadioGroup`] checks the widget.
52+
/// If the [`RadioButton`] is focusable, it can also be checked using the `Enter` or `Space` keys,
53+
/// in which case the event will likewise be emitted.
4854
#[derive(Component, Debug)]
4955
#[require(AccessibilityNode(accesskit::Node::new(Role::RadioButton)), Checkable)]
5056
#[derive(Reflect)]
@@ -132,7 +138,12 @@ fn radio_group_on_key_input(
132138

133139
let (next_id, _) = radio_buttons[next_index];
134140

135-
// Trigger the on_change event for the newly checked radio button
141+
// Trigger the value change event on the radio button
142+
commands.trigger(ValueChange::<bool> {
143+
source: next_id,
144+
value: true,
145+
});
146+
// Trigger the on_change event for the newly checked radio button on radio group
136147
commands.trigger(ValueChange::<Entity> {
137148
source: ev.focused_entity,
138149
value: next_id,
@@ -141,82 +152,101 @@ fn radio_group_on_key_input(
141152
}
142153
}
143154

144-
fn radio_group_on_button_click(
145-
mut ev: On<Pointer<Click>>,
155+
// Provides functionality for standalone focusable [`RadioButton`] to react
156+
// on `Space` or `Enter` key press.
157+
fn radio_button_on_key_input(
158+
mut ev: On<FocusedInput<KeyboardInput>>,
159+
q_radio_button: Query<(Has<InteractionDisabled>, Has<Checked>), With<RadioButton>>,
146160
q_group: Query<(), With<RadioGroup>>,
147-
q_radio: Query<(Has<Checked>, Has<InteractionDisabled>), With<RadioButton>>,
148161
q_parents: Query<&ChildOf>,
149-
q_children: Query<&Children>,
150162
mut commands: Commands,
151163
) {
152-
if q_group.contains(ev.entity) {
153-
// Starting with the original target, search upward for a radio button.
154-
let radio_id = if q_radio.contains(ev.original_event_target()) {
155-
ev.original_event_target()
156-
} else {
157-
// Search ancestors for the first radio button
158-
let mut found_radio = None;
159-
for ancestor in q_parents.iter_ancestors(ev.original_event_target()) {
160-
if q_group.contains(ancestor) {
161-
// We reached a radio group before finding a radio button, bail out
162-
return;
163-
}
164-
if q_radio.contains(ancestor) {
165-
found_radio = Some(ancestor);
166-
break;
167-
}
168-
}
169-
170-
match found_radio {
171-
Some(radio) => radio,
172-
None => return, // No radio button found in the ancestor chain
173-
}
174-
};
164+
let Ok((disabled, checked)) = q_radio_button.get(ev.focused_entity) else {
165+
// Not a radio button
166+
return;
167+
};
168+
169+
let event = &ev.event().input;
170+
if event.state == ButtonState::Pressed
171+
&& !event.repeat
172+
&& (event.key_code == KeyCode::Enter || event.key_code == KeyCode::Space)
173+
{
174+
ev.propagate(false);
175175

176-
// Radio button is disabled.
177-
if q_radio.get(radio_id).unwrap().1 {
176+
// Radio button is disabled or already checked
177+
if disabled || checked {
178178
return;
179179
}
180180

181-
// Gather all the enabled radio group descendants for exclusion.
182-
let radio_buttons = q_children
183-
.iter_descendants(ev.entity)
184-
.filter_map(|child_id| match q_radio.get(child_id) {
185-
Ok((checked, false)) => Some((child_id, checked)),
186-
Ok((_, true)) | Err(_) => None,
187-
})
188-
.collect::<Vec<_>>();
189-
190-
if radio_buttons.is_empty() {
191-
return; // No enabled radio buttons in the group
192-
}
181+
trigger_radio_button_and_radio_group_value_change(
182+
ev.focused_entity,
183+
&q_group,
184+
&q_parents,
185+
&mut commands,
186+
);
187+
}
188+
}
193189

194-
// Pick out the radio button that is currently checked.
195-
ev.propagate(false);
196-
let current_radio = radio_buttons
197-
.iter()
198-
.find(|(_, checked)| *checked)
199-
.map(|(id, _)| *id);
190+
fn radio_button_on_click(
191+
mut ev: On<Pointer<Click>>,
192+
q_group: Query<(), With<RadioGroup>>,
193+
q_radio: Query<(Has<InteractionDisabled>, Has<Checked>), With<RadioButton>>,
194+
q_parents: Query<&ChildOf>,
195+
mut commands: Commands,
196+
) {
197+
let Ok((disabled, checked)) = q_radio.get(ev.entity) else {
198+
// Not a radio button
199+
return;
200+
};
200201

201-
if current_radio == Some(radio_id) {
202-
// If they clicked the currently checked radio button, do nothing
203-
return;
204-
}
202+
ev.propagate(false);
203+
204+
// Radio button is disabled or already checked
205+
if disabled || checked {
206+
return;
207+
}
205208

206-
// Trigger the on_change event for the newly checked radio button
209+
trigger_radio_button_and_radio_group_value_change(
210+
ev.entity,
211+
&q_group,
212+
&q_parents,
213+
&mut commands,
214+
);
215+
}
216+
217+
fn trigger_radio_button_and_radio_group_value_change(
218+
radio_button: Entity,
219+
q_group: &Query<(), With<RadioGroup>>,
220+
q_parents: &Query<&ChildOf>,
221+
commands: &mut Commands,
222+
) {
223+
commands.trigger(ValueChange::<bool> {
224+
source: radio_button,
225+
value: true,
226+
});
227+
228+
// Find if radio button is inside radio group
229+
let radio_group = q_parents
230+
.iter_ancestors(radio_button)
231+
.find(|ancestor| q_group.contains(*ancestor));
232+
233+
// If is inside radio group
234+
if let Some(radio_group) = radio_group {
235+
// Trigger event for radio group
207236
commands.trigger(ValueChange::<Entity> {
208-
source: ev.entity,
209-
value: radio_id,
237+
source: radio_group,
238+
value: radio_button,
210239
});
211240
}
212241
}
213242

214-
/// Plugin that adds the observers for the [`RadioGroup`] widget.
243+
/// Plugin that adds the observers for [`RadioButton`] and [`RadioGroup`] widget.
215244
pub struct RadioGroupPlugin;
216245

217246
impl Plugin for RadioGroupPlugin {
218247
fn build(&self, app: &mut App) {
219248
app.add_observer(radio_group_on_key_input)
220-
.add_observer(radio_group_on_button_click);
249+
.add_observer(radio_button_on_click)
250+
.add_observer(radio_button_on_key_input);
221251
}
222252
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
title: "`RadioButton`, `RadioGroup` widget minor improvements"
3+
authors: ["@PPakalns"]
4+
pull_requests: [21294]
5+
---
6+
7+
`RadioButton` and `RadioGroup` usage remains fully backward compatible.
8+
9+
Improvements:
10+
11+
- Event propagation from user interactions will now be canceled even if
12+
widgets are disabled. Previously, some relevant event propagation
13+
was not properly canceled.
14+
- `RadioButton` now emits a `ValueChange<bool>` entity event when checked,
15+
even when checked via a `RadioGroup`. Consistent with other `Checkable` widgets.
16+
As a `RadioButton` cannot be unchecked through direct user interaction with this widget,
17+
a `ValueChange` event with value `false` can not be triggered for `RadioButton`.
18+
- If a `RadioButton` is focusable, a value change event can be triggered
19+
using the **Space** or **Enter** keys when focused.
20+
- `RadioGroup` is now optional and can be replaced with a custom implementation.

0 commit comments

Comments
 (0)