Skip to content
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

Add UI keyboard navigation #8882

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -1815,6 +1815,17 @@ description = "Illustrates creating and updating a button"
category = "UI (User Interface)"
wasm = true


[[example]]
name = "keyboard_navigation"
path = "examples/ui/keyboard_navigation.rs"

[package.metadata.example.keyboard_navigation]
name = "Keyboard Navigation"
description = "A simple UI containing several buttons that modify a counter, to demonstrate keyboard navigation"
category = "UI (User Interface)"
wasm = true

[[example]]
name = "display_and_visibility"
path = "examples/ui/display_and_visibility.rs"
Expand Down
10 changes: 8 additions & 2 deletions crates/bevy_a11y/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,14 @@ impl AccessKitEntityExt for Entity {
}

/// Resource representing which entity has keyboard focus, if any.
#[derive(Resource, Default, Deref, DerefMut)]
pub struct Focus(Option<Entity>);
#[derive(PartialEq, Eq, Debug, Resource, Default)]
pub struct Focus {
/// The entity that has keyboard focus.
pub entity: Option<Entity>,
/// Focus has been reached through keyboard navigation and so a focus style should be displayed.
/// This is similar to the `:focus-visible` pseudo-class in css.
pub focus_visible: bool,
}

/// Plugin managing non-GUI aspects of integrating with accessibility APIs.
pub struct AccessibilityPlugin;
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_render/src/view/visibility/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ impl ComputedVisibility {
pub const HIDDEN: Self = ComputedVisibility {
flags: ComputedVisibilityFlags::empty(),
};
/// A [`ComputedVisibility`], set as visible.
pub const VISIBLE: Self = ComputedVisibility {
flags: ComputedVisibilityFlags::all(),
};

/// Whether this entity is visible to something this frame. This is true if and only if [`Self::is_visible_in_hierarchy`] and [`Self::is_visible_in_view`]
/// are true. This is the canonical method to call to determine if an entity should be drawn.
Expand Down
12 changes: 11 additions & 1 deletion crates/bevy_ui/src/focus.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use crate::{camera_config::UiCameraConfig, CalculatedClip, Node, UiScale, UiStack};
use bevy_a11y::Focus;
use bevy_derive::{Deref, DerefMut};
use bevy_ecs::{
change_detection::DetectChangesMut,
entity::Entity,
prelude::{Component, With},
query::WorldQuery,
reflect::ReflectComponent,
system::{Local, Query, Res},
system::{Local, Query, Res, ResMut},
};
use bevy_input::{mouse::MouseButton, touch::Touches, Input};
use bevy_math::Vec2;
Expand Down Expand Up @@ -142,6 +143,7 @@ pub fn ui_focus_system(
ui_stack: Res<UiStack>,
mut node_query: Query<NodeQuery>,
primary_window: Query<Entity, With<PrimaryWindow>>,
mut focus: ResMut<Focus>,
) {
let primary_window = primary_window.iter().next();

Expand Down Expand Up @@ -267,9 +269,17 @@ pub fn ui_focus_system(
while let Some(node) = iter.fetch_next() {
if let Some(mut interaction) = node.interaction {
if mouse_clicked {
if focus.entity != Some(node.entity) {
*focus = Focus {
entity: Some(node.entity),
focus_visible: false,
};
}

// only consider nodes with Interaction "pressed"
if *interaction != Interaction::Pressed {
*interaction = Interaction::Pressed;

// if the mouse was simultaneously released, reset this Interaction in the next
// frame
if mouse_released {
Expand Down
264 changes: 264 additions & 0 deletions crates/bevy_ui/src/keyboard_navigation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
use crate::{Interaction, UiStack};
use bevy_a11y::Focus;
use bevy_ecs::prelude::Component;
use bevy_ecs::query::With;
use bevy_ecs::system::{Local, Query, Res, ResMut, Resource};
use bevy_ecs::{change_detection::DetectChangesMut, entity::Entity};
use bevy_input::{prelude::KeyCode, Input};
use bevy_reflect::Reflect;
use bevy_render::view::ComputedVisibility;

/// A component that represents if a UI element is focused.
#[derive(Reflect, Component, Clone, Debug, Default, Eq, PartialEq)]
pub struct Focusable {
focus_state: FocusState,
}

impl Focusable {
/// The entity is currently focused, similar to the `:focus` css pseudo-class.
/// To check if the focus has been achieved through keyboard navigation, see [`Focusable::is_focus_visible`].
pub fn is_focused(&self) -> bool {
matches!(self.focus_state, FocusState::Focused { .. })
}

/// Focus has been reached through keyboard navigation and so a focus style should be displayed.
/// This is similar to the `:focus-visible` pseudo-class in css.
pub fn is_focus_visible(&self) -> bool {
matches!(self.focus_state, FocusState::Focused { visible: true })
}
}

#[derive(Reflect, Clone, Debug, Default, Eq, PartialEq)]
enum FocusState {
/// Entity is not focused
#[default]
None,
0HyperCube marked this conversation as resolved.
Show resolved Hide resolved
/// Entity is focused
Focused {
/// Focus has been reached through keyboard navigation and so a focus style should be displayed.
/// This is similar to the `:focus-visible` pseudo-class in css.
visible: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why make this a field of FocusState::Focused instead of a field of Focusable?

Copy link
Author

Choose a reason for hiding this comment

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

It doesn't really make sense to have an entity that is not focused, but at the same time has the focus as visible. The visibility of the focus is only applicable when the entity is actually focused.

I'm happy to change this if you prefer, but I don't see any compelling reasons why having this as a field of Focusable would be advantageous.

Copy link
Contributor

Choose a reason for hiding this comment

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

I approved the PR. I'll leave it up to you.

Personally I try to avoid boolean fields generally. It's a bad pattern. It especially makes little sense as enum field. It can be replaced by:

enum FocusState {
  FocusedVisible,
  FocusedInvisible,
  None,

Also I don't think visible is the appropriate term here. Semantically I think it means "last interaction was through a keyboard navigation interaction". Whether it's visible or not is up to the end user. Within bevy code, using "visible" is just plain misleading. I wouldn't hold the web standards as a reference for naming things, it's notoriously bad in this regard (eg: referer header).

Also I still am doubtful this is something we want for bevy. Can you give me the example of a game with tab-based keyboard navigation?

Copy link
Author

Choose a reason for hiding this comment

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

Can you give me the example of a game with tab-based keyboard navigation?

Games with form style interfaces (like username / password input boxes e.g. Veloren) will have at least have some kind of tab navigation.

Copy link
Author

Choose a reason for hiding this comment

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

Also if bevy UI is used to make a level editor, then tab navigation will be necessary for that.

},
}

/// Resource for the configuration of keyboard navigation of UI with <kbd>tab</kbd>.
#[derive(Resource)]
pub struct KeyboardNavigationInput {
pub enabled: bool,
}

impl Default for KeyboardNavigationInput {
fn default() -> Self {
Self { enabled: true }
}
}

/// Should the [`keyboard_navigation_system`] run?
pub(crate) fn tab_pressed(
keyboard_input: Res<Input<KeyCode>>,
keyboard_navigation: Res<KeyboardNavigationInput>,
) -> bool {
keyboard_navigation.enabled && keyboard_input.just_pressed(KeyCode::Tab)
}

/// The system updates the [`Focus`] resource when the user uses keyboard navigation with <kbd>tab</kbd> or <kbd>shift</kbd> + <kbd>tab</kbd>.
///
/// Entities can be focused if [`ComputedVisibility`] is visible and they have the [`Focusable`] component.
pub(crate) fn keyboard_navigation_system(
mut focus: ResMut<Focus>,
mut interactions: Query<&mut Interaction>,
focusables: Query<&ComputedVisibility, With<Focusable>>,
keyboard_input: Res<Input<KeyCode>>,
ui_stack: Res<UiStack>,
) {
let reverse_order =
keyboard_input.pressed(KeyCode::ShiftLeft) || keyboard_input.pressed(KeyCode::ShiftRight);

let can_focus = |entity: &&Entity| {
focusables
.get(**entity)
.map_or(false, |computed_visibility| {
computed_visibility.is_visible()
})
};

let ui_nodes = &ui_stack.uinodes;

// Current index of the focused entity within the ui nodes list.
let current_index = ui_nodes
.iter()
.position(|&ui_node| Some(ui_node) == focus.entity);

let new_focus = if reverse_order {
// Start with the entity before the current focused or at the end of the list
let first_index = current_index.unwrap_or_default();

let before = ui_nodes.iter().take(first_index);
let after = ui_nodes.iter().skip(first_index);
let mut wrapped = before.rev().chain(after.rev());
wrapped.find(can_focus).copied()
} else {
// Start with the entity after the current focused or at the start of the list
let first_index = current_index.map(|index| index + 1).unwrap_or_default();

let after = ui_nodes.iter().skip(first_index);
let before = ui_nodes.iter().take(first_index);
let mut wrapped = after.chain(before);
wrapped.find(can_focus).copied()
};

// Reset the clicked state
if new_focus != focus.entity {
if let Some(mut interaction) = focus
.entity
.and_then(|entity| interactions.get_mut(entity).ok())
{
if *interaction == Interaction::Pressed {
*interaction = Interaction::None;
}
}
}

focus.set_if_neq(Focus {
entity: new_focus,
focus_visible: true,
});
}

/// Change the [`FocusState`] for the specified entity
fn set_focus_state(
entity: Option<Entity>,
focusable: &mut Query<&mut Focusable>,
focus_state: FocusState,
) {
if let Some(mut focusable) = entity.and_then(|entity| focusable.get_mut(entity).ok()) {
focusable.set_if_neq(Focusable { focus_state });
}
}

/// Modify the [`FocusState`] of the [`Focusable`] component, based on the [`Focus`] resource.
pub(crate) fn update_focused_state(
0HyperCube marked this conversation as resolved.
Show resolved Hide resolved
mut focusable: Query<&mut Focusable>,
focus: Res<Focus>,
mut old_focused_entity: Local<Option<Entity>>,
) {
let new_focused_entity = focus.entity;

// Remove the interaction from the last focused entity
if *old_focused_entity != new_focused_entity {
set_focus_state(*old_focused_entity, &mut focusable, FocusState::None);
}

let new_state = FocusState::Focused {
visible: focus.focus_visible,
};
// Set the focused interaction on the newly focused entity
set_focus_state(new_focused_entity, &mut focusable, new_state);

*old_focused_entity = new_focused_entity;
}

/// Should the [`keyboard_click`] system run?
pub(crate) fn trigger_click(keyboard_input: Res<Input<KeyCode>>) -> bool {
keyboard_input.just_pressed(KeyCode::Space) || keyboard_input.just_pressed(KeyCode::Return)
}

/// Trigger the [`Focus`] entity to be clicked.
pub(crate) fn keyboard_click(mut interactions: Query<&mut Interaction>, focus: Res<Focus>) {
if let Some(mut interaction) = focus
.entity
.and_then(|entity| interactions.get_mut(entity).ok())
{
interaction.set_if_neq(Interaction::Pressed);
}
}

/// Should the [`end_keyboard_click`] system run?
pub(crate) fn trigger_click_end(keyboard_input: Res<Input<KeyCode>>) -> bool {
keyboard_input.just_released(KeyCode::Space) || keyboard_input.just_released(KeyCode::Return)
}

/// Reset the clicked state.
pub(crate) fn end_keyboard_click(mut interactions: Query<&mut Interaction>) {
interactions.for_each_mut(|mut interaction| {
if *interaction == Interaction::Pressed {
// The click was triggered by the keyboard, so it doesn't make sense to go to `Interaction::Hovered`.
*interaction = Interaction::None;
}
});
}

#[cfg(test)]
mod test {
use super::*;
use crate::prelude::{ButtonBundle, NodeBundle};
use crate::{ui_stack_system, Focusable};
use bevy_ecs::prelude::*;
use bevy_ecs::system::CommandQueue;
use bevy_hierarchy::BuildChildren;
use bevy_utils::default;

#[test]
fn keyboard_navigation() {
let mut world = World::default();

let mut schedule = Schedule::new();
schedule.add_systems((
ui_stack_system,
keyboard_navigation_system.after(ui_stack_system),
update_focused_state.after(keyboard_navigation_system),
));

let mut queue = CommandQueue::default();
let mut commands = Commands::new(&mut queue, &world);

commands.init_resource::<bevy_a11y::Focus>();
commands.init_resource::<Input<KeyCode>>();
commands.init_resource::<UiStack>();

let mut children = Vec::new();
commands
.spawn(NodeBundle::default())
.with_children(|parent| {
for _child in 0..3 {
children.push(
parent
.spawn(ButtonBundle {
computed_visibility: ComputedVisibility::VISIBLE,
..default()
})
.id(),
);
}
});
queue.apply(&mut world);

schedule.run(&mut world);
assert!(
world.get::<Focusable>(children[0]).unwrap().is_focused(),
"navigation should start at the first button"
);
schedule.run(&mut world);
assert!(
world.get::<Focusable>(children[1]).unwrap().is_focused(),
"navigation should go to the second button"
);

// Simulate pressing shift
let mut keyboard_input = world
.get_resource_mut::<Input<KeyCode>>()
.expect("keyboard input resource");
keyboard_input.press(KeyCode::ShiftLeft);
0HyperCube marked this conversation as resolved.
Show resolved Hide resolved

schedule.run(&mut world);
assert!(
world.get::<Focusable>(children[0]).unwrap().is_focused(),
"backwards navigation"
);
schedule.run(&mut world);
assert!(
world.get::<Focusable>(children[2]).unwrap().is_focused(),
"navigation should loop around"
);
}
}
Loading
Loading