[Studio UI] Add WAI-ARIA tree roles to element tree - #4056
Open
idaiv wants to merge 10 commits into
Open
Conversation
- Container: role="tree" - Nodes: role="treeitem" (replaces role="button") - Child lists: role="group" - aria-expanded on expandable nodes - aria-selected on all nodes - aria-level reflecting nesting depth Screen readers can now identify the tree structure, announce folder state, selection, and depth. Completes WCAG 2.1 4.1.2 (Name, Role, Value) for tree components. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
idaiv
added
the
Skip Milestone Check
Contributor
There was a problem hiding this comment.
Pull request overview
Verdict: Needs changes. Adds WAI-ARIA tree semantics to the shared element-tree component.
Changes:
- Adds
tree,treeitem, andgrouproles. - Exposes expanded, selected, and hierarchy state through ARIA attributes.
Review contract:
- Claim: Improve element-tree accessibility for WCAG 4.1.2.
- Root cause: Only partially addressed; naming, hierarchy, focus, and levels remain incorrect.
- Call sites: Shared implementation covers all consumers, but labels require consumer updates.
- Boundary: The shared element-tree component is appropriate.
- Compatibility: No breaking public API change currently introduced.
- Tests: No automated accessibility regression coverage was added.
- Docs/changelog: No public documentation impact identified.
- Risks: Screen-reader behavior remains unverified and currently exposes invalid semantics.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
element-tree.tsx |
Adds the tree container role. |
list/tree-list.tsx |
Marks child lists as groups. |
node/tree-node.tsx |
Adds treeitem role and state attributes. |
Suppressed comments (2)
assets/js/src/core/components/element-tree/element-tree.tsx:137
- This behavioral accessibility fix has no automated regression coverage despite existing
ElementTreetests and an integration test that renders the realTreeNode. Add assertions for the named tree, parent-owned groups, a tabbable active item, state attributes, valid levels with and without a visible root, and paginated position metadata.
role="tree"
assets/js/src/core/components/element-tree/list/tree-list.tsx:59
- These child lists are paginated (
totalandRenderPager), but their treeitems expose no absolute position or full set size. A screen reader will derive values only from the rendered page, so page-two items can be announced as “1 of N” rather than their real position. Pass absolutearia-posinsetandaria-setsize={total}to each rendered child.
role="group"
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| onKeyDown={ onKeyDown } | ||
| ref={ setRef } | ||
| role='button' | ||
| role='treeitem' |
| <div | ||
| className={ ['tree', styles.tree].join(' ') } | ||
| data-testid={ treeTestId } | ||
| role="tree" |
| <div | ||
| className='tree-list' | ||
| data-testid={ `tree-list-${node.id}` } | ||
| role="group" |
| <Flex | ||
| align="center" | ||
| aria-expanded={ props.hasChildren === true ? isExpanded : undefined } | ||
| aria-level={ level + 1 } |
Root node has level={-1}, producing aria-level={0} which is invalid
(WAI-ARIA minimum is 1). Clamp to Math.max(1, level + 1).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Each tree instance (Asset, Document, Data Object) now passes a translated aria-label to the role="tree" container so screen readers can distinguish between multiple trees on screen. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SonarCloud flags role="group" on a div. Use a ul element instead which is semantically appropriate for a list of tree items and satisfies the rule while maintaining the WAI-ARIA tree pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SonarCloud S6819 only accepts <details>, <fieldset>, <optgroup>, or <address> for the group role — none appropriate for a tree. The semantic <ul> element inside a role="tree" container is already understood by assistive technology as a grouping, so the explicit role attribute is unnecessary. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
idaiv
removed
the
Skip Milestone Check
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.
Summary
role="tree"role="treeitem"(replacesrole="button")role="group"aria-expanded,aria-selected,aria-levelon all nodesTest plan
role="tree"role="treeitem",aria-level,aria-selectedaria-expanded="true"role="group"🤖 Generated with Claude Code
Relates to https://github.com/pimcore/product-management/issues/372