ENH Add aria attrs to site tree by emteknetnz · Pull Request #3120 · silverstripe/silverstripe-cms

emteknetnz · 2025-10-09T01:14:34Z

Issue silverstripe/silverstripe-admin#2000

GuySartorelli

There are various recommendations in the W3C example which probably apply to our usage, since that example is for a tree used as navigation which is what we're doing here.

I recommend we follow that example, including add a nav parent to the tree list (instead of the current div), and additional recommendations such as using the aria-current attribute etc.
Note that not everything in the live example has a corresponding textual recommendation - but IMO we should treat the example's aria attributes as best practice anyway. I've marked some (but not all) of the differences between their example and this implementation in separate PR comments.

We can however ignore some of the recommendations from that example that are for regular websites, such as the use of the banner landmark.

I will note that Orca (the screen reader I'm using for testing) works really well both in terms of correctly announcing tree items and the tree itself and in terms of respecting correct key bindings with the W3C example than with our site tree. In our one orca didn't seem to recognise it was in a tree at all, and wouldn't respect the left/right arrow keybindings.

lang/en.yml

GuySartorelli · 2025-10-13T21:22:41Z

templates/SilverStripe/CMS/Controllers/Includes/CMSMain_SubTree.ss

+        ><ins class="jstree-icon font-icon-right-dir"
+              role="button"


We should just use a semantic button element instead of an inserted text element which is just completely symantically incorrect.

While screen readers should use the role attribute, some won't (unfortunately orca which I'm using to test is one that won't) so we should use semantically correct HTML elements wherever possible.

That said - in the W3C example the arrows aren't actually separately tabbable - instead the treeitem intercepts key presses for left/right arrows for expanding/collapsing, and the arrow is just a span with a click handler for mouse interaction.

This is more consistent with the keyboard interaction recommendations W3C make (it makes no mention of tab or enter for expanding/collapsing child nodes). Perhaps we should follow their example here as well?

I'm not going to change HTML tags (will affect styling) or tab order (will affect behat tests) in this PR - I've created a follow up issue for this

It feels like you might have not seen the second comment there, about making these not tabbable and not using the button role? If we do that, it doesn't really matter what the HTML tag is because assistive technologies will ignore them anyway - they'll only be for mouse interaction for sighted users.

As previously mentioned, changing either the html tags or the tab order will affect styling / css and behat and I do not want to that in this PR as this one is focused on aria attrs. I have created a follow up issue to implement those.

This is nearly the only problem now with what the screen reader is saying. Whenever I tab to one of these button-not-buttons, it leaves the context of the tree. The screen reader changes to "browse mode". When I tab back off it, it enters the context of the tree again, and I get told all the tree information (level 2, etc).

What interactive elements live where very much impacts the screen reader experience - it's hard to say "yes this is done the screen reader experience is good now" when this interactive element is interfering with that experience. These two things inform one another.

I guess I have to call it done and then when the follow up issue is implemented, I have to check it again and see how it impacts the screen reader experience? And then if more changes are needed to the aria attributes, we'll open another separate card to address it separately again?

templates/SilverStripe/CMS/Controllers/Includes/CMSMain_TreeView.ss

templates/SilverStripe/CMS/Controllers/Includes/CMSMain_TreeNode.ss

templates/SilverStripe/CMS/Controllers/Includes/CMSMain_SubTree.ss

GuySartorelli

The experience with a screen reader still isn't the same or as good as the W3C example. Are you using a screen reader to test this?
One clear example is when I use left/right arrow keys to expand/collapse a node, with the W3C example the screen reader announces "collapsed" or "expanded" as appropriate. With this PR I get no feedback at all.

I again recommend we follow the W3C example as closely as we can.

GuySartorelli · 2025-10-15T00:50:35Z

templates/SilverStripe/CMS/Controllers/Includes/CMSMain_SubTree.ss

+        ><ins class="jstree-icon font-icon-right-dir"
+              role="button"


It feels like you might have not seen the second comment there, about making these not tabbable and not using the button role? If we do that, it doesn't really matter what the HTML tag is because assistive technologies will ignore them anyway - they'll only be for mouse interaction for sighted users.

emteknetnz · 2025-10-15T05:25:43Z

Have moved all the aria stuff off the <li> on to the <a>, seems to be closer to the w3c example now

GuySartorelli

There's a merge conflict.

The only other thing remaining that bothers me (other than the <ins> stuff) is the level starts at level 2? Feels like the root level pages should all be level 1.

GuySartorelli · 2025-10-16T01:32:45Z

templates/SilverStripe/CMS/Controllers/Includes/CMSMain_SubTree.ss

+        ><ins class="jstree-icon font-icon-right-dir"
+              role="button"


This is nearly the only problem now with what the screen reader is saying. Whenever I tab to one of these button-not-buttons, it leaves the context of the tree. The screen reader changes to "browse mode". When I tab back off it, it enters the context of the tree again, and I get told all the tree information (level 2, etc).

What interactive elements live where very much impacts the screen reader experience - it's hard to say "yes this is done the screen reader experience is good now" when this interactive element is interfering with that experience. These two things inform one another.

I guess I have to call it done and then when the follow up issue is implemented, I have to check it again and see how it impacts the screen reader experience? And then if more changes are needed to the aria attributes, we'll open another separate card to address it separately again?

GuySartorelli · 2025-10-16T01:35:52Z

client/src/legacy/CMSMain.Tree.js

+  // Add role="tree" to the root level <ul> for accessibility
+  // This needs to be done with JS as jstree will override any attributes set server-side
+  $('.cms-tree > ul').entwine({
+    onmatch: function() {
+      this.attr('role', 'tree');
+    }
+  });


We should probably use one of the jstree events, such as loaded.jstree (similar to other events already in silverstripe/silverstripe-admin#2016)

emteknetnz mentioned this pull request Oct 9, 2025

Accessibility: Add aria attributes to site tree silverstripe/silverstripe-admin#2000

Open

4 tasks

emteknetnz force-pushed the pulls/6/tree-aria branch 2 times, most recently from 6441414 to 3a39359 Compare October 13, 2025 01:45

emteknetnz marked this pull request as ready for review October 13, 2025 01:45

GuySartorelli requested changes Oct 13, 2025

View reviewed changes

emteknetnz force-pushed the pulls/6/tree-aria branch 2 times, most recently from e971648 to 75a3ff2 Compare October 14, 2025 05:37

GuySartorelli requested changes Oct 15, 2025

View reviewed changes

emteknetnz force-pushed the pulls/6/tree-aria branch from 75a3ff2 to 1d05442 Compare October 15, 2025 05:04

GuySartorelli requested changes Oct 16, 2025

View reviewed changes

GuySartorelli reviewed Oct 16, 2025

View reviewed changes

emteknetnz force-pushed the pulls/6/tree-aria branch 2 times, most recently from 8f458be to ebfc0e6 Compare March 27, 2026 01:24

ENH Add aria attrs to site tree

5ea51f7

emteknetnz force-pushed the pulls/6/tree-aria branch from ebfc0e6 to 5ea51f7 Compare March 27, 2026 02:29

Conversation

emteknetnz commented Oct 9, 2025

Uh oh!

GuySartorelli left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GuySartorelli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emteknetnz commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GuySartorelli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

GuySartorelli left a comment •

edited

Loading

emteknetnz commented Oct 15, 2025 •

edited

Loading