Skip to content

Settings Editor: Add aria labels for input elements Fixes: #54836#55543

Merged
cleidigh merged 1 commit intomicrosoft:masterfrom
cleidigh:settings-aria-labels1/tweak
Aug 1, 2018
Merged

Settings Editor: Add aria labels for input elements Fixes: #54836#55543
cleidigh merged 1 commit intomicrosoft:masterfrom
cleidigh:settings-aria-labels1/tweak

Conversation

@cleidigh
Copy link
Contributor

@cleidigh cleidigh commented Aug 1, 2018

@roblourens

This addresses labels for input elements as best as possible for elements within a tree.
My research suggests interactive elements within a tree are not formally supported
but at least NVDA does its best.

I believe will have to revisit this with more information, perhaps we need to use treegrid or
grid both of which explicitly support interactive elements. These might need to be added as options
for list and tree widgets

I will also work at adding support for the links within descriptions but I will wait till after this milestone

No response yet on my question on this for NVDA

@cleidigh cleidigh self-assigned this Aug 1, 2018
@cleidigh cleidigh added accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues settings-editor VS Code settings editor issues labels Aug 1, 2018
@cleidigh cleidigh added this to the July 2018 milestone Aug 1, 2018
@cleidigh cleidigh merged commit 2ba4c71 into microsoft:master Aug 1, 2018
@roblourens
Copy link
Member

Thanks! Yeah I'm still asking around, I think we are on our own to figure this out though...

I think using treegrid or grid would be fine if you know how to do that.

@cleidigh
Copy link
Contributor Author

cleidigh commented Aug 1, 2018

the ambiguities are really frustrating

we will have to change the widgets or implement something separately to use any of the grid
approaches as these affect all levels of the implementation especially as it pertains to labeling
I'm going to keep researching I think this will affect other things as well.

what's more important is to get the navigation in good order, I think I have seen some other errors
traversing the settings I will document as I find them

@cleidigh cleidigh deleted the settings-aria-labels1/tweak branch August 1, 2018 03:27
@roblourens
Copy link
Member

I just got another tip about someone who can possibly help... I'll keep you in the loop.

@roblourens
Copy link
Member

To give you a quick update on this - I am thinking about #54039 and prototyping an alternative. Since this impacts the accessibility story quite a bit, we can wait on solving this. I do have an expert to ask and I will talk to them this week.

@cleidigh
Copy link
Contributor Author

cleidigh commented Aug 7, 2018

sounds good, when you know how you want to do the structure , I can help the aria stuff,
I've learned a fair amount through necessity. ;-)

I know there's a lot of issues to do in August, I'll try to help out where I am useful

@roblourens
Copy link
Member

roblourens commented Aug 9, 2018

I've checked in the change for #54039, so now there is no list selection. I think we want to ideally read the title, description, and value when the control is focused. I think getting rid of "role=tree" will help and I'm ok with that.

Do you know how to do that, or should we put together a simple example that I can take to someone to ask for help? It sounds like <label> and aria-labelledby should be useful here but just playing with examples on https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/forms/Basic_form_hints I'm having issues...

@cleidigh
Copy link
Contributor Author

cleidigh commented Aug 9, 2018

@roblourens
just saw your post now
I'm finishing up a PR for the base select box, then I'll go ahead and look at your new layout. IF you now have the settings "collection" - I'm purposely not using list here to not confuse - as a flat set of form
elements, we should have all the ability we need to create fully controlled labels. I should be able to work with you on that , I think we should be good.

Are you using a table for the layout? That would give us a couple options but it's not an absolute necessity.

Are we past endgame when you commits are okay?

@roblourens
Copy link
Member

roblourens commented Aug 9, 2018

I am still using the tree control, that hasn't changed. But I disable actual row selection and focus. Only controls and links are focusable.

And yes, commits to master are ok.

@cleidigh
Copy link
Contributor Author

cleidigh commented Aug 9, 2018

@roblourens
just for clarity when you say you're still using the tree control === tree widget ?
if that is case the problem is that is where the aria role = tree set and managed.
I looked at this when I was doing all the labeling experiments. let me take a look
and get a better idea of where it stands I'm sure we can make it work somehow ;-)

@roblourens
Copy link
Member

Yes, but we can remove that aria role. If we have to change the tree widget to avoid adding it in this case, we can do that.

@cleidigh
Copy link
Contributor Author

one step ahead of you, I tried that when working on the labels earlier, we have to change the other management attributes as well. One thought I had is to use and modify the list widget to use
either tree or tree grid.

@roblourens
Copy link
Member

modify the list widget to use either tree or tree grid.

Not sure what you mean by that exactly, but I'd rather just change whatever attributes are needed in the tree widget to get the result we need.

@cleidigh
Copy link
Contributor Author

Looking really good !
I should be able to do some experimenting now
I'll try to come up with some options

treegrid and grid are aria roles that both cleanly support interactive elements within them.
they also require different HTML structure than tree not just different attributes
that's what I was referring to

@roblourens
Copy link
Member

I see what you're saying. Since the tree and row elements themselves are no longer selectable at all, I wonder whether those attributes really come into play here? Since the screenreader will have nothing to focus that reads "row 5 of 10" or whatever. I have no clue.

@cleidigh
Copy link
Contributor Author

there are actually issues at multiple points.
First, what parent structure/role is being used at the "top" in our case both the list and tree widgets
structure themselves as a aria tree .
That requires a particular HTML structure, specifically an unordered list with li elements as tree rows
and with the attribute role=treeitem
descendent elements under the tree item are usually just text or links or sub branches. The readers
can infer for some information from this structure or be helped by additional attributes. Level, active-descendent, posinset are examples and these are managed by our widgets.

I did quite a bit of research and it became clear or unclear about how or if interactive form elements
are fully supported within a tree . The labels and editable state and content selection all appear to have some quirks in this case. I'm still trying to figure out if electron 3.0 addresses any of these.

ARIA treegrid and grid unambiguously supports form elements, although I have seen some labeling issues with these as well.

we can just some experiments to see what the state of things is.

@roblourens
Copy link
Member

I understand. But I wonder whether we can just remove the roles, instead of switching to a different type of role like treegrid or grid. If the screenreader just sees some form input elements, and doesn't see tree rows at all, that might be a better experience.

@cleidigh
Copy link
Contributor Author

I'm hoping we can do just that. I got some experiments done yesterday
but I have to do a couple more to know if the form elements will work.
what's the state of the electron version?

@roblourens
Copy link
Member

We plan to upgrade to 2.0.7 this month which is just a minor upgrade from the current version. So there won't be a big behavior change in chrome/screenreaders.

@cleidigh
Copy link
Contributor Author

@roblourens

YeeHah !

I think I finally found the magic combo ! I think I can post to PR later so you can look at it.

I assume you utilize NVDA

Do you know about the screen viewer under tools? this allows you to see exactly what is being read
which is sometimes difficult to follow. another thing you can do just to turn off the speech
with nvda_key+s
I also finally change the voice to a Microsoft SAPI 5
much more pleasant ;-)

@roblourens
Copy link
Member

Great! Can't wait to see it.

@cleidigh
Copy link
Contributor Author

cleidigh commented Aug 13, 2018

okay don't forget it's wip

this appears to address the problem of labels and input elements if they fall under a tree (and no one is there to hear it)

I changed the top level role=tree to form AND ALSO removed the role attribute from the individual element
it also fixed the incorrect screenreader output for the selected text
I believe we can now completely control the labeling correctly, in fact links popped up correctly also

Current general form:

Category Item [Modified] edit
selected XYZ

Some notes and decisions:

  • the first item in the list of settings does not get read correctly when it first gets focus from the search bar this is probably because you dynamically add tab index to it? it gets red subsequently correctly
  • link announcements are preceded by one or more "clickable" - research indicates this may be a chrome and NVDA bug
  • we should think how we want to have links labeled if other than default
  • do we want to announce the current value before or after the modified keyword?
  • should we experiment with "describedby" to announce item descriptions - special key?

Separate suggested: I thought it reset to default per item would be a good addition

PR
#56277

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues settings-editor VS Code settings editor issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants