From 49e705350a8b91b4f31a4329362b3d2c95217848 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Pakalns?= Date: Tue, 30 Sep 2025 12:08:02 +0300 Subject: [PATCH 1/2] Make Radio Button behaviour modular and consistent with other widgets --- crates/bevy_ui_widgets/src/radio.rs | 154 +++++++++++++++++----------- 1 file changed, 92 insertions(+), 62 deletions(-) diff --git a/crates/bevy_ui_widgets/src/radio.rs b/crates/bevy_ui_widgets/src/radio.rs index 1dac539782f54..3cdf9a1809464 100644 --- a/crates/bevy_ui_widgets/src/radio.rs +++ b/crates/bevy_ui_widgets/src/radio.rs @@ -6,7 +6,7 @@ use bevy_ecs::{ entity::Entity, hierarchy::{ChildOf, Children}, observer::On, - query::{Has, With}, + query::{Has, With, Without}, reflect::ReflectComponent, system::{Commands, Query}, }; @@ -39,12 +39,18 @@ use crate::ValueChange; #[require(AccessibilityNode(accesskit::Node::new(Role::RadioGroup)))] pub struct RadioGroup; -/// Headless widget implementation for radio buttons. These should be enclosed within a -/// [`RadioGroup`] widget, which is responsible for the mutual exclusion logic. +/// Headless widget implementation for radio buttons. They can be used independently, +/// but enclosing them in a [`RadioGroup`] widget allows them to behave as a single, +/// mutually exclusive unit. /// /// According to the WAI-ARIA best practices document, radio buttons should not be focusable, /// but rather the enclosing group should be focusable. /// See / +/// +/// The widget emits a [`ValueChange`] event with the value `true` whenever it becomes checked, +/// either through a mouse click or when a [`RadioGroup`] checks the widget. +/// If the [`RadioButton`] is focusable, it can also be checked using the `Enter` or `Space` keys, +/// in which case the event will likewise be emitted. #[derive(Component, Debug)] #[require(AccessibilityNode(accesskit::Node::new(Role::RadioButton)), Checkable)] #[derive(Reflect)] @@ -132,7 +138,12 @@ fn radio_group_on_key_input( let (next_id, _) = radio_buttons[next_index]; - // Trigger the on_change event for the newly checked radio button + // Trigger the value change event on the radio button + commands.trigger(ValueChange:: { + source: next_id, + value: true, + }); + // Trigger the on_change event for the newly checked radio button on radio group commands.trigger(ValueChange:: { source: ev.focused_entity, value: next_id, @@ -141,82 +152,101 @@ fn radio_group_on_key_input( } } -fn radio_group_on_button_click( - mut ev: On>, +// Provides functionality for standalone focusable [`RadioButton`] to react +// on `Space` or `Enter` key press. +fn radio_button_on_key_input( + mut ev: On>, + q_radio_button: Query, (With, Without)>, q_group: Query<(), With>, - q_radio: Query<(Has, Has), With>, q_parents: Query<&ChildOf>, - q_children: Query<&Children>, mut commands: Commands, ) { - if q_group.contains(ev.entity) { - // Starting with the original target, search upward for a radio button. - let radio_id = if q_radio.contains(ev.original_event_target()) { - ev.original_event_target() - } else { - // Search ancestors for the first radio button - let mut found_radio = None; - for ancestor in q_parents.iter_ancestors(ev.original_event_target()) { - if q_group.contains(ancestor) { - // We reached a radio group before finding a radio button, bail out - return; - } - if q_radio.contains(ancestor) { - found_radio = Some(ancestor); - break; - } - } + let Ok(checked) = q_radio_button.get(ev.focused_entity) else { + // Not a radio button + return; + }; + + // Radio button already checked + if checked { + return; + } - match found_radio { - Some(radio) => radio, - None => return, // No radio button found in the ancestor chain - } - }; + let event = &ev.event().input; + if event.state == ButtonState::Pressed + && !event.repeat + && (event.key_code == KeyCode::Enter || event.key_code == KeyCode::Space) + { + ev.propagate(false); - // Radio button is disabled. - if q_radio.get(radio_id).unwrap().1 { - return; - } + trigger_radio_button_and_radio_group_value_change( + ev.focused_entity, + &q_group, + &q_parents, + &mut commands, + ); + } +} - // Gather all the enabled radio group descendants for exclusion. - let radio_buttons = q_children - .iter_descendants(ev.entity) - .filter_map(|child_id| match q_radio.get(child_id) { - Ok((checked, false)) => Some((child_id, checked)), - Ok((_, true)) | Err(_) => None, - }) - .collect::>(); - - if radio_buttons.is_empty() { - return; // No enabled radio buttons in the group - } +fn radio_button_on_click( + mut ev: On>, + q_group: Query<(), With>, + q_radio: Query, (With, Without)>, + q_parents: Query<&ChildOf>, + mut commands: Commands, +) { + let Ok(checked) = q_radio.get(ev.entity) else { + // Not a radio button + return; + }; - // Pick out the radio button that is currently checked. - ev.propagate(false); - let current_radio = radio_buttons - .iter() - .find(|(_, checked)| *checked) - .map(|(id, _)| *id); - - if current_radio == Some(radio_id) { - // If they clicked the currently checked radio button, do nothing - return; - } + ev.propagate(false); + + // Radio button is already checked + if checked { + return; + } - // Trigger the on_change event for the newly checked radio button + trigger_radio_button_and_radio_group_value_change( + ev.entity, + &q_group, + &q_parents, + &mut commands, + ); +} + +fn trigger_radio_button_and_radio_group_value_change( + radio_button: Entity, + q_group: &Query<(), With>, + q_parents: &Query<&ChildOf>, + commands: &mut Commands, +) { + commands.trigger(ValueChange:: { + source: radio_button, + value: true, + }); + + // Find if radio button is inside radio group + let radio_group = q_parents + .iter_ancestors(radio_button) + .find(|ancestor| q_group.contains(*ancestor)); + + // If is inside radio group + if let Some(radio_group) = radio_group { + // Trigger event for radio group commands.trigger(ValueChange:: { - source: ev.entity, - value: radio_id, + source: radio_group, + value: radio_button, }); } } -/// Plugin that adds the observers for the [`RadioGroup`] widget. +/// Plugin that adds the observers for [`RadioButton`] and [`RadioGroup`] widget. pub struct RadioGroupPlugin; impl Plugin for RadioGroupPlugin { fn build(&self, app: &mut App) { app.add_observer(radio_group_on_key_input) - .add_observer(radio_group_on_button_click); + .add_observer(radio_button_on_click) + .add_observer(radio_button_on_key_input); } } From 98064683c0d8412469462d73b0014d8695e304ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Pakalns?= Date: Thu, 2 Oct 2025 15:50:08 +0300 Subject: [PATCH 2/2] Fix bug with event propagation cancellation --- crates/bevy_ui_widgets/src/radio.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ui_widgets/src/radio.rs b/crates/bevy_ui_widgets/src/radio.rs index 3cdf9a1809464..ec625bc0a74fa 100644 --- a/crates/bevy_ui_widgets/src/radio.rs +++ b/crates/bevy_ui_widgets/src/radio.rs @@ -6,7 +6,7 @@ use bevy_ecs::{ entity::Entity, hierarchy::{ChildOf, Children}, observer::On, - query::{Has, With, Without}, + query::{Has, With}, reflect::ReflectComponent, system::{Commands, Query}, }; @@ -156,21 +156,16 @@ fn radio_group_on_key_input( // on `Space` or `Enter` key press. fn radio_button_on_key_input( mut ev: On>, - q_radio_button: Query, (With, Without)>, + q_radio_button: Query<(Has, Has), With>, q_group: Query<(), With>, q_parents: Query<&ChildOf>, mut commands: Commands, ) { - let Ok(checked) = q_radio_button.get(ev.focused_entity) else { + let Ok((disabled, checked)) = q_radio_button.get(ev.focused_entity) else { // Not a radio button return; }; - // Radio button already checked - if checked { - return; - } - let event = &ev.event().input; if event.state == ButtonState::Pressed && !event.repeat @@ -178,6 +173,11 @@ fn radio_button_on_key_input( { ev.propagate(false); + // Radio button is disabled or already checked + if disabled || checked { + return; + } + trigger_radio_button_and_radio_group_value_change( ev.focused_entity, &q_group, @@ -190,19 +190,19 @@ fn radio_button_on_key_input( fn radio_button_on_click( mut ev: On>, q_group: Query<(), With>, - q_radio: Query, (With, Without)>, + q_radio: Query<(Has, Has), With>, q_parents: Query<&ChildOf>, mut commands: Commands, ) { - let Ok(checked) = q_radio.get(ev.entity) else { + let Ok((disabled, checked)) = q_radio.get(ev.entity) else { // Not a radio button return; }; ev.propagate(false); - // Radio button is already checked - if checked { + // Radio button is disabled or already checked + if disabled || checked { return; }