Merged
Conversation
lucasmerlin
force-pushed
the
lucas/group-accesskit-nodes-by-ui
branch
from
0f9886d to
c1d8d10
Compare
…it-nodes-by-ui # Conflicts: # crates/egui/src/context.rs # crates/egui_demo_app/src/accessibility_inspector.rs
…it-nodes-by-ui # Conflicts: # crates/egui_demo_app/src/accessibility_inspector.rs
# Conflicts: # crates/egui/src/context.rs # crates/egui/src/plugin.rs
…it-nodes-by-ui # Conflicts: # crates/egui/src/containers/window.rs # crates/egui_demo_app/src/accessibility_inspector.rs
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors egui's accessibility system to create AccessKit nodes for each Ui rather than using a global parent stack. This change aims to improve accessibility by providing better parent-child relationships between UI elements.
Key changes:
- Replaces global accessibility parent stack with per-UI parent assignment via
UiBuilder - Creates AccessKit nodes with
GenericContainerrole for allUiinstances - Updates accessibility parent registration to use explicit parent-child mapping
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/egui/src/ui_builder.rs | Adds accessibility_parent field and method to UiBuilder |
| crates/egui/src/ui.rs | Updates Ui creation to register accessibility parents and create AccessKit nodes |
| crates/egui/src/context.rs | Replaces parent stack with parent mapping system and removes with_accessibility_parent method |
| crates/egui/src/pass_state.rs | Changes AccessKitPassState to use parent/child maps instead of parent stack |
| crates/egui/src/response.rs | Adds Panel widget type mapping to AccessKit role |
| crates/egui/src/lib.rs | Adds Panel widget type enum variant |
| crates/egui/src/data/output.rs | Adds string representation for Panel widget type |
| crates/egui/src/containers/window.rs | Updates window resize interaction to use new accessibility parent system |
| crates/egui/src/containers/panel.rs | Adds panel widget info registration |
| crates/egui/src/containers/area.rs | Sets accessibility parent for area content UI |
| crates/egui/src/text_selection/accesskit_text.rs | Updates text widget accessibility to use new parent registration |
| crates/egui_demo_app/src/accessibility_inspector.rs | Removes usage of deprecated with_accessibility_parent method |
Comments suppressed due to low confidence (1)
crates/egui/src/context.rs:1
- Large commented-out code block should be removed. If this code is needed for reference or future implementation, consider moving it to a separate document or issue tracker.
#![warn(missing_docs)] // Let's keep `Context` well-documented.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
emilk
approved these changes
Oct 7, 2025
Comment on lines
+205
to
+206
| #[cfg(feature = "accesskit")] | ||
| ui.ctx().accesskit_node_builder(ui.unique_id, |node| { |
Owner
There was a problem hiding this comment.
I think it's time we remove the "accesskit" feature, and just always depend on accesskit:
That is: replace the compile-time flag with a runtime flag. (not in this PR though)
…it-nodes-by-ui # Conflicts: # crates/egui_demo_app/src/accessibility_inspector.rs
# Conflicts: # Cargo.toml # crates/egui/src/containers/panel.rs # crates/egui_demo_app/src/accessibility_inspector.rs
lucasmerlin
removed
the
do-not-merge
Masterchef365
pushed a commit
to Masterchef365/egui
that referenced
this pull request
Apr 3, 2026
* closes emilk#5674 This changes egui to create an AccessKit node for each `Ui`. I'm not sure if this alone will directly improve accessibility, but it should make it easier to create the correct parent / child relations (e.g. grouping menus as children of menu buttons). Instead of having a global stack of parent ids, they are now passed via a parent_id field in `UiBuilder`. If having all these `GenericContainer` nodes somehow is bad for accessibility, the PR could also be changed to only create nodes if there is actually some accessibility info with it (the relevant is currently commented-out in the PR). But I think screen readers should just ignore these nodes, so it should be fine? We could also use this as motivation to git red of some unnecessary wrapped `Ui`s, e.g. CentralPanel creates 3 Uis when 2 should be enough (the initial Ui and a Frame, maybe we could even only show the `Frame` if we can give it an UiBuilder and somehow show the Frame with `Ui::new`). Here is a screenshot from the accessibility inspector (emilk#7368) with this PR: <img width="431" height="744" alt="Screenshot 2025-07-24 at 12 09 55" src="https://github.com/user-attachments/assets/6c4e5ff6-5c38-450e-9500-0776c9018d8c" /> Without this PR: https://github.com/user-attachments/assets/270e32fc-9c7a-4dad-8c90-7638c487a602
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This changes egui to create an AccessKit node for each
Ui. I'm not sure if this alone will directly improve accessibility, but it should make it easier to create the correct parent / child relations (e.g. grouping menus as children of menu buttons).Instead of having a global stack of parent ids, they are now passed via a parent_id field in
UiBuilder.If having all these
GenericContainernodes somehow is bad for accessibility, the PR could also be changed to only create nodes if there is actually some accessibility info with it (the relevant is currently commented-out in the PR). But I think screen readers should just ignore these nodes, so it should be fine? We could also use this as motivation to git red of some unnecessary wrappedUis, e.g. CentralPanel creates 3 Uis when 2 should be enough (the initial Ui and a Frame, maybe we could even only show theFrameif we can give it an UiBuilder and somehow show the Frame withUi::new).Here is a screenshot from the accessibility inspector (#7368) with this PR:
Without this PR:
Screen.Recording.2025-07-24.at.12.12.34.mov