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

Create tests for APG design pattern example: Actions Menu Button Example Using aria-activedescendant #378

Merged
merged 30 commits into from
Mar 11, 2021

Conversation

IsaDC
Copy link
Contributor

@IsaDC IsaDC commented Jan 14, 2021

Preview Tests

Resolves #362

@IsaDC IsaDC marked this pull request as ready for review January 15, 2021 21:12
Copy link
Contributor

@jongund jongund left a comment

Choose a reason for hiding this comment

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

Test 12 and 14: VO for MacOS
Assertion "Number of items in the menu (4) is conveyed" is missing, is this on purpose? It is required on other VO for Mac tests.

The other tests look good to me,

@jscholes
Copy link
Contributor

jscholes commented Feb 4, 2021

@jongund Thanks for catching that; it also applied to test 13. Now fixed.

@hadirangin
Copy link

In the last step of the Test Instructions, instead of having the terminology be "Using the following commands ...", we think it would be more clear if it said something like "Using the following screen reader commands/keystrokes ...". For example, the tab and arrow keys are not screen reader commands.

@hadirangin
Copy link

Currently, the Assertion table has 3 columns, assertion, Success case and Failure cases. however, in the data cell fields we have a radio group that goes over the last two columns namely Success and Failure cases. I am not sure that putting a control over 2 cells is a good practice. I suggest that we consolidate the Success and Failure columns into one column and provide a header like Output result.

@hadirangin
Copy link

When reading the radio options in the Assertions table in reading mode, there is no space between “output” and “for”. Please put a space between them as it will make the label more readable.

@hadirangin
Copy link

Note that in Jaws, as soon as you select a radio button, your screen reader goes into the interactive mode and the user won't be able to utilize the table navigation to go back to the Assertion data cell. This would mean that after any selection of Success or Failure cases, the user has to manually switch back to the reading mode so he/she can navigate in the table to the next assertion. This is a different issue than the radio buttons being in multiple cells. This may require more discussion and I suggest that we add it to our next agenda.

@jscholes
Copy link
Contributor

@hadirangin Thanks so much for this feedback! I agree with all of it, but it doesn't relate specifically to these tests. Everything you've described is generated by the ARIA-AT test runner app (https://github.com/w3c/aria-at-app) or templates in this repository, and we have no control over it from individual test patterns.

I think this is valuable accessibility feedback for the app development team, and should be filed as separate issues if you're happy to do so. Let me know if not, and I'd be happy to take care of it.

@hadirangin
Copy link

@jscholes Thank you very much for your help. As I am not sure if there is a desired format for filing these as issues, it would be very helpful if you could pass this feedback on to the app development team for me.

@hadirangin
Copy link

In both Chrome and Firefox with JAWS in default mode, upon clicking on the "Open test page", Jaws reads the entire content of the new page even though the visual focus indicator is on the "Navigate forwards from here" link. This means that none of the commands (b, f, tab or down arrow) takes the user to the Action Menu
button.
In Jaws default mode, the option “Read document and webpages” is set to read automatically on page load and I don’t know if there is a way to change it. You can turn off this option easily if you are not in Jaws default mode. I am afraid that as long as this option is on, Jaws would read the page from the anchor link down through end of the page. The user can stop reading the content anytime by pressing the space bar including when the link text “Navigate forward from here” is announced, but the user might easily miss it.
Finding a reliable solution that would work across NVDA and Jaws in default mode might need further discussion.

@robfentress
Copy link
Contributor

I suppose this is the sort of thing that will be covered when actually performing test runs and recording results, but, just in case it has something to do with the way things are coded, I wanted to point out that when performing "Testing Task 6, Navigate backwards to a menu button" on VoiceOver, executing Shift + Control + Option + Command + J does not take you backwards to the menu button as expected, but instead takes you to the field which follows it.

@jscholes
Copy link
Contributor

jscholes commented Mar 4, 2021

@robfentress Thanks for these comments!

I wanted to point out that when performing "Testing Task 6, Navigate backwards to a menu button" on VoiceOver, executing Shift + Control + Option + Command + J does not take you backwards to the menu button as expected, but instead takes you to the field which follows it.

I'll look into this. As I understand it, that command should move to the previous control. With focus on the "Navigate backwards from here" link, the previous control is definitely the menu button, but it seems like VO is moving to the next control instead.

From the Index of Assistive Technology Test Files page, under "Task ID 22, Navigate to an item in a menu by typing a character", when I click on VoiceOver for MacOS, I am taken to a blank page.

Good catch. I've just pushed a fix for this which also addresses another problem with the same test (no commands being present).

@jscholes
Copy link
Contributor

jscholes commented Mar 4, 2021

Okay, now the issues are actually fixed. I had to manually regenerate the test files locally, because the GitHub action didn't seem to delete the incorrect ones.

@jscholes
Copy link
Contributor

jscholes commented Mar 4, 2021

@robfentress

when performing "Testing Task 6, Navigate backwards to a menu button" on VoiceOver, executing Shift + Control + Option + Command + J does not take you backwards to the menu button as expected, but instead takes you to the field which follows it.

This seems like a VO issue. My best guess of what's happening is:

  1. focus initially lands on a link;
  2. VO doesn't consider links to be "controls" for the purposes of VO+Command+J navigation; so...
  3. trying to move to the previous control from the link results in the wrong behaviour, i.e. moving to the text input after it rather than the menu button before it.

This is completely nonsensical. Even if it doesn't treat links as "controls", it knows its current position in the DOM and that the text box is after that position. There is no world in which pressing a command to navigate to a previous element should move to something after the current DOM position.

The only suggestions I can make, other than or in addition to filing a bug with Apple, are:

  1. remove this command from this test; or
  2. replace the "Navigate forwards/backwards from here" elements with buttons instead of links.

I haven't confirmed #2 would work; I'm just guessing Apple considers buttons to be "controls" even though such a term is pretty much meaningless. It would also make the tests inconsistent with others that use links, unless we switch to buttons across the board. For #1, would we also want to remove VO+Command+J from the corresponding "navigate forwards" tests?

Alternatively, we keep everything as is, and let testers report that they ended up on the wrong element and mark the assertions as failed. Arguably, this is a screen reader bug that our tests should catch.

@jscholes
Copy link
Contributor

jscholes commented Mar 4, 2021

CC @cookiecrook for the behaviour described in #378 (comment) and #378 (comment).

@mcking65
Copy link
Contributor

mcking65 commented Mar 8, 2021

@hadirangin wrote:

In the last step of the Test Instructions, instead of having the terminology be "Using the following commands ...", we think it would be more clear if it said something like "Using the following screen reader commands/keystrokes ...". For example, the tab and arrow keys are not screen reader commands.

I don't understand this feedback. Hadi, it seems like you are asking for the instructions to distinguish between assistive technology commands and keyboard commands that are implemented by the browser or javascript. What would we call the commands that are not assistive technology commands?

I'm uncertain what advantage would be gained from adding that extra information to the instructions. It would add complexity to test writing and add words to the instructions. How would that extra information benefit the person conducting the test?

@hadirangin
Copy link

hadirangin commented Mar 9, 2021

@mcking65 wrote:

@hadirangin wrote:

In the last step of the Test Instructions, instead of having the terminology be "Using the following commands ...", we think it would be more clear if it said something like "Using the following screen reader commands/keystrokes ...". For example, the tab and arrow keys are not screen reader commands.

I don't understand this feedback. Hadi, it seems like you are asking for the instructions to distinguish between assistive technology commands and keyboard commands that are implemented by the browser or javascript. What would we call the commands that are not assistive technology commands?

I'm uncertain what advantage would be gained from adding that extra information to the instructions. It would add complexity to test writing and add words to the instructions. How would that extra information benefit the person conducting the test?

I am for separating screen reader commands from OS or application commands. Yes, it would make the instruction more wordy and unnecessarily difficult but I think it is beneficial to know which commands relate to the screen readers and which are keyboard commands. But considering that we will be doing only testing with screen readers, then I think it would be okay if we leave them combined as is.

@hadirangin
Copy link

hadirangin commented Mar 9, 2021

In "Task ID 10 Open a menu in reading mode" and "Task ID 11 Open a menu in interaction mode", the 3rd assertion reads “Role of the focused item, ‘menu item’, is conveyed”. I am wondering if we need this assertion at all. We already know that we are on a menu button and knowingly we open it with space bar or enter key which exposes the menu items and the focus goes to the first menu item and read it.

@hadirangin
Copy link

hadirangin commented Mar 9, 2021

For "Task ID 2 Navigate backwards to a menu button in reading mode", the second command is “F / SHIFT + F” when it should only be “SHIFT + F”. This is on both the JAWS and NVDA testing pages.

@jscholes
Copy link
Contributor

jscholes commented Mar 9, 2021

@hadirangin

In "Task ID 10 Open a menu in reading mode" and "Task ID 11 Open a menu in interaction mode", the 3rd assertion reads “Role of the focused item, ‘menu item’, is conveyed”. I am wondering if we need this assertion at all. We already know that we are on a menu button and knowingly we open it with space bar or enter key which exposes the menu items and the focus goes to the first menu item and read it.

Focus moves to a menu item, so the screen reader can convey the role of that new focus. Note that I say "can" rather than "should", because many screen readers don't actually do this and therefore, the assertion is listed as optional. Arguably, a screen reader saying "menu item" after every navigation action would be irritating to some users so we don't state that it should be mandatory.

@jscholes
Copy link
Contributor

jscholes commented Mar 9, 2021

@hadirangin

the second command is “F / SHIFT + F” when it should only be “SHIFT + F”. This is on both the JAWS and NVDA testing pages.

Thanks! This should now be fixed once the test files are regenerated.

@hadirangin
Copy link

In the “Task ID 15: Read information about a menu item in interaction mode”, we are directed to a menu item. I believe it is essential to know what object this specific menu item belongs to. So I suggest to add the assertions: “Role ‘menu’ is conveyed” and “Name ‘Actions’ is conveyed”.

@jscholes
Copy link
Contributor

jscholes commented Mar 9, 2021

@hadirangin

I believe it is essential to know what object this specific menu item belongs to. So I suggest to add the assertions: “Role ‘menu’ is conveyed” and “Name ‘Actions’ is conveyed”.

The menu item has focus, and I don't believe any screen reader to report information about its parent container when reviewing the current focus at present. Nor would I expect it to; if I press, say, NVDA+Tab, I expect it to report the object with focus and nothing more.

If we were to assert that it should also indicate what menu the item is in, it's not clear where we would set boundaries for similar tests. E.g. should reviewing information about a focused listbox item also report information about the listbox itself?

Appreciate any additional input on this. But I think the tests, as they stand, represent the intent behind the commands in the various ATs. Namely, to report the current focus only.

@mcking65
Copy link
Contributor

@jscholes wrote:

@hadirangin

I believe it is essential to know what object this specific menu item belongs to. So I suggest to add the assertions: “Role ‘menu’ is conveyed” and “Name ‘Actions’ is conveyed”.

The menu item has focus, and I don't believe any screen reader to report information about its parent container when reviewing the current focus at present. Nor would I expect it to; if I press, say, NVDA+Tab, I expect it to report the object with focus and nothing more.

If we were to assert that it should also indicate what menu the item is in, it's not clear where we would set boundaries for similar tests. E.g. should reviewing information about a focused listbox item also report information about the listbox itself?

Appreciate any additional input on this. But I think the tests, as they stand, represent the intent behind the commands in the various ATs. Namely, to report the current focus only.

I agree with Hadi, I think this is something that screen readers are not doing but should be able to do. The fact that commands like JAWS/NVDA insert+tab do not provide good access to container information in radio groups, trees, menus, menubars, etc. is a serious gap. The semantics are there but not being used. This is a holdover from insufficient legacy desktop apps where the semantics were often not present.

So, this gets into tricky territory, very tricky territory for the project.

I propose that we use optional assertions to suggest improved utilization of ARIA semantics available in the patterns. The point of the patterns is to standardize expectations for authors. Part of the purpose of this project is to ensure those semantics are both consistently and usefully rendered. There are places where web patterns have much better and more useful semantics than legacy desktop software. We should expose that.

That said, we should have consensus among us for these kinds of stretches. We should add them only after careful discussion.

I've raised #397 to discuss this proposal by Hadi.

@mcking65 mcking65 merged commit 9646312 into master Mar 11, 2021
@mcking65 mcking65 deleted the tests/menu-button-actions-active-descendant branch March 11, 2021 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create tests for APG design pattern example: Actions Menu Button Example Using aria-activedescendant
6 participants