File Viewer Tree Examples: Add aria-selected #1869

kdoberst · 2021-04-23T18:20:18Z

Addresses #1680.

Address: w3c#1680 When an item receives a tab index of 0, then the aria-selected flag also turns to true.

a11ydoer · 2021-06-21T21:19:39Z

@kdoberst Thanks for your PR as well as your patience.
We will discuss this PR at ARIA APG meeting on June 22 and update the issue.

smhigley

Thanks for starting this PR! Let me know if you have questions on my feedback.

smhigley · 2021-06-29T18:11:28Z

examples/treeview/treeview-1/js/tree.js

@@ -93,9 +93,11 @@ Tree.prototype.setFocusToItem = function (treeitem) {

    if (ti === treeitem) {
      ti.domNode.tabIndex = 0;
+      ti.domNode.ariaSelected = true;


To set the aria-selected attribute, you'd need to use el.setAttribute('aria-selected', 'true'). AOM attribute reflection isn't broadly supported, or stable.

The selected state should also likely be set to the actual selected item, and not the currently focused item. We should also probably look into adding a visual selected state, though we could conceivably add the attribute even without the visual state.

setAttribute fixed. Visual selected indicator not fixed yet. What should it look like?

examples/treeview/treeview-1/js/tree.js

mcking65 · 2021-10-31T19:06:09Z

Added to November 2, 2021 Agenda · w3c/aria-practices Wiki

a11ydoer · 2021-11-02T01:55:41Z

I don't see any aria-selected attribute from current treview example.

@jongund or @zcorpan any idea?

zcorpan · 2021-11-02T13:09:41Z

@a11ydoer right, this PRs adds it. But I guess it should also include aria-selected="false" in markup on each item, as it's a required state.

Edit: added in e93dbfd

jongund · 2021-11-02T13:46:51Z

@zcorpan The roles, properties and states table needs to be updated and also the regression tests for the additional aria-selected attribute to be documented and tested in the example.

a11ydoer · 2021-11-02T17:08:17Z

@jongund I agree with you.
Can you point where the roles, properties and states table is? It has been a while to see that table and I will make sure whether we have updated every attribute in the table.

zcorpan · 2021-11-03T15:39:35Z

The implementation at first was setting aria-selected="true" to the currently focused item. Since the example doesn't change which file is "selected" (as shown in the "File or Folder Selected" box) when moving focus, only when clicking or pressing enter, I changed the implementation to set aria-selected="true" in onClick.

I've also written regression tests.

mzgoddard

Reading how tree.js and treeitem.js behave this example implements single selection and not multi selection. Is that right?

If that is right does the following apply to this example?

https://www.w3.org/TR/wai-aria-practices-1.2/#keyboard-interaction-23

When a single-select tree receives focus:

If none of the nodes are selected before the tree receives focus, focus is set on the first node.

If a node is selected before the tree receives focus, focus is set on the selected node.

https://www.w3.org/TR/wai-aria-practices-1.2/#tree_roles_states_props

If the tree does not support multiple selection, aria-selected is set to true for the selected node and it is not present on any other node in the tree.

if the tree supports multiple selection:

All selected nodes have aria-selected set to true.

All nodes that are selectable but not selected have aria-selected set to false.

If the tree contains nodes that are not selectable, those nodes do not have the aria-selected state.

If that applies, I think a few changes need to be made:

No treeitem in the modified example html should have the attribute aria-selected="false"
Add event handling to select the first node when no node is selected
Add event handling to focus the selected node when tree receives focus from outside the tree
Regression test that clicking selects an item
Regression test that Key.SPACE selects an item (ENTER is already covered in this PR)

mzgoddard · 2021-11-04T17:12:15Z

examples/treeview/treeview-1/js/tree.js

@@ -87,6 +88,14 @@ Tree.prototype.init = function () {
  this.firstTreeitem.domNode.tabIndex = 0;
 };

+Tree.prototype.setSelectedToItem = function (treeitem) {
+  if (this.selectedItem) {


What do you think of mirroring how setFocusToItem is implemented with something like:

for (let i = 0; i < this.treeitems.length; i++) { const ti = this.treeitems[i]; if (ti === treeitem) { ti.domNode.setAttribute('aria-selected', 'true'); } else { ti.domNode.removeAttribute('aria-selected'); } }

mzgoddard · 2021-11-04T17:18:20Z

test/tests/treeview_treeview-1a.js

+        t,
+        ex.treeitemSelector + '[tabindex="0"]'
+      );
+      const focusMoved = items[i] !== nextItem;


As an aside this will always be true. As I understand it, nextItem may represent the same element in the dom but webdriver's js api doesn't guarentee that it will be the same object in this context. To test that they are the same I think you'd use WebElement.equals(items[i], nextItem) or (await items[i].getId()) !== (await nextItem.getId()).

mzgoddard · 2021-11-04T17:36:05Z

test/tests/treeview_treeview-1a.js

+);
+
+ariaTest(
+  'aria-selected attribute on treeitem on down arrow and enter',


I think this test tries to test too many behaviors in a single test and that one of the behaviors should also be tested with Key.SPACE.

So maybe this should be tested with three tests? One to test that selection does not follow focus in response to Key.DOWN_ARROW. One to test that Key.ENTER selects on each treeitem. One to test that Key.SPACE selects on each treeitem.

The 'does not follow focus' test could be something like:

ariaTest( 'aria-selected attribute does not follow focus', exampleFile, 'treeitem-aria-selected-not-focus', async (t) => { t.plan((ex.treeitemCount - 1) * 2); const items = await t.context.queryElements(t, ex.treeitemSelector); // Test all but the last treeitem. The last treeitem does not lose focus when `Key.DOWN_ARROW` is pressed. const itemsExcludeLast = items.slice(0, items.length - 1); for (let i = 0; i < itemsExcludeLast.length; i++) { const treeitem = itemsExcludeLast[i]; await treeitem.sendKeys(Key.ENTER); await treeitem.sendKeys(Key.ARROW_DOWN); const focusedItem = await t.context.queryElement( t, ex.treeitemSelector + '[tabindex="0"]' ); t.is( await treeitem.getAttribute('aria-selected'), 'true', 'selected treeitem does not change after sending ARROW DOWN' ); t.is( await focusedItem.getAttribute('aria-selected'), 'false', 'focused treeitem does not have aria-selected="true"' ); } } );

mcking65 · 2021-11-08T06:36:24Z

@mzgoddard

If the tree does not support multiple selection, aria-selected is set to true for the selected node and it is not present on any other node in the tree.

With the changes made in ARIA 1.3 and already implemented in browsers, that statement in the pattern will need to change. I'll create a separate PR for that. If selection does not follow focus, like in this particular tree, then it is necessary to specify aria-selected on all selectable nodes. Otherwise, the browser might assume that selection follows focus and provide an implicit selected state to the focused treeitem.

mcking65 · 2021-11-08T07:07:13Z

@mzgoddard pointed out this aspect of the pattern:

When a single-select tree receives focus:

If none of the nodes are selected before the tree receives focus, focus is set on the first node.

If a node is selected before the tree receives focus, focus is set on the selected node.

This example is not putting tabindex 0 on the item that is selected by pressing enter.

In the navigation tree, we not only ensure the current item is kept in the tab sequence but also ensure that it is visible after blur.

mcking65 · 2021-11-08T07:12:28Z

The example might be more clear if:

The label on the tree were "Select folder or file to view". Currently, the tree is labeled "File viewer", but the tree is not a viewer; it is a chooser.
The label on the read-only edit field were "Selected folder or file:" Putting the word "selected" first may improve clarity.

mcking65 · 2021-11-08T07:14:05Z

I noticed that enter both expands and selects a parent node. Shouldn't Enter only select?

mcking65 · 2021-11-08T15:23:02Z

I've started drafting updated selection guidance for the tree pattern in #2121.

I still have to add a note advising against using both selected and checked in the same tree and a note about when browsers imply selection. I'm hoping I can find time to finish it later tonight.

a11ydoer · 2021-11-09T19:12:07Z

APG meeting on Nov 9, 2021 (Note: plan to merge this tomorrow morning.)

change label on tree to be "My Documents", not "File viewer" The heading of "File Viewer" can stay.
add a visual selection state - refer the visual example of navigation tree
remove expanding/collapsing behavior from enter key(make it consistent with other APG patterns)

mcking65 · 2021-11-13T23:12:32Z

@howard-e

FYI ... I was mistaken when I said the label change was good. I didn't actually look at this until I was editing other parts of the file:

<h3 id="tree1" aria-label="My Documents">File Viewer</h3>

Except in some very rare circumstances, none of which come to mind at the moment, it is not good practice to put aria-label on a heading. Headings get their name from content, and the aria-label overrides the name from content. Thus, in this case, screen reader users will think the heading is "My Documents", but the words on screen are "File Viewer".

I was imagining that we would keep the "File Viewer" heading as a heading on the entire example (tree + edit field). And, I thought @smhigley had suggested adding a label of "My Documents" for just the tree. Instead, I just changed the heading to "My Documents" and removed the aria-label. This may have been what @smhigley was suggesting anyway. I may have initially misinterpreted her suggestion.

BTW, for a more extensive description about name from content, which types of elements are named from content, and the dangers of aria-label, have a look at Section 5.3.2.1: Naming with Child Content.

mcking65 · 2021-11-13T23:14:28Z

@jongund or @smhigley
I think I have this in a good place to ship. Would be nice if either of you could take a look and change to approving review.

mcking65

Finished editorial changes; this is approving editorial review.

jongund

The icons used to represent the open and closed folders and a document in the declared properties example are broken, they work in the computed example.
The documentation for the space key opening and closing a file folder is not documented in either example, and the APG guidance section only discusses the space key in the context of multi-selectable trees. It is also curious to note the behavior of the space and enter keys is different when focus is on a folder node, space opens and closes, enter does not.

jongund · 2021-11-15T15:32:48Z

@mcking65 @howard-e I added a review, a few more things I think need to be addressed.

mcking65 · 2021-11-15T15:39:46Z

@jongund wrote:

The icons used to represent the open and closed folders and a document in the declared properties example are broken, they work in the computed example.

@howard-e can you fix this this morning?

The documentation for the space key opening and closing a file folder is not documented in either example, and the APG guidance section only discusses the space key in the context of multi-selectable trees. It is also curious to note the behavior of the space and enter keys is different when focus is on a folder node, space opens and closes, enter does not.

Behavior of space should be identical to Enter, which is that it should only select -- perform the default action.

@howard-e can you change space key behavior so it does not open and close parent nodes? It should perform selection only, just like Enter.

howard-e · 2021-11-15T15:50:27Z

Sure, I'll take a look @mcking65

… example

jongund · 2021-11-15T16:20:42Z

@mcking65 @howard-e
Approved this pull request

… RETURN and updated regression tests

howard-e · 2021-11-15T16:56:17Z

Approved this pull request

Thanks @jongund.

can you change space key behavior so it does not open and close parent nodes? It should perform selection only, just like Enter

Also just pushed the final change to account for @mcking65's request and updated the relevant regression tests.

smhigley · 2021-11-15T18:49:56Z

test/tests/treeview_treeview-1a.js

+        'treeitem should have aria-selected="true" after selecting by sending key ENTER'
+      );
+      // move focus to the next item
+      await items[i].sendKeys(Key.ARROW_DOWN);


Why this line?

Set aria selected to active tree view item

a8ebc01

Address: w3c#1680 When an item receives a tab index of 0, then the aria-selected flag also turns to true.

a11ydoer requested a review from mcking65 June 22, 2021 18:54

smhigley requested changes Jun 29, 2021

View reviewed changes

mcking65 added this to the 1.2 Release 1 milestone Oct 18, 2021

zcorpan reviewed Oct 29, 2021

View reviewed changes

examples/treeview/treeview-1/js/tree.js Outdated Show resolved Hide resolved

Apply suggestions from code review

2d96a4b

Add aria-selected=false to all treeitems

e93dbfd

Add aria-selected documentation and tests (WIP, test is failing)

9987e3f

zcorpan mentioned this pull request Nov 3, 2021

Alternatives to adding a PR comment with the output of coverage-report.yml workflow #1811

Open

zcorpan closed this Nov 3, 2021

zcorpan reopened this Nov 3, 2021

Update the implementation to set aria-selected onclick and fix tests

fbaacd6

zcorpan force-pushed the Issue-1680-add-aria-selected-to-treeview-items branch from 00cb024 to fbaacd6 Compare November 3, 2021 15:49

mzgoddard reviewed Nov 4, 2021

View reviewed changes

mcking65 changed the title ~~Set aria selected to active tree view item~~ File Viewer Tree Examples: Add aria-selected Nov 8, 2021

howard-e added 2 commits November 9, 2021 19:23

added "My Documents" label for "File Viewer" heading

63f4875

stop ENTER from expanding/collapsing tree; support selecting with ENTER

907999f

mcking65 added 9 commits November 13, 2021 13:05

Improve intro

356bcc5

Improve intro

4b337c7

editorial revisions to keyboard documentation

cbb4486

Update states and properties documentation

3dc9bfa

Replace named entity for quote with quote char inside code tag elements

72d3334

Change IDREF to ID_REFERENCE

58d6da7

fix white space

8fa0ee6

Remove aria-label from h3 element

583e944

Add info about selection not following focus to kb documentation section

8d649b5

mcking65 approved these changes Nov 13, 2021

View reviewed changes

jongund requested changes Nov 15, 2021

View reviewed changes

File Viewer Tree Example: fix for broken icons on declared properties…

f623ddb

… example

howard-e force-pushed the Issue-1680-add-aria-selected-to-treeview-items branch from 8c0e9fb to f623ddb Compare November 15, 2021 16:12

jongund approved these changes Nov 15, 2021

View reviewed changes

File Viewer Tree Example: behavior of SPACE changed to be the same as…

fa2b72c

… RETURN and updated regression tests

smhigley approved these changes Nov 15, 2021

View reviewed changes

mcking65 merged commit 1c89267 into w3c:main Nov 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

File Viewer Tree Examples: Add aria-selected #1869

File Viewer Tree Examples: Add aria-selected #1869

kdoberst commented Apr 23, 2021 •

edited by mcking65

Loading

a11ydoer commented Jun 21, 2021 •

edited

Loading

smhigley left a comment

smhigley Jun 29, 2021

zcorpan Oct 29, 2021

mcking65 commented Oct 31, 2021

a11ydoer commented Nov 2, 2021 •

edited

Loading

zcorpan commented Nov 2, 2021 •

edited

Loading

jongund commented Nov 2, 2021

a11ydoer commented Nov 2, 2021

zcorpan commented Nov 3, 2021 •

edited

Loading

mzgoddard left a comment

mzgoddard Nov 4, 2021

mzgoddard Nov 4, 2021

mzgoddard Nov 4, 2021

mcking65 commented Nov 8, 2021

mcking65 commented Nov 8, 2021

mcking65 commented Nov 8, 2021

mcking65 commented Nov 8, 2021

mcking65 commented Nov 8, 2021

a11ydoer commented Nov 9, 2021 •

edited by mcking65

Loading

mcking65 commented Nov 13, 2021

mcking65 commented Nov 13, 2021

mcking65 left a comment

jongund left a comment •

edited

Loading

jongund commented Nov 15, 2021

mcking65 commented Nov 15, 2021

howard-e commented Nov 15, 2021

jongund commented Nov 15, 2021

howard-e commented Nov 15, 2021

smhigley Nov 15, 2021

File Viewer Tree Examples: Add aria-selected #1869

File Viewer Tree Examples: Add aria-selected #1869

Conversation

kdoberst commented Apr 23, 2021 • edited by mcking65 Loading

a11ydoer commented Jun 21, 2021 • edited Loading

smhigley left a comment

Choose a reason for hiding this comment

smhigley Jun 29, 2021

Choose a reason for hiding this comment

zcorpan Oct 29, 2021

Choose a reason for hiding this comment

mcking65 commented Oct 31, 2021

a11ydoer commented Nov 2, 2021 • edited Loading

zcorpan commented Nov 2, 2021 • edited Loading

jongund commented Nov 2, 2021

a11ydoer commented Nov 2, 2021

zcorpan commented Nov 3, 2021 • edited Loading

mzgoddard left a comment

Choose a reason for hiding this comment

mzgoddard Nov 4, 2021

Choose a reason for hiding this comment

mzgoddard Nov 4, 2021

Choose a reason for hiding this comment

mzgoddard Nov 4, 2021

Choose a reason for hiding this comment

mcking65 commented Nov 8, 2021

mcking65 commented Nov 8, 2021

mcking65 commented Nov 8, 2021

mcking65 commented Nov 8, 2021

mcking65 commented Nov 8, 2021

a11ydoer commented Nov 9, 2021 • edited by mcking65 Loading

mcking65 commented Nov 13, 2021

mcking65 commented Nov 13, 2021

mcking65 left a comment

Choose a reason for hiding this comment

jongund left a comment • edited Loading

Choose a reason for hiding this comment

jongund commented Nov 15, 2021

mcking65 commented Nov 15, 2021

howard-e commented Nov 15, 2021

jongund commented Nov 15, 2021

howard-e commented Nov 15, 2021

smhigley Nov 15, 2021

Choose a reason for hiding this comment

kdoberst commented Apr 23, 2021 •

edited by mcking65

Loading

a11ydoer commented Jun 21, 2021 •

edited

Loading

a11ydoer commented Nov 2, 2021 •

edited

Loading

zcorpan commented Nov 2, 2021 •

edited

Loading

zcorpan commented Nov 3, 2021 •

edited

Loading

a11ydoer commented Nov 9, 2021 •

edited by mcking65

Loading

jongund left a comment •

edited

Loading