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

[Merged by Bors] - Integrate AccessKit #6874

Closed
wants to merge 66 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
0e07c28
Integrate AccessKit.
ndarilek Dec 6, 2022
8b67099
Add support for non-container UI widgets.
ndarilek Nov 15, 2022
269293b
Sync UI focus with AccessKit.
ndarilek Nov 15, 2022
c78a81d
Print out all `ActionRequest` events.
ndarilek Nov 15, 2022
07c6ec8
Clean up focus-handling and attempt to work around more corner cases.…
ndarilek Nov 16, 2022
8503824
Add non-functional example for adding custom `AccessibilityNode`s to …
ndarilek Dec 6, 2022
6c8bc71
Appease Clippy.
ndarilek Dec 6, 2022
3465deb
WIP: use `Mutex` rather than channels for pulling actions.
ndarilek Dec 6, 2022
26444b5
Merge remote-tracking branch 'origin' into accesskit
ndarilek Dec 12, 2022
9a9cf83
Introduce `bevy_a11y` crate.
ndarilek Dec 13, 2022
7d7ac22
Begin decoupling bevy_ui from bevy_winit.
ndarilek Dec 13, 2022
1cf5730
Decouple bevy_ui from bevy_winit and continue integrating bevy_a11y.
ndarilek Dec 13, 2022
0fc80fc
Give structs less generic names.
ndarilek Dec 13, 2022
ebaac9b
Lazily calculate AccessKit updates only when necessary.
ndarilek Dec 14, 2022
058264b
Only run node update logic if nodes/focus changes.
ndarilek Dec 14, 2022
efd9d80
Remove unnecessary logging.
ndarilek Dec 14, 2022
b86a2c3
Perform `u128` conversion first.
ndarilek Dec 14, 2022
b52e587
Remove code simulating keyboard focus-handling until bevy_ui explicit…
ndarilek Dec 14, 2022
06fe868
Remove unnecessary resource initialization.
ndarilek Dec 14, 2022
c503915
Only set focus on tree updates if the window itself has focus.
ndarilek Dec 14, 2022
79bebfa
Use 2 rather than 1 for entity-to-node ID conversions since ID 1 is r…
ndarilek Dec 14, 2022
f84cbf0
Consistently set name of root window.
ndarilek Dec 14, 2022
6ec2a87
Merge remote-tracking branch 'origin' into accesskit
ndarilek Jan 23, 2023
647a408
Merge branch 'main' into accesskit
ndarilek Jan 23, 2023
2f09ebb
Revert accidental formatting change.
ndarilek Jan 23, 2023
3aa8c8e
Bump accesskit_winit dependency.
ndarilek Jan 23, 2023
43db5ac
Restore accidentally-removed code.
ndarilek Jan 23, 2023
f466741
Merge branch 'main' into accesskit
ndarilek Jan 25, 2023
f276404
s/`ResMut`/`NonSendMut`/
ndarilek Jan 25, 2023
791fbe4
Make `WinitActionHandlers` a regular resource.
ndarilek Jan 25, 2023
aac5116
Remove unused function.
ndarilek Jan 25, 2023
9fee1ee
Merge branch 'main' into accesskit
ndarilek Jan 30, 2023
05b03d3
Merge remote-tracking branch 'origin' into accesskit
ndarilek Feb 2, 2023
06714ea
Add crate metadata and documentation.
ndarilek Feb 2, 2023
7894849
Update accessibility nodes on bounds changes.
ndarilek Feb 2, 2023
eafe226
Mirror hierarchies to accessibility tree where possible.
ndarilek Feb 2, 2023
2677ae4
Move `AccessibilityNode` bounds calculation into separate system.
ndarilek Feb 2, 2023
3b75e5e
Add more module-level docs.
ndarilek Feb 3, 2023
69166c6
Add `AccessibilityRequested` resource to track whether accessibility …
ndarilek Feb 3, 2023
1571633
Merge branch 'main' into accesskit
ndarilek Feb 3, 2023
f00c893
Remove unnecessary system since tree updates are no longer pushed on …
ndarilek Feb 3, 2023
6ed0c14
Only generate `TreeUpdate`s if accessibility was requested.
ndarilek Feb 3, 2023
f67de8e
Add comment explaining the use of `Label`.
ndarilek Feb 3, 2023
ca99b51
Use `Option`/branching to minimize performance hit of UI updates.
ndarilek Feb 3, 2023
eb6a30d
Merge branch 'main' into accesskit
ndarilek Feb 3, 2023
3b25d00
Merge branch 'main' into accesskit
ndarilek Feb 6, 2023
7ddfea0
Update to newer AccessKit.
ndarilek Feb 6, 2023
0ac0051
Merge branch 'main' into accesskit
ndarilek Feb 13, 2023
f9f251d
Bump dependencies.
ndarilek Feb 13, 2023
293b240
Update crates/bevy_winit/src/lib.rs
ndarilek Feb 14, 2023
7771ddc
Fix broken link in documentation.
ndarilek Feb 15, 2023
2b6a114
Fix CI errors.
ndarilek Feb 15, 2023
c6e36c1
Fix more conflicts.
ndarilek Feb 15, 2023
a375b56
fix wasm build
mockersf Feb 15, 2023
c4e95bc
Update crates/bevy_a11y/Cargo.toml
ndarilek Feb 15, 2023
2917a86
Add bevy_a11y to publishing script.
ndarilek Feb 15, 2023
6ea06b6
Merge pull request #1 from mockersf/fix-wasm-build
ndarilek Feb 15, 2023
03a89e3
Best-effort attempt at correctly calculating accessibility bounding b…
ndarilek Feb 16, 2023
fed306d
Update crates/bevy_a11y/src/lib.rs
ndarilek Feb 20, 2023
47e8d0e
Reduce nesting.
ndarilek Feb 20, 2023
d7a1011
Feature-gate Unix AccessKit adapter for now.
ndarilek Feb 20, 2023
0349cd7
Merge branch 'main' into accesskit
ndarilek Feb 20, 2023
d9a9adc
Merge branch 'main' into accesskit
ndarilek Feb 20, 2023
5841343
Merge branch 'main' into accesskit
ndarilek Feb 27, 2023
8d43a23
Merge remote-tracking branch 'origin/main' into pr/ndarilek/6874
cart Mar 1, 2023
cc2ed5c
Update cargo features
cart Mar 1, 2023
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
1 change: 1 addition & 0 deletions crates/bevy_ui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ bevy_sprite = { path = "../bevy_sprite", version = "0.9.0" }
bevy_text = { path = "../bevy_text", version = "0.9.0" }
bevy_transform = { path = "../bevy_transform", version = "0.9.0" }
bevy_window = { path = "../bevy_window", version = "0.9.0" }
bevy_winit = { path = "../bevy_winit", version = "0.9.0" }
ndarilek marked this conversation as resolved.
Show resolved Hide resolved
bevy_utils = { path = "../bevy_utils", version = "0.9.0" }

# other
Expand Down
219 changes: 219 additions & 0 deletions crates/bevy_ui/src/accessibility.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
use bevy_app::{App, CoreStage, Plugin};
use bevy_derive::{Deref, DerefMut};
use bevy_ecs::{
prelude::{Entity, EventReader},
query::{Changed, Without},
system::{Commands, NonSend, Query, RemovedComponents, ResMut, Resource},
};
use bevy_hierarchy::Children;
use bevy_render::{camera::RenderTarget, prelude::Camera};
use bevy_text::Text;
use bevy_transform::prelude::GlobalTransform;
use bevy_utils::{default, HashMap};
use bevy_window::Windows;
use bevy_winit::{
accessibility::{AccessKitEntityExt, AccessibilityNode, Adapters},
accesskit::{
kurbo::Rect, Action, ActionRequest, DefaultActionVerb, Node as AccessKitNode, Role,
TreeUpdate,
},
};

use crate::{
prelude::{Button, Label, UiCameraConfig},
Interaction, Node, UiImage,
};

fn calc_name(texts: &Query<&Text>, children: &Children) -> Option<Box<str>> {
Copy link
Member

Choose a reason for hiding this comment

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

It's not clear to me that we should be reading the text here, rather than using the Name component (and encouraging people to write to it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree in principle, but I'd like to see these sorts of ideas pushed forward in a general UI overhaul. Put in terms more familiar to those of us who work with accessibility, it's better when designed in from the start vs. bolted on later.

To be clear, I'm not critiquing bevy_ui for not being accessible from the start, particularly since bevy_ui is lacking quite a few things and accessibility is in good company. :) I'd just like to see us address these sorts of issues as part of a greater UI refactor of which accessibility is just one part. Without this groundwork, we wouldn't necessarily be able to meaningfully include accessibility as a design consideration when bevy_ui eventually gets more attention.

Copy link
Member

Choose a reason for hiding this comment

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

Just curious: Is there a reason this is used for set_name and not set_description or something similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mainly because I didn't want to take on fixing bevy_ui's shortcomings in this PR. I needed something to test with, and we don't currently have a way of describing UI widgets in any form. So I calculated a name/title and left it there.

Copy link
Member

Choose a reason for hiding this comment

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

I see. And so the shortcoming you're talking about is that bevy_ui doesn't enforce Name components to extract the a11y name from?

If it did, would this function instead be used to extract the node's description?

Copy link
Contributor Author

@ndarilek ndarilek Feb 20, 2023

Choose a reason for hiding this comment

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

It's more that bevy_ui will eventually need something like Flutter's Semantics class to truly be accessible. Ideally, in that situation, we'll have clearer understandings of how and when we're able to auto-generate names/descriptions, the Semantics component will be nicely integrated with the editor so accessibility properties can be set directly, etc. And I'm trying to take a lighter touch with all those things right now because I don't know what the right answer is.

Also, names and descriptions are very different things, and you don't necessarily want to scattershot the same property value everywhere so e.g. you don't double-announce a name because a screen reader might read a widget like "<name> <description>: button" or similar. Which properties to use and when is fairly complex and differs per widget, but in general setting names is universal and fairly low-hanging fruit. Something more complex I likely wouldn't auto-generate unless the widget semantics clearly call for it.

Ideally, we'll use AccessKit's NodeBuilder or a supported subset of its schema as the core of our Semantics component, and come to consensus about how we automatically fill in properties vs. expecting developers to fill them in themselves.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, thanks for the explanation!

let mut name = None;
for child in children.iter() {
if let Ok(text) = texts.get(*child) {
let values = text
.sections
.iter()
.map(|v| v.value.to_string())
.collect::<Vec<String>>();
name = Some(values.join(" "));
}
}
name.map(|v| v.into_boxed_str())
}

fn calc_bounds(transform: &GlobalTransform, node: &Node) -> Rect {
Rect::new(
transform.translation().x.into(),
transform.translation().y.into(),
(transform.translation().x + node.calculated_size.x).into(),
(transform.translation().y + node.calculated_size.y).into(),
)
}

fn button_changed(
mut commands: Commands,
query: Query<(Entity, &GlobalTransform, &Node, &Children), Changed<Button>>,
texts: Query<&Text>,
) {
for (entity, transform, node, children) in &query {
let node = AccessKitNode {
role: Role::Button,
bounds: Some(calc_bounds(transform, node)),
name: calc_name(&texts, children),
focusable: true,
default_action_verb: Some(DefaultActionVerb::Click),
..default()
};
commands
.entity(entity)
.insert(AccessibilityNode::from(node));
ndarilek marked this conversation as resolved.
Show resolved Hide resolved
}
}

fn image_changed(
mut commands: Commands,
query: Query<(Entity, &GlobalTransform, &Node, &Children), (Changed<UiImage>, Without<Button>)>,
texts: Query<&Text>,
) {
for (entity, transform, node, children) in &query {
let node = AccessKitNode {
role: Role::Image,
bounds: Some(calc_bounds(transform, node)),
name: calc_name(&texts, children),
..default()
};
commands
.entity(entity)
.insert(AccessibilityNode::from(node));
}
}

fn label_changed(
mut commands: Commands,
query: Query<(Entity, &GlobalTransform, &Node, &Text), Changed<Label>>,
) {
for (entity, transform, node, text) in &query {
let values = text
.sections
.iter()
.map(|v| v.value.to_string())
.collect::<Vec<String>>();
let name = values.join(" ");
let bounds = Rect::new(
transform.translation().x.into(),
transform.translation().y.into(),
(transform.translation().x + node.calculated_size.x).into(),
(transform.translation().y + node.calculated_size.y).into(),
);
let node = AccessKitNode {
role: Role::LabelText,
bounds: Some(bounds),
name: Some(name.into_boxed_str()),
..default()
};
commands
.entity(entity)
.insert(AccessibilityNode::from(node));
}
}

#[derive(Resource, Default, Deref, DerefMut)]
struct InteractionCache(HashMap<Entity, Interaction>);

fn interaction_changed(
mut cache: ResMut<InteractionCache>,
adapters: NonSend<Adapters>,
query: Query<(Entity, &Interaction), Changed<Interaction>>,
) {
for (entity, interaction) in &query {
if let Some(adapter) = adapters.get_primary_adapter() {
if *interaction == Interaction::Hovered {
let focus = Some(entity.to_node_id());
adapter.update(TreeUpdate { focus, ..default() });
} else if let Some(old_interaction) = cache.get(&entity) {
if *old_interaction == Interaction::Hovered {
adapter.update(TreeUpdate {
focus: None,
..default()
});
}
}
}
cache.insert(entity, interaction.clone());
}
}

fn interaction_removed(
mut cache: ResMut<InteractionCache>,
adapters: NonSend<Adapters>,
removed: RemovedComponents<Interaction>,
) {
for entity in removed.iter() {
if let Some(old_interaction) = cache.get(&entity) {
if *old_interaction == Interaction::Hovered {
ndarilek marked this conversation as resolved.
Show resolved Hide resolved
if let Some(adapter) = adapters.get_primary_adapter() {
adapter.update(TreeUpdate {
focus: None,
..default()
});
}
}
}
cache.remove(&entity);
}
}

fn action_requested(
mut events: EventReader<ActionRequest>,
transforms: Query<&GlobalTransform>,
camera: Query<(&Camera, Option<&UiCameraConfig>)>,
mut windows: ResMut<Windows>,
) {
for action in events.iter() {
let target = <Entity as AccessKitEntityExt>::from_node_id(&action.target);
match action.action {
Action::Focus => {
if let Ok(transform) = transforms.get(target) {
let is_ui_disabled = |camera_ui| {
matches!(camera_ui, Some(&UiCameraConfig { show_ui: false, .. }))
};
let window_id = camera
.iter()
.filter(|(_, camera_ui)| !is_ui_disabled(*camera_ui))
.filter_map(|(camera, _)| {
if let RenderTarget::Window(window_id) = camera.target {
Some(window_id)
} else {
None
}
})
.next();
if let Some(window_id) = window_id {
if let Some(window) = windows.get_mut(window_id) {
if window.is_focused() {
let position = transform.translation().truncate();
window.set_cursor_position(position);
}
}
}
}
}
_ => {
println!("Unsupported: {:?}", action);
}
};
println!("AT action request: {:?}", action);
}
}

pub(crate) struct AccessibilityPlugin;

impl Plugin for AccessibilityPlugin {
fn build(&self, app: &mut App) {
app.init_resource::<InteractionCache>()
.add_system_to_stage(CoreStage::PreUpdate, button_changed)
.add_system_to_stage(CoreStage::PreUpdate, image_changed)
.add_system_to_stage(CoreStage::PreUpdate, label_changed)
.add_system_to_stage(CoreStage::PreUpdate, interaction_changed)
.add_system_to_stage(CoreStage::PostUpdate, interaction_removed)
.add_system_to_stage(CoreStage::PreUpdate, action_requested);
}
}
6 changes: 4 additions & 2 deletions crates/bevy_ui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ mod render;
mod stack;
mod ui_node;

mod accessibility;
pub mod camera_config;
pub mod node_bundles;
pub mod update;
Expand All @@ -25,8 +26,7 @@ pub use ui_node::*;
pub mod prelude {
#[doc(hidden)]
pub use crate::{
camera_config::*, geometry::*, node_bundles::*, ui_node::*, widget::Button, Interaction,
UiScale,
camera_config::*, geometry::*, node_bundles::*, ui_node::*, widget::*, Interaction, UiScale,
};
}

Expand Down Expand Up @@ -104,6 +104,8 @@ impl Plugin for UiPlugin {
.register_type::<UiImage>()
.register_type::<Val>()
.register_type::<widget::Button>()
.register_type::<widget::Label>()
.add_plugin(accessibility::AccessibilityPlugin)
.add_system_to_stage(
CoreStage::PreUpdate,
ui_focus_system.label(UiSystem::Focus).after(InputSystem),
Expand Down
9 changes: 9 additions & 0 deletions crates/bevy_ui/src/widget/label.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use bevy_ecs::prelude::Component;
use bevy_ecs::reflect::ReflectComponent;
use bevy_reflect::std_traits::ReflectDefault;
use bevy_reflect::Reflect;

/// Marker struct for labels
Copy link
Member

Choose a reason for hiding this comment

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

Nit: It might be good to indicate why this component should be used. Seems like it's mainly for the Role::Label, but that's not immediately obvious (users may skip this in favor of just relying on Text).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you have any wording suggestions I'm open to them--brain is a bit fried ATM. Alternately I can just remove it in favor of attaching a custom NodeBuilder to those nodes in particular. I added Label before I realized how easy it was to just slap on a custom NodeBuilder and make whatever customizations I needed there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, I feel like a label is a fairly common UI widget type, and we're going to want this or something like it at some point anyway. So it isn't really an accessibility thing, more a general widget I probably introduced before it's time thing.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have any wording suggestions off the top of my head. If it's meant to be part of a larger widget ecosystem, then maybe we come back to this (proper documentation) when we're ready to fully implement it? I imagine its usage and design could change a lot between now and when we truly define a "label" widget.

#[derive(Component, Debug, Default, Clone, Copy, Reflect)]
#[reflect(Component, Default)]
pub struct Label;
2 changes: 2 additions & 0 deletions crates/bevy_ui/src/widget/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

mod button;
mod image;
mod label;
mod text;

pub use button::*;
pub use image::*;
pub use label::*;
pub use text::*;
6 changes: 5 additions & 1 deletion crates/bevy_window/src/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,16 @@ impl WindowId {
}
/// The [`WindowId`] for the primary window.
pub const fn primary() -> Self {
WindowId(Uuid::from_u128(0))
WindowId(Uuid::from_u128(1))
}
/// Get whether or not this [`WindowId`] is for the primary window.
pub fn is_primary(&self) -> bool {
*self == WindowId::primary()
}
/// Get this [`WindowId`] as a `u128`.
pub fn as_u128(&self) -> u128 {
self.0.as_u128()
}
}

use crate::CursorIcon;
Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_winit/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ x11 = ["winit/x11"]
[dependencies]
# bevy
bevy_app = { path = "../bevy_app", version = "0.9.0" }
bevy_derive = { path = "../bevy_derive", version = "0.9.0" }
bevy_ecs = { path = "../bevy_ecs", version = "0.9.0" }
bevy_input = { path = "../bevy_input", version = "0.9.0" }
bevy_math = { path = "../bevy_math", version = "0.9.0" }
Expand All @@ -24,6 +25,8 @@ bevy_utils = { path = "../bevy_utils", version = "0.9.0" }

# other
winit = { version = "0.27", default-features = false }
accesskit = "0.8"
accesskit_winit = "0.7"
approx = { version = "0.5", default-features = false }
raw-window-handle = "0.5"

Expand Down
Loading