Skip to content

chore(treeview): add treeitem role to shadow dom node #3894

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.

Sign up for GitHub

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
francinelucca merged 18 commits into from
Feb 6, 2026
Merged

chore(treeview): add treeitem role to shadow dom node #3894

Changes from 4 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1933706
chore(treeview): add treeitem role to shadow dom node
francinelucca Jan 28, 2026
6690fe2
Merge branch 'main' into chore/add-treeview-role
francinelucca Jan 28, 2026
62394fb
Merge branch 'main' of github.com:primer/view_components into chore/a…
francinelucca Feb 3, 2026
75f87c7
Merge branch 'chore/add-treeview-role' of github.com:primer/view_comp…
francinelucca Feb 3, 2026
75781e9
only add treeitem role when treeview fragment NOT hidden
francinelucca Feb 4, 2026
877807d
Merge branch 'main' into chore/add-treeview-role
francinelucca Feb 4, 2026
2a8b768
Merge branch 'main' into chore/add-treeview-role
francinelucca Feb 4, 2026
919cedd
Merge branch 'main' of github.com:primer/view_components into chore/a…
francinelucca Feb 4, 2026
68a0677
Merge branch 'chore/add-treeview-role' of github.com:primer/view_comp…
francinelucca Feb 4, 2026
5417621
test
francinelucca Feb 4, 2026
e43283f
Merge branch 'main' into chore/add-treeview-role
francinelucca Feb 5, 2026
9ad047a
replace includeFragment node when hidden, add keyboard test
francinelucca Feb 5, 2026
801b8c9
Merge branch 'chore/add-treeview-role' of github.com:primer/view_comp…
francinelucca Feb 5, 2026
1e80bba
prettier
francinelucca Feb 5, 2026
67ff13b
[WIP] Fix treeitem role on shadow DOM node based on feedback (#3918)
Copilot Feb 5, 2026
3b55d7c
Extract duplicated include-fragment check into private method (#3917)
Copilot Feb 5, 2026
ba55fbe
Update treeview with treeitem role
francinelucca Feb 6, 2026
4947038
Merge branch 'main' into chore/add-treeview-role
francinelucca Feb 6, 2026
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
2 changes: 1 addition & 1 deletion app/components/primer/alpha/tree_view/spinner_loader.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<% with_loading_failure_message # set the default %>
<% end %>

<%= render(Primer::BaseComponent.new(tag: :"tree-view-include-fragment", src: @src, loading: :lazy, data: { target: "tree-view-sub-tree-node.subTree tree-view-sub-tree-node.includeFragment", path: @container.path.to_json }, hidden: @container.expanded?, accept: "text/fragment+html")) do %>
<%= render(Primer::BaseComponent.new(tag: :"tree-view-include-fragment", src: @src, loading: :lazy, data: { target: "tree-view-sub-tree-node.subTree tree-view-sub-tree-node.includeFragment", path: @container.path.to_json }, hidden: @container.expanded?, accept: "text/fragment+html", role: "treeitem")) do %>

Copilot AI Jan 28, 2026

Copy link

Choose a reason for hiding this comment

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

Adding role="treeitem" to the tree-view-include-fragment element creates an invalid ARIA tree structure. According to the WAI-ARIA specification, a treeitem element cannot contain a group element, but this tree-view-include-fragment wraps a ul[role="group"] (SubTreeContainer).

The actual DOM structure becomes:

li[role="none"]
  └── tree-view-include-fragment[role="treeitem"]  ← Invalid
      └── ul[role="group"]
          └── li[role="none"]
              └── div[role="treeitem"]

The tree-view-include-fragment is a technical implementation detail for lazy loading and should remain role-less (allowing it to be transparent in the accessibility tree). The actual treeitems are the div[role="treeitem"] elements inside the ul[role="group"] that this fragment contains.

Note that skeleton_loader.html.erb does not have this role attribute, creating an inconsistency. The correct fix depends on what the actual accessibility issue is. If the concern is that the loading indicator needs to be announced, the role should be on the inner Node element (which already has role="treeitem" from Node.rb line 108), not on the outer include-fragment wrapper.

Suggested change
<%= render(Primer::BaseComponent.new(tag: :"tree-view-include-fragment", src: @src, loading: :lazy, data: { target: "tree-view-sub-tree-node.subTree tree-view-sub-tree-node.includeFragment", path: @container.path.to_json }, hidden: @container.expanded?, accept: "text/fragment+html", role: "treeitem")) do %>
<%= render(Primer::BaseComponent.new(tag: :"tree-view-include-fragment", src: @src, loading: :lazy, data: { target: "tree-view-sub-tree-node.subTree tree-view-sub-tree-node.includeFragment", path: @container.path.to_json }, hidden: @container.expanded?, accept: "text/fragment+html")) do %>

Copilot uses AI. Check for mistakes.
<%= render(@container) do %>
<%= render(Primer::Alpha::TreeView::Node.new(path: [*@container.path, :loader], data: { target: "tree-view-sub-tree-node.loadingIndicator" }, node_variant: :div)) do |node| %>
<% node.with_text_content { "Loading..." } %>
Expand Down
Loading