From bb3a64600e948cf1dec0c1ab916086b13e8fbade Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Thu, 14 Nov 2024 17:11:17 +0100 Subject: [PATCH 1/2] Update accesskit to 0.17 --- Cargo.lock | 34 +++++++++---------- crates/egui-winit/Cargo.toml | 2 +- crates/egui/Cargo.toml | 2 +- crates/egui/src/context.rs | 22 ++++++------ crates/egui/src/pass_state.rs | 2 +- crates/egui/src/response.rs | 9 ++--- .../egui/src/text_selection/accesskit_text.rs | 4 +-- crates/egui/src/widgets/drag_value.rs | 2 +- crates/egui/tests/accesskit.rs | 14 ++++---- .../src/demo/demo_app_windows.rs | 2 +- crates/egui_kittest/README.md | 4 +-- crates/egui_kittest/src/builder.rs | 4 +-- crates/egui_kittest/src/lib.rs | 4 +-- 13 files changed, 50 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 000165018c6..5d2f88807e2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -20,9 +20,9 @@ checksum = "c71b1793ee61086797f5c80b6efa2b8ffa6d5dd703f118545808a7f2e27f7046" [[package]] name = "accesskit" -version = "0.16.3" +version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "99b76d84ee70e30a4a7e39ab9018e2b17a6a09e31084176cc7c0b2dec036ba45" +checksum = "45c97bb3cc1dacbdc6d1147040fc61309590d3e1ab5efd92a8a09c7a2e07284c" dependencies = [ "enumn", "serde", @@ -30,9 +30,9 @@ dependencies = [ [[package]] name = "accesskit_atspi_common" -version = "0.9.3" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f5393c75d4666f580f4cac0a968bc97c36076bb536a129f28210dac54ee127ed" +checksum = "03db49d2948db6875c69a1ef17816efa8e3d9f36c7cd79e467d8562a6695662b" dependencies = [ "accesskit", "accesskit_consumer", @@ -44,9 +44,9 @@ dependencies = [ [[package]] name = "accesskit_consumer" -version = "0.24.3" +version = "0.25.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7a12dc159d52233c43d9fe5415969433cbdd52c3d6e0df51bda7d447427b9986" +checksum = "fa3a17950ce0d911f132387777b9b3d05eddafb59b773ccaa53fceefaeb0228e" dependencies = [ "accesskit", "immutable-chunkmap", @@ -54,9 +54,9 @@ dependencies = [ [[package]] name = "accesskit_macos" -version = "0.17.4" +version = "0.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bfc6c1ecd82053d127961ad80a8beaa6004fb851a3a5b96506d7a6bd462403f6" +checksum = "e8d94b7544775dddce398e2500a8b3cc2be3655190879071ce6a9e5610195be4" dependencies = [ "accesskit", "accesskit_consumer", @@ -68,9 +68,9 @@ dependencies = [ [[package]] name = "accesskit_unix" -version = "0.12.3" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be7f5cf6165be10a54b2655fa2e0e12b2509f38ed6fc43e11c31fdb7ee6230bb" +checksum = "3a88d913b144104dd825f75db1b82c63d754b01c53c2f9b7545dcdfae63bb0ed" dependencies = [ "accesskit", "accesskit_atspi_common", @@ -86,9 +86,9 @@ dependencies = [ [[package]] name = "accesskit_windows" -version = "0.23.2" +version = "0.24.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "974e96c347384d9133427167fb8a58c340cb0496988dacceebdc1ed27071023b" +checksum = "0aaa870a5d047338f03707706141f22c98c20e79d5403bf3c9b195549e6cdeea" dependencies = [ "accesskit", "accesskit_consumer", @@ -100,9 +100,9 @@ dependencies = [ [[package]] name = "accesskit_winit" -version = "0.22.4" +version = "0.23.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aea3522719f1c44564d03e9469a8e2f3a98b3a8a880bd66d0789c6b9c4a669dd" +checksum = "3555a67a9bb208f620cfc3746f1502d1512f0ffbdb19c6901aa90b111aa56ec5" dependencies = [ "accesskit", "accesskit_macos", @@ -2226,7 +2226,7 @@ checksum = "e2db585e1d738fc771bf08a151420d3ed193d9d895a36df7f6f8a9456b911ddc" [[package]] name = "kittest" version = "0.1.0" -source = "git+https://github.com/rerun-io/kittest?branch=main#1336a504aefd05f7e9aa7c9237ae44ba9e72acdd" +source = "git+https://github.com/rerun-io/kittest?branch=main#63c5b7d58178900e523428ca5edecbba007a2702" dependencies = [ "accesskit", "accesskit_consumer", @@ -2261,7 +2261,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4979f22fdb869068da03c9f7528f8297c6fd2606bc3a4affe42e6a823fdb8da4" dependencies = [ "cfg-if", - "windows-targets 0.48.5", + "windows-targets 0.52.6", ] [[package]] @@ -4465,7 +4465,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] diff --git a/crates/egui-winit/Cargo.toml b/crates/egui-winit/Cargo.toml index 4d179fc2d88..2f65548757d 100644 --- a/crates/egui-winit/Cargo.toml +++ b/crates/egui-winit/Cargo.toml @@ -69,7 +69,7 @@ winit = { workspace = true, default-features = false } #! ### Optional dependencies # feature accesskit -accesskit_winit = { version = "0.22", optional = true } +accesskit_winit = { version = "0.23", optional = true } ## Enable this when generating docs. document-features = { workspace = true, optional = true } diff --git a/crates/egui/Cargo.toml b/crates/egui/Cargo.toml index 487a4eac6e9..bba94af066d 100644 --- a/crates/egui/Cargo.toml +++ b/crates/egui/Cargo.toml @@ -87,7 +87,7 @@ ahash.workspace = true nohash-hasher.workspace = true #! ### Optional dependencies -accesskit = { version = "0.16", optional = true } +accesskit = { version = "0.17.0", optional = true } backtrace = { workspace = true, optional = true } diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 331430c7865..94eb25e14db 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -552,13 +552,13 @@ impl ContextImpl { crate::profile_scope!("accesskit"); use crate::pass_state::AccessKitPassState; let id = crate::accesskit_root_id(); - let mut builder = accesskit::NodeBuilder::new(accesskit::Role::Window); + let mut root_node = accesskit::Node::new(accesskit::Role::Window); let pixels_per_point = viewport.input.pixels_per_point(); - builder.set_transform(accesskit::Affine::scale(pixels_per_point.into())); - let mut node_builders = IdMap::default(); - node_builders.insert(id, builder); + root_node.set_transform(accesskit::Affine::scale(pixels_per_point.into())); + let mut nodes = IdMap::default(); + nodes.insert(id, root_node); viewport.this_pass.accesskit_state = Some(AccessKitPassState { - node_builders, + nodes, parent_stack: vec![id], }); } @@ -640,9 +640,9 @@ impl ContextImpl { } #[cfg(feature = "accesskit")] - fn accesskit_node_builder(&mut self, id: Id) -> &mut accesskit::NodeBuilder { + fn accesskit_node_builder(&mut self, id: Id) -> &mut accesskit::Node { let state = self.viewport().this_pass.accesskit_state.as_mut().unwrap(); - let builders = &mut state.node_builders; + let builders = &mut state.nodes; if let std::collections::hash_map::Entry::Vacant(entry) = builders.entry(id) { entry.insert(Default::default()); let parent_id = state.parent_stack.last().unwrap(); @@ -1279,7 +1279,7 @@ impl Context { #[cfg(feature = "accesskit")] if enabled && sense.click - && input.has_accesskit_action_request(id, accesskit::Action::Default) + && input.has_accesskit_action_request(id, accesskit::Action::Click) { res.fake_primary_click = true; } @@ -2359,9 +2359,9 @@ impl ContextImpl { let root_id = crate::accesskit_root_id().accesskit_id(); let nodes = { state - .node_builders + .nodes .into_iter() - .map(|(id, builder)| (id.accesskit_id(), builder.build())) + .map(|(id, node)| (id.accesskit_id(), node)) .collect() }; let focus_id = self @@ -3270,7 +3270,7 @@ impl Context { pub fn accesskit_node_builder( &self, id: Id, - writer: impl FnOnce(&mut accesskit::NodeBuilder) -> R, + writer: impl FnOnce(&mut accesskit::Node) -> R, ) -> Option { self.write(|ctx| { ctx.viewport() diff --git a/crates/egui/src/pass_state.rs b/crates/egui/src/pass_state.rs index bbeaca9b3c6..da42e0932ae 100644 --- a/crates/egui/src/pass_state.rs +++ b/crates/egui/src/pass_state.rs @@ -70,7 +70,7 @@ impl ScrollTarget { #[cfg(feature = "accesskit")] #[derive(Clone)] pub struct AccessKitPassState { - pub node_builders: IdMap, + pub nodes: IdMap, pub parent_stack: Vec, } diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index 9051d3a2d7f..a772633d219 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -988,7 +988,7 @@ impl Response { } #[cfg(feature = "accesskit")] - pub(crate) fn fill_accesskit_node_common(&self, builder: &mut accesskit::NodeBuilder) { + pub(crate) fn fill_accesskit_node_common(&self, builder: &mut accesskit::Node) { if !self.enabled { builder.set_disabled(); } @@ -1001,15 +1001,12 @@ impl Response { if self.sense.focusable { builder.add_action(accesskit::Action::Focus); } - if self.sense.click && builder.default_action_verb().is_none() { - builder.set_default_action_verb(accesskit::DefaultActionVerb::Click); - } } #[cfg(feature = "accesskit")] fn fill_accesskit_node_from_widget_info( &self, - builder: &mut accesskit::NodeBuilder, + builder: &mut accesskit::Node, info: crate::WidgetInfo, ) { use crate::WidgetType; @@ -1039,7 +1036,7 @@ impl Response { builder.set_disabled(); } if let Some(label) = info.label { - builder.set_name(label); + builder.set_label(label); } if let Some(value) = info.current_text_value { builder.set_value(value); diff --git a/crates/egui/src/text_selection/accesskit_text.rs b/crates/egui/src/text_selection/accesskit_text.rs index f56ab9eddb1..d0c3869038d 100644 --- a/crates/egui/src/text_selection/accesskit_text.rs +++ b/crates/egui/src/text_selection/accesskit_text.rs @@ -29,8 +29,6 @@ pub fn update_accesskit_for_text_widget( }); } - builder.set_default_action_verb(accesskit::DefaultActionVerb::Focus); - builder.set_role(role); parent_id @@ -44,7 +42,7 @@ pub fn update_accesskit_for_text_widget( for (row_index, row) in galley.rows.iter().enumerate() { let row_id = parent_id.with(row_index); ctx.accesskit_node_builder(row_id, |builder| { - builder.set_role(accesskit::Role::InlineTextBox); + builder.set_role(accesskit::Role::TextRun); let rect = row.rect.translate(galley_pos.to_vec2()); builder.set_bounds(accesskit::Rect { x0: rect.min.x.into(), diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index feae91ade1e..feec4d07ff5 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -686,7 +686,7 @@ impl<'a> Widget for DragValue<'a> { } // The name field is set to the current value by the button, // but we don't want it set that way on this widget type. - builder.clear_name(); + builder.clear_label(); // Always expose the value as a string. This makes the widget // more stable to accessibility users as it switches // between edit and button modes. This is particularly important diff --git a/crates/egui/tests/accesskit.rs b/crates/egui/tests/accesskit.rs index 1354198d47d..83efeab3f37 100644 --- a/crates/egui/tests/accesskit.rs +++ b/crates/egui/tests/accesskit.rs @@ -46,7 +46,7 @@ fn button_node() { .find(|(_, node)| node.role() == Role::Button) .expect("Button should exist in the accesskit output"); - assert_eq!(button.name(), Some(button_text)); + assert_eq!(button.label(), Some(button_text)); assert!(!button.is_disabled()); } @@ -72,7 +72,7 @@ fn disabled_button_node() { .find(|(_, node)| node.role() == Role::Button) .expect("Button should exist in the accesskit output"); - assert_eq!(button.name(), Some(button_text)); + assert_eq!(button.label(), Some(button_text)); assert!(button.is_disabled()); } @@ -97,7 +97,7 @@ fn toggle_button_node() { .find(|(_, node)| node.role() == Role::Button) .expect("Toggle button should exist in the accesskit output"); - assert_eq!(toggle.name(), Some(button_text)); + assert_eq!(toggle.label(), Some(button_text)); assert!(!toggle.is_disabled()); } @@ -165,14 +165,14 @@ fn accesskit_output_single_egui_frame(run_ui: impl FnMut(&Context)) -> TreeUpdat } #[track_caller] -fn assert_button_exists(tree: &TreeUpdate, name: &str, parent: NodeId) { +fn assert_button_exists(tree: &TreeUpdate, label: &str, parent: NodeId) { let (node_id, _) = tree .nodes .iter() .find(|(_, node)| { - !node.is_hidden() && node.role() == Role::Button && node.name() == Some(name) + !node.is_hidden() && node.role() == Role::Button && node.label() == Some(label) }) - .expect("No visible button with that name exists."); + .expect("No visible button with that label exists."); assert_parent_child(tree, parent, *node_id); } @@ -183,7 +183,7 @@ fn assert_window_exists(tree: &TreeUpdate, title: &str, parent: NodeId) -> NodeI .nodes .iter() .find(|(_, node)| { - !node.is_hidden() && node.role() == Role::Window && node.name() == Some(title) + !node.is_hidden() && node.role() == Role::Window && node.label() == Some(title) }) .expect("No visible window with that title exists."); diff --git a/crates/egui_demo_lib/src/demo/demo_app_windows.rs b/crates/egui_demo_lib/src/demo/demo_app_windows.rs index 790e7289953..7b3f263831b 100644 --- a/crates/egui_demo_lib/src/demo/demo_app_windows.rs +++ b/crates/egui_demo_lib/src/demo/demo_app_windows.rs @@ -409,7 +409,7 @@ mod tests { let window = harness.node().children().next().unwrap(); // TODO(lucasmerlin): Windows should probably have a label? - //let window = harness.get_by_name(name); + //let window = harness.get_by_label(name); let size = window.raw_bounds().expect("window bounds").size(); harness.set_size(Vec2::new(size.width as f32, size.height as f32)); diff --git a/crates/egui_kittest/README.md b/crates/egui_kittest/README.md index db07f7e8d99..2ffda785c05 100644 --- a/crates/egui_kittest/README.md +++ b/crates/egui_kittest/README.md @@ -20,13 +20,13 @@ fn main() { let mut harness = Harness::builder().with_size(egui::Vec2::new(200.0, 100.0)).build(app); - let checkbox = harness.get_by_name("Check me!"); + let checkbox = harness.get_by_label("Check me!"); assert_eq!(checkbox.toggled(), Some(Toggled::False)); checkbox.click(); harness.run(); - let checkbox = harness.get_by_name("Check me!"); + let checkbox = harness.get_by_label("Check me!"); assert_eq!(checkbox.toggled(), Some(Toggled::True)); // You can even render the ui and do image snapshot tests diff --git a/crates/egui_kittest/src/builder.rs b/crates/egui_kittest/src/builder.rs index 45ad00e83a1..65c4dfbf0f6 100644 --- a/crates/egui_kittest/src/builder.rs +++ b/crates/egui_kittest/src/builder.rs @@ -56,7 +56,7 @@ impl HarnessBuilder { /// }); /// }, checked); /// - /// harness.get_by_name("Check me!").click(); + /// harness.get_by_label("Check me!").click(); /// harness.run(); /// /// assert_eq!(*harness.state(), true); @@ -85,7 +85,7 @@ impl HarnessBuilder { /// ui.checkbox(checked, "Check me!"); /// }, checked); /// - /// harness.get_by_name("Check me!").click(); + /// harness.get_by_label("Check me!").click(); /// harness.run(); /// /// assert_eq!(*harness.state(), true); diff --git a/crates/egui_kittest/src/lib.rs b/crates/egui_kittest/src/lib.rs index 8e410d84d11..081a7e616dd 100644 --- a/crates/egui_kittest/src/lib.rs +++ b/crates/egui_kittest/src/lib.rs @@ -119,7 +119,7 @@ impl<'a, State> Harness<'a, State> { /// }); /// }, checked); /// - /// harness.get_by_name("Check me!").click(); + /// harness.get_by_label("Check me!").click(); /// harness.run(); /// /// assert_eq!(*harness.state(), true); @@ -144,7 +144,7 @@ impl<'a, State> Harness<'a, State> { /// ui.checkbox(checked, "Check me!"); /// }, checked); /// - /// harness.get_by_name("Check me!").click(); + /// harness.get_by_label("Check me!").click(); /// harness.run(); /// /// assert_eq!(*harness.state(), true); From 7bde0d8001543887ea36edd3bd44b542018e64a9 Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Thu, 14 Nov 2024 18:30:25 +0100 Subject: [PATCH 2/2] Fixes from review --- crates/egui/src/response.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index a772633d219..f49383c70d1 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -1001,6 +1001,9 @@ impl Response { if self.sense.focusable { builder.add_action(accesskit::Action::Focus); } + if self.sense.click { + builder.add_action(accesskit::Action::Click); + } } #[cfg(feature = "accesskit")] @@ -1036,7 +1039,11 @@ impl Response { builder.set_disabled(); } if let Some(label) = info.label { - builder.set_label(label); + if matches!(builder.role(), Role::Label) { + builder.set_value(label); + } else { + builder.set_label(label); + } } if let Some(value) = info.current_text_value { builder.set_value(value);