Skip to content
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.

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

Keyboard and ARIA accessibility of tree grid #709

Closed
aleventhal opened this issue Mar 23, 2017 · 47 comments
Closed

Keyboard and ARIA accessibility of tree grid #709

aleventhal opened this issue Mar 23, 2017 · 47 comments

Comments

@aleventhal
Copy link
Contributor

aleventhal commented Mar 23, 2017

While the accessibility of a normal tree is in very good shape, the treegrid is a little bit of a problem for screen reader users (although it has a great start).

The treegrid only puts focus on the first cell of each column. This means that as the user navigates up and down between rows, the user only hears the information in the first column, and it isn't even necessarily clear that there are other columns. Solving this is not necessarily simple.

I don't think there have necessarily been a lot of great accessible treegrid widgets made in the past, so we are breaking a little ground here. This may take a couple of rounds and testing with some users to get it right. I can help with the testing and ARIA recommendations, etc.

What do you think about having a separate row navigation mode and column navigation mode?

Row navigation mode:

  • When pressing up/down, the user should move into row navigation mode. In this mode, focus/aria-activedescendant should go on the row, so that the entire row is read aloud. These are the role="row" items.
  • aria-level, aria-setsize and aria-posinset should be set on the row so the screen reader user hears correctly the information "Level 3, item 2 of 7" etc.

Column navigation mode:

  • When pressing ctrl+left/ctrl+right (cmd left/right on mac) the user will move left/right by cells but restricted to the current row. These are the role="gridcell items"

Other:

  • No aria-labelledby is necessary, because when there is no aria-labelledby/aria-label, the child text content is used by default. Note: this is true for "tree" mode as well.
  • In a treegrid, no role="treeitem" elements are necessary -- just the rows and cells
  • On each row (also on treeitem in a tree), aria-expanded should be set to "true" or "false" on parent nodes. This attribute should be removed for leaf nodes. Therefore it's really a tristate attribute. Setting it to false indicates that the node is expandable, whereas leaving it off indicates that the node is not expandable.
  • The aria-selected attribute is another tristate (true|false|undefined). On each row, aria-selected should be set only if and only if the node is selectable. If checkboxes/selection are not available, it should be removed. Mixed checkboxes can be defined as such by using aria-checked="mixed".
  • The aria-owns may need to be set, but I need to look at what different browsers are doing with this , and it may be harmful
@aleventhal
Copy link
Contributor Author

The keyboard navigation model is just my current suggestion. There is no major precedent for it, so it's possible the community will decide that it's not ideal. However, having row navigation and focus be the default seems right, otherwise the user will miss important information.

@aleventhal
Copy link
Contributor Author

I'm following up with some colleagues and we have some other ideas that might work really well. So let's hold off just a bit until I get more thoughts together on this!

@mar10
Copy link
Owner

mar10 commented Mar 24, 2017

Thanks for taking the time!

Another aproach is described in #655. If I understand correctly, this is a recommended for WAI treegrids,
but I was hesitant to implement this, since it seemed too far away from the current behavior.
(It may be aded as separate extension, however.)
The pragmatic aproach you are describing seems reasonable to me.

The discussion result could be documented in a wiki document, like this (outdated).

Even if I may not be able to implement all of it immediately: I consider the ARIA feature very important, and really apreciate your input.

@mar10 mar10 added the aria label Mar 24, 2017
@aleventhal
Copy link
Contributor Author

I've started to engage the WAI ARIA authoring guide on this. See w3c/aria-practices#91 -- I'm planning to work up a proposal.

@mar10
Copy link
Owner

mar10 commented Apr 1, 2017

Great!

@aleventhal
Copy link
Contributor Author

Here's an example I'm showing around now:
https://cdn.rawgit.com/aleventhal/treegrid-tab-active-cells/0.1.10/treegrid.html

What do you think?

@clozinski
Copy link

I love it.
I think that there is too much noise on the web. Things needs to be organized as a hierarchy. That can be expanded or contracted as needed.

Take a look at how I am doing it.
http://WhyPoland.info

Personally I am amazed that way more people are not using FancyTree.
Let alone ZODB.

Chris

@mar10
Copy link
Owner

mar10 commented Apr 15, 2017

@aleventhal nice work!

I haven't yet tried it with a screen reader, but some unstructured thoughts:

  • if a row with an collapsed node is active, I cannot enter cell mode directly,
    because RIGHT will expand. Only the next RIGHT will mark the cell.
    Expanding may not be what I want, since it can create lots of new rows.
  • Tabbing out of the last column will leave the whole widget.
    Users might expect to focus the next item in the following row?

I am not sure if this assumption always holds:

treegrids don't have that many columns

(But maybe if they have, they may be hard to make accessible :-/)

Anyway, the specification and demo should include concrete guidelines how to handle
input controls.
So a scenario like in the 'gridnav' demo should be possible:
http://wwwendt.de/tech/fancytree/demo/sample-multi-ext.html

I am not saying that gridnav is the 'best' approach, of course. It just was handy in
the case of a grid that consisted of mainly checkboxes and some dropdowns:

  • While a grid of checkboxes can be freely navigated in all directions using cursor keys.
    For text-inputs LEFT/RIGHT will stay in that control and only UP/DOWN will change the row.
    For dropdowns, UP/DOWN will select from the list, while LEFT/RIGHT changes the columns.
  • The title column is tabable. (Looking at the sample, I realize, that the
    Fanctree checkbox in the leftmost column should probably be tabable as well...).
  • Only if the title column has focus, LEFT/RIGHT will expand / collapse
  • SPACE is also aware of the active column, since it modifies an edit control,
    selects a checkbox, or toggles the fancytree row selection respectively.
  • TAB will move to the next tabbable element (which may me the first element in the next row).

Again, I don't propose that this for the specification, just add it as a
potential use case.

Some unstructrued questions for discussion:

  • Should a treegrid feel like Excel concerning cursor keys,
    i.e. should every cell be reachable using UP/DOWN/LEFT/RIGHT?
  • How should merged or empty cells be handled (i.e. focusable?)
  • Should disabled checkboxes be skipped?
  • Should TAB navigation include all rows or only the active row?
  • Should keystrokes like 'a', '*', '+', SPACE modfy active, interactive cells,
    or be ignored (or handled as tree commands, like 'expand all' or 'select row')?
  • Should UP/DOWN change an active dropdown selection immediately, or should
    the user enter an edit mode with F2 for example?
    Similar: should an ascii key stroke edit the current text control?
  • What happens if one cell contains multiple html input controls?

An alternative approach that we could investigate is to have a separate 'edit-mode'
that can be activated.
For example F2 could switch from a row-based read-only mode to a cell based
edit mode.
In row mode

  • LEFT/RIGHT expands
  • UP/DOWN changes row
  • SPACE (de)selects a row
  • TAB leaves the treegrid
    In edit mode
  • LEFT/DOWN/RIGHT/UP navigates cells
  • SPACE selects a checkbox if in current cell
  • TAB moves to next input (even if in next row)
  • Special keys like '+'/'-' are required to expand / collapse
    May also depend on the currently active cell
  • Ascii enters text input or selects dropdown
  • F2 focuses dropdown or text input, so that cursor keys are handled by the
    controls.
    It can also put a node title into rename mode
  • ESC ends edit mode of the grid (or cancels 'rename')

If often used a large treegrid with lots of controls as main component of a page.
Editing of the structure was the main purpose, so it should be possible to
define the edit mode as 'always on'?
So I am not convinced myself that this would be a perfect solution.

@aleventhal
Copy link
Contributor Author

aleventhal commented Apr 17, 2017

if a row with an collapsed node is active, I cannot enter cell mode directly,
because RIGHT will expand. Only the next RIGHT will mark the cell.
Expanding may not be what I want, since it can create lots of new rows.

We could introduce new keystrokes that separate expand vs. next column, but it would not be discoverable. An earlier prototype used the move-by-word keys to switch columns. We could add them back in as well. It just would not be discoverable, so I'm not sure it's worth it.

Tabbing out of the last column will leave the whole widget.
Users might expect to focus the next item in the following row?

I like it this way because you can arrow to the next row and then tab, and there are a reasonable number of items in the tab order. If tab went to the next row, the user would not be able to escape to the next widget in the case of 1000s of rows.

I am not sure if this assumption always holds:

treegrids don't have that many columns

An earlier version needed this assumption because there was not an easily discoverable move by cell command. However, with the addition of right arrow and tab, I feel that we can remove this as an assumption.

Anyway, the specification and demo should include concrete guidelines how to handle
input controls.
So a scenario like in the 'gridnav' demo should be possible:
http://wwwendt.de/tech/fancytree/demo/sample-multi-ext.html

Agreed. I'll add that in at some point.

Looking at your gridnav example, the only issue I have with it is that the title column is tabbable. IMO that's really a problem because it's common to have many many rows, and so effectively you are locking users into that widget. I prefer the solution where tabbing only works on the active row, and arrows move to other rows.

I am not saying that gridnav is the 'best' approach, of course. It just was handy in
the case of a grid that consisted of mainly checkboxes and some dropdowns:

Yeah, I'm not saying my approach is necessarily the best either, but I'm trying to get consensus within the ARIA working group too. With lots of different ideas that may be the most difficult thing about this problem

While a grid of checkboxes can be freely navigated in all directions using cursor keys.
For text-inputs LEFT/RIGHT will stay in that control and only UP/DOWN will change the row.
For dropdowns, UP/DOWN will select from the list, while LEFT/RIGHT changes the columns.
Seems reasonable to me.

Only if the title column has focus, LEFT/RIGHT will expand / collapse
SPACE is also aware of the active column, since it modifies an edit control,
selects a checkbox, or toggles the fancytree row selection respectively.

Do you think it's better than having the default be row focus? To me that's easier to understand. I'm not sure if users will understand that there is a special column that does different things.

Some unstructrued questions for discussion:

Should a treegrid feel like Excel concerning cursor keys,
i.e. should every cell be reachable using UP/DOWN/LEFT/RIGHT?

Yes, although as you pointed out a textfield or select may take some of those keys

How should merged or empty cells be handled (i.e. focusable?)

Merged cells - focus them as a group or the first cell of the group
Empty cells - yes, definitely focus.

Should disabled checkboxes be skipped?

I'd say reachable via arrows but not tabs.

Should TAB navigation include all rows or only the active row?

IMO active row because of the many rows tabbing problem

Should keystrokes like 'a', '*', '+', SPACE modfy active, interactive cells,
or be ignored (or handled as tree commands, like 'expand all' or 'select row')?

IMO it depends on row vs. cell mode

Should UP/DOWN change an active dropdown selection immediately, or should
the user enter an edit mode with F2 for example?

I don't think users will know about F2 or other keys, but maybe Enter. It's probably ok if they just drop down immediately.

Similar: should an ascii key stroke edit the current text control?

I would say just start editing

What happens if one cell contains multiple html input controls?

We explode! Actually I suppose they can all easily be in the tab order for the active row.

An alternative approach that we could investigate is to have a separate 'edit-mode'
that can be activated.
For example F2 could switch from a row-based read-only mode to a cell based
edit mode.

I don't like using keys that no one else uses. No one knows about them and you also run the risk of conflicting with browser, OS or web app keys.

Are we at all close to finding a common solution? :)

@mar10
Copy link
Owner

mar10 commented Apr 17, 2017

Are we at all close to finding a common solution? :)

There's only one way to find out ;)
I can draft a protoype extension that follows your spec during the next days, so we can play with it in addition to your example implementation.

@aleventhal
Copy link
Contributor Author

aleventhal commented Apr 17, 2017 via email

@aleventhal
Copy link
Contributor Author

aleventhal commented Apr 17, 2017 via email

@aleventhal
Copy link
Contributor Author

I have a grand unified theory of treegrids for everyone. I'd love some feedback. Same code, different focus options. It turns out that sometimes people want a treegrid that's more like a tree, and sometimes they want it to behave more like a grid. The difference seems to be whether the rows focus by default, and whether rows can get focus at all.

This is the same code with a different value for the URL param called cell:
Start focus on rows: https://cdn.rawgit.com/aleventhal/treegrid-tab-active-cells/0.1.15/treegrid.html
Start focus on cells: https://cdn.rawgit.com/aleventhal/treegrid-tab-active-cells/0.1.15/treegrid.html?cell=start
Start and keep focus on cells (disallow row focus): https://cdn.rawgit.com/aleventhal/treegrid-tab-active-cells/0.1.15/treegrid.html?cell=force

Internally, the URL parameter sets the boolean variables doStartRowFocus and doAllowRowFocus. There is hardly any branching logic required.

The aria-expanded attribute is mirrored from the row to the first cell. Pressing Enter on the first cell is another way of toggling expansion.

@mar10
Copy link
Owner

mar10 commented Apr 20, 2017

Thats great news.
I started a branch with a new protoype extension for Fancytree, (ext-ariagrid) and it is basically working already.

I will implement your latest changes to the spec, and we can have an option to toggle the mode as well.

There are still some problems with focus behavior when cells contain embedded checboxes or text inputs in combination with cursor keys, tab key and/or clicking. Hope to fix this this until next weekend, so we have another prototype evaluate. I think I can simply add this to the trunk and put it online, since hardly any changes were needed in the core code.

@mar10
Copy link
Owner

mar10 commented Apr 20, 2017

btw. one idea I experimented with was to allow ESC to switch from cell mode back to row mode. What do you think?

(and maybe ENTER to switch from row- to cell-mode, but I am not so sure here)

@aleventhal
Copy link
Contributor Author

aleventhal commented Apr 20, 2017

Awesome. I imagine a config option called cellFocus with the following values:

  • 'allow' (default)
  • 'start'
  • 'force'

Here are some ARIA recommendations:

  • Use aria-level, aria-posinset, aria-setsize on each row
  • Use aria-expanded on the row except in the cellFocus=force case, in which case it should be on the title cell
  • Make sure the aria-expanded is not present when the row has no children
  • Mirror aria-expanded from each row to the cell with the expander
  • Don't use aria-labelledby. This will be computed automatically by the browser. True in trees as well.
  • When the tree/grid is multiselectable, use aria-multiselectable="true" on the tree/treegrid element
  • When a cell has a single focusable widget, the aria-activedescendant should point to the widget instead of the parent
  • If rows are hidden I suggest aria-hidden="true" on them (may be optional)
  • I suggest removing the ARIA option and just turning it on always. It should have a negligible effect on performance, and most authors won't think of turning it on, yet they don't know much about screen reader use or whether a future user will need it. Think of how sad that would be for a screen reader user to encounter a Fancytree and not be able to use it because ARIA was off.

Keyboard:

  • Escape is not the best idea for focusing the row, because screen readers unfortunately use that to exit forms mode. Yuck.
  • Enter key should continue to perform the default action on the cell. If there is no other default action on the cell and it contains an expander, then Enter should toggle expansion.
  • Also, if you move past the first cell it should move to row focus if not in options.cellFocus !== 'force'
  • Do you deal with RTL content? Do you switch left/right arrow in that case?

Edits:

  • aria-expanded only on row or cell, not both
  • Escape key bad

@aleventhal
Copy link
Contributor Author

Getting closer to a spec / design pattern over here:
https://cdn.rawgit.com/w3c/aria-practices/treegrid/examples/treegrid/treegrid-row-nav-primary-1.html

I can review your branch when you're ready.

@mar10
Copy link
Owner

mar10 commented Apr 23, 2017

I merged the ext-ariagrid extension to master (example here, code here).

I have not yet implemented all of your recommendations above, so it is a bit too early for review - I'll let you know.
Just to make sure: you proposed to remove aria-labelledby even from the plain tree?
Should we add aria-posinset, -setsize, and -level to the tree as well, or are the readers smart enough to figure out ?

BTW. I agree that aria should be enabled by default, when the spec has settled and performance is not affected too much, so the extension may become part of ext-table later.

@aleventhal
Copy link
Contributor Author

Yes, we can remove aria-labelledby even from the tree view. Browsers have a "name from contents" rules that is utilized on elements of role treeitem. This is also true of role="row" although Chrome only does this correctly for row in recently nightly builds.

aria-posinset, aria-setsize and aria-level should be implemented for the treegrid, since all of the rows are at the same level. For the normal tree, there appears to be enough DOM structure that the browser can compute this on behalf of the screen reader. I will doublecheck that later and let you know if there's an issue with that.

Have you seen any performance issues caused by ARIA usage before?

@mar10
Copy link
Owner

mar10 commented Apr 24, 2017

I see, thanks.

Have you seen any performance issues caused by ARIA usage before?

I observed that small changes to the rendering logic sometimes had unexpected (by me) impact.
Havn't measured aria so far, just mentioned it, because performance with a high number of nodes is one important requirement. We can simply benchmark and/or optimize, once the spec is implemented.

@aleventhal
Copy link
Contributor Author

aleventhal commented Apr 24, 2017 via email

@mar10
Copy link
Owner

mar10 commented Apr 24, 2017

Also usability for huge grids with embedded cells is an important use case for Fancytree.
I hope you don't mind if I brainstorm some more...

I am still a bit concerned about having to expand (potentially a lot of) nodes, just to switch to cell mode.
S.th. like enter/esc seemed natural to me.

Same with having to cursor to the boundaries just to switch back to row-mode, because I want to expand or collapse now.
Would it be valid for Fancytree to support + / -even if it is not defined in the spec?
(Maybe having a strict and relaxed aria mode)

Enter key should continue to perform the default action on the cell. If there is no other default action on the cell and it contains an expander, then Enter should toggle expansion.

Agreed, but in row-mode? Your current implementation defines alert column 3 as default action.
Could Fancytree simply define enter cell mode as default action on rows?

Escape is not the best idea for focusing the row, because screen readers unfortunately use that to exit forms mode. Yuck.

Too bad, I wasn't even aware of a forms mode.
I noticed in grid example 2 that ENTER/ESC is used to enter/exit edit mode on embedded inputs and dropdowns.

So rows could be handled as another layer of edibility, so that "cell-mode" means "edit-row"-mode and it seems to have a nice symmetry:

row-mode  =ENTER=>  cell-mode  =ENTER=>    edit-input
          <=ESC=               <=Esc/Enter

Any hope that Esccan be handled by screen readers this way, if W3C specifies so?

This would also help with the problem in my current implementation, where you can get stuck in a text-input when you move around with cursor keys. Then only Tab can get you out.
If however Enter was required to edit a cell input, you can simply run over it with cursor keys.
(Unlike in the grid example, I would allow to start editing by typing alphanumeric keys as well. Maybe in relaxed mode only)

As you can see, I am a complete greenhorn concerning screen reader behavior, so hopefully some of the above makes sense :)

@aleventhal
Copy link
Contributor Author

aleventhal commented Apr 24, 2017

Also usability for huge grids with embedded cells is an important use case for Fancytree.
I hope you don't mind if I brainstorm some more...

Sounds good.

I am still a bit concerned about having to expand (potentially a lot of) nodes, just to switch to cell
mode.
S.th. like enter/esc seemed natural to me.

Do you envision row mode being used for huge grids? Or is it more natural to use ?cell=force mode where there is no row focus? I feel like row mode is for grids that are more like trees -- usually fewer columns.
Some people wanted Enter to toggle row expansion. Then sometimes there is a default action like open this message. Then Fancytree might use it to go into cell mode. I guess my concern is if Enter always seems to do something different. I hear you, though.

Same with having to cursor to the boundaries just to switch back to row-mode, because I want to expand or collapse now.
Would it be valid for Fancytree to support + / -even if it is not defined in the spec?
(Maybe having a strict and relaxed aria mode)

Ultimately the authoring guide is a guide, and they are just suggestions to help the community deal with all the confusion in accessibility, by providing something that helps bring everyone together on the basics as much as possible. If someone comes up with a better way, they should use it as long as users can discover it and it doesn't confuse everyone by being completely different, and yet calling itself the same thing.

Enter key should continue to perform the default action on the cell. If there is no other default > action on the cell and it contains an expander, then Enter should toggle expansion.
Agreed, but in row-mode? Your current implementation defines alert column 3 as default action.
Could Fancytree simply define enter cell mode as default action on rows?

It could. Maybe it's better. FWIW all of these things are just suggestions, so if you like this better, maybe other people will too. Not that many people have weighed in yet. I'll see what people think of using Enter.

Escape is not the best idea for focusing the row, because screen readers unfortunately use that to
exit forms mode. Yuck.
Too bad, I wasn't even aware of a forms mode.
I noticed in grid example 2 that ENTER/ESC is used to enter/exit edit mode on embedded inputs and dropdowns.

Yes, it works in those 2 cases. I've been told those are exceptions that the screen readers allow because it's so expected.

So rows could be handled as another layer of edibility, so that "cell-mode" means "edit-row
"-mode and it seems to have a nice symmetry:

row-mode =ENTER=> cell-mode =ENTER=> edit-input
<=ESC= <=Esc/Enter
Any hope that Esccan be handled by screen readers this way, if W3C specifies so?

The problem is really the JAWS screen reader, which is like the IE of the accessibility world. Even if we get them to update JAWS, lots of people don't update. It doesn't help that the updates cost a lot of money. We try to avoid creating incompatibilities with current screen readers as a matter of pragmatism. It can suck. But when we get idealistic and try to make everything better, yet older stuff doesn't work, it also sucks.

This would also help with the problem in my current implementation, where you can get stuck in a > text-input when you move around with cursor keys. Then only Tab can get you out.
If however Enter was required to edit a cell input, you can simply run over it with cursor keys.
(Unlike in the grid example, I would allow to start editing by typing alphanumeric keys as well.
Maybe in relaxed mode only)

As you can see, I am a complete greenhorn concerning screen reader behavior, so hopefully some > of the above makes sense :)

Screen reader stuff is hard. I guess the keyboard navigation is really separate from screen reader stuff, so you are as good as anyone on that. Keep asking questions and we'll get somewhere. I like the Enter/escape idea unless there is a default action. I wish that Escape was not an issue for screen readers in the non textedit/popup cases. None of us like that. Maybe we'll find a workaround, I can try to find out more.

@mar10
Copy link
Owner

mar10 commented Apr 24, 2017

Thanks for taking the time to answering and sharing some background. I ping you when I have s.th. testable (please allow some time for me to implement).

@aleventhal
Copy link
Contributor Author

Hi @mar10, let me know if there's any way I can help.

@mar10
Copy link
Owner

mar10 commented Apr 30, 2017

Hey @aleventhal ,

I finally found some time to work on the prototype. There are still some problems when navigating in the different modes with Cursor-Keys, Tab-Keys, and mouse clicks, but if you like, you could give some early feedback.
http://wwwendt.de/tech/fancytree/demo/sample-aria-treegrid.html

I added the cellMode option and a new extendedMode option, so we can play with some additional ideas in the same code base.
There is also a newdefaultCellAction callback.

p.s.
(could you point me to the latest specs, some of the rawgit links are broken for me.)

@aleventhal
Copy link
Contributor Author

Thanks @mar10 -- here's an updated rawgit link:
https://rawgit.com/w3c/aria-practices/treegrid/examples/treegrid/treegrid-1.html

@aleventhal
Copy link
Contributor Author

aleventhal commented May 3, 2017

Hi, I checked out the example.

Here are my suggestions for markup improvements, from most important to least:

  • The aria-activedescendant must point to the row when a row has focus, and point to a gridcell when a cell has focus. This is critical.
  • Don't put role="treeitem" anywhere in a treegrid, they are currently on
    a grandchild of the gridcell. The treeitem should only be used in a tree
  • Don't put role="button" on expanders if they are not visible (leaf nodes)
  • For role="img" on folders:
    • Are they important to understanding the line? I'm not sure they are given that
      we can use aria-expanded to say whether something is a folder. Therefore I suggest
      to put role="presentation" instead
    • If it is really is important for understanding the line, add an aria-label to it so the screen reader knows what the img means

Would you like me to help with the code? I'd be happy to.

@mar10
Copy link
Owner

mar10 commented May 3, 2017

Thanks for looking into it!

The aria-activedescendant must point to the row when a row has focus, and point to a gridcell when a cell has focus. This is critical.

Do we need to render IDs on all rows and cells or can we simply use a constant ID aria-activedescendant=X and then move the id=X attribute around? This would be easy, but I have a feeling it would violate some specs?

Concerning the 'button' and 'img' roles, I did not know what I was doing there, probably seen it somewhere else. We should change as you suggested.

Would you like me to help with the code? I'd be happy to.

sure, very welcome!

@mar10
Copy link
Owner

mar10 commented May 4, 2017

@aleventhal fyi, I fixed and uploaded your first two suggestions...

@aleventhal
Copy link
Contributor Author

Do we need to render IDs on all rows and cells or can we simply use a constant ID aria-
activedescendant=X and then move the id=X attribute around? This would be easy, but I have a
feeling it would violate some specs
You could probably do something like:

  1. Create a new id on the new row to focus
  2. Point aria-activedescendant at the new id
  3. Remove the id from the old row

You could use a counter or even alternate between 2 ids this way.

@aleventhal
Copy link
Contributor Author

Excellent: I tried master with NVDA and Chrome Canary, and got much better results than before. Obviously there is still polish but this works well enough that once it's released, I can start testing automation-inspector with my colleagues that use screen readers.

(It's at https://github.com/google/automation-inspector if you're curious.)

@aleventhal
Copy link
Contributor Author

It's good enough that I would love to use it as soon as I can. What do you still need to do before you are ready to release it?

@mar10
Copy link
Owner

mar10 commented May 5, 2017

I released as patch, since still unofficial and I want to merge another branch into the next feature release. Let me know how it works out :)

@aleventhal
Copy link
Contributor Author

aleventhal commented May 5, 2017

We are closer, but I believe the regular ARIA tree is not working as well as it used to. In a regular tree, the aria-activedescendant is pointing to something like a <span id="ui-id-5" class="fancytree-node ...">. It has a last child of <span class="fancytree-title" role="treeitem" aria-selected="blah">. The role and aria-selected attributes should be set on the parent .fancytree-node element, where the aria-activedescendant is pointing.

@mar10
Copy link
Owner

mar10 commented May 5, 2017

I probably broke that recently, but I think before active-descendant was pointing to <span class="fancytree-title" , not <span class="fancytree-node. Will the readers understand that the node's title is in a sub-element, even if active-descendant points to the parent?

@aleventhal
Copy link
Contributor Author

For some roles, the ones marked NameFrom contents in the spec, the reader will read all the descendant text and labels. So yes, the screen reader will get the right thing here. Let's point it to the parent.

@mar10
Copy link
Owner

mar10 commented May 5, 2017

Did it, and moved aria-selected, and aria-expanded from the <li> to the node span as well.
Also modified 'button' and 'img' roles as suggested.
You can view here and here, before I release a new patch.

@mar10
Copy link
Owner

mar10 commented May 6, 2017

I released the current state as patch (I will be mostly offline next week)

@aleventhal
Copy link
Contributor Author

aleventhal commented May 7, 2017

We are very very close. Here are some recommended improvements:

  • Improvements for aria-expanded:
  1. Treegrid: Currently when something has children but is not expanded, we don't always set aria-expanded, but it always should be aria-expanded="false" (which means expandable but not currently expanded). This is different from not having the attribute present (which means not expandable). It seems to work correctly on your example, but in mine, I have to expand and then collapse a node to get aria-expanded="false" on it.
  2. Tree: Currently aria-expanded="true" is being set on leaves, but it should be left out there. This seems to be due to the logic for removing aria-expanded assuming that leaves are not expanded, but the data model treats leaves as expanded?
  • The setsize/posinset is no longer getting reported in the plain tree. This probably used to work via the browser autocomputing it for us, since you have a nice nested li structure. However, now that the the focus is going on the child span, it won't help us, since the autocomputed attribute is showing up on the parent. Two options:
  1. Put aria-setsize and aria-posinset attributes on the span, similar to how we do it for treegrid.
  2. Or, point the aria-activedescendant to the li and move all the other ARIA attributes that are currently on the span child up one level to the li.

My apologies if I led you down the wrong path on that before.

@aleventhal
Copy link
Contributor Author

aleventhal commented May 7, 2017

Since you're going to be traveling, here's a PR for theses issues:
#732

@mar10
Copy link
Owner

mar10 commented May 11, 2017

Thank you Aaron, and sorry for the delayed response!

ad A.1.
I tried your /src patch locally, but it does not seem to handle lazy nodes correctly (see my review comments). I did not test your changes to the /dist branch however, which seems to be different, but will be overridden by the build step.
Could you check that please?
(Testing for hasChildren() !== false should work, since it returns undefined for lazy nodes, and we want to treat lazy nodes as if they had children.)

ad A.2.
I admit that the 'expanded' flag in the data model is a bit confusing. It may be set on empty nodes as well, because (in OS X Finder for example) even empty folders may be expanded. I wanted to make this behavior possible some time, even if the current implementation will hide the expanded icon in that case.
For now the aria-expanded attribute should match the visible status of the expander icon, i guess (expanded, collapsed, absent)? So if your patch works, it's fine.

ad B.
I realized that in ChromeVox the rows got reported as "N of M", but the M was the total number of rows, not the sibling count. I never know, what I can expect from the reader software. Great, if your patch fixes that.

My apologies if I led you down the wrong path on that before.

No problem; we should go as many rounds as it takes to find the optimum :)

@aleventhal
Copy link
Contributor Author

I'll take care of the Chromevox experience with any necessary patches, so no worries there. I haven't tested there recently as I've been waiting for some of the fixes to make it into the developer channel. I'll probably need to make more patches to Chrome and Chromevox itself in the end.

@aleventhal
Copy link
Contributor Author

@mar10, the PR is updated.

@mar10
Copy link
Owner

mar10 commented May 11, 2017

@aleventhal merged, thanks.

@mar10 mar10 mentioned this issue Sep 9, 2017
28 tasks
@stale
Copy link

stale bot commented Oct 22, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Oct 22, 2019
@mar10 mar10 added stale Flagged by stalebot and removed wontfix labels Oct 22, 2019
@stale stale bot removed the stale Flagged by stalebot label Oct 22, 2019
@stale
Copy link

stale bot commented Jan 20, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Flagged by stalebot label Jan 20, 2020
@stale stale bot closed this as completed Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants