Open zcorpan opened 4 years ago
To clarify, is https://w3c.github.io/aria-at/review/menubar-editor.html up to date and ready to review?
If so, some initial feedback:
I also put together a matrix to help me identify at a high level which features, states, and modes were tested. I marked a few that I thought might be missing with a question mark. Maybe it would be helpful to generate a table like this for the review page? @mcking65 @spectranaut (not sure what the best structure of the table would be)
Feature | Feature state | Navigate - reading mode | Navigate - interaction mode | Read - reading mode | Read - interaction mode | Operate - reading mode | Operate - Interaction mode |
---|---|---|---|---|---|---|---|
Menu item checkbox | To unchecked | 2 | 1 | ||||
Menu item checkbox | To checked | ||||||
Menu item radio | To checked | 4 | 3 | ||||
Submenu (menuitem[aria-expanded] | To closed | 6 | 5 | ||||
menubar | - | ? | 7 | ? | ? | ||
manuitem | - | 9 | 8 | 19 | 18 | ||
Menuitem checkbox | - | 11 | 10 | 21 | 20 | ||
Menuitem radio | - | 13 | 12 | 23 | 22 | ||
Submenu (menuitem[aria-expanded] | To open | 15 | 14 | ||||
menuitem | disabled | ? | 17 | 16 | |||
Menubar | Mode switches to forms | 24 | |||||
Yes its up to date!
About the technical bugs: There is a PR open that will address the bug that made (2 - duplicate keys), and I can look into (5 - character rendering) today or tomorrow and plan to fix (7 - setup script description) today or tomorrow.
About the key commands: I think Jon and Yohta just didn't supply commands for VoiceOver and NVDA. Who can do that?
Thank you @spectranaut - I can do it, but not before Wed.
I've just finished commands for VoiceOver and NVDA (see 'Command' sheet for attached files), Would you be able to encode them for review @spectranaut @mfairchild365 ? Otherwise, I'll touch base with Jon and encode them once he's back next week.
Quick notes:
NVDA key
200218_Menubar_NVDA_test_writing.xlsx
VO key
@Yohta89 if you upload a spreadsheet I can add them. Can you reupload the spreadsheet that is already in the repository, so it's in the same format?
Edit: oh sorry I misread, I see you have added the commands! I'll work on encoding them.
Thanks @spectranaut ! I attached the file when I edited the comment! Let me know if the format didn't fit.
@Yohta89 -- I added the new commands to this checked in CSV file, and then used Jon's script to convert them to the commands.json format.
The commands can all be reviewed in the menubar testplan review page
Community Group) Although we agreed to start peer-review for the menubar at the meeting today, I noticed some of the issues raised by @mfairchild365 on his post Feb.23rd isn't yet fixed. I'm gonna fix those asap and cue here once the menubar example is ready to review.
Once the PR #160 is merged, this should be ready for peer-review.
From 2020-04-22 call; I'll document the existing behavior of native menu bars with JAWS. @Yohta89 @jongund will document NVDA and VO.
Sample of aspects to document:
Context | Mode | Action | Information | Notes |
---|---|---|---|---|
menutiem | interactive | move focus to menubar | menu item information | no information about menubar spoken |
menuitem[aria-haspopup] | interactive | Move focus to menuitem with a popup or popout menu | Speaks label, Sub menu and shortcut key | no difference in speech for popup or popout menu even though keyboard commands are different to open |
menuitem | interactive | Move focus to menuitem in sub menu | Speaks label and keyboard shortcut | |
menuitemradio | interactive | Move focus to menuitemradio in sub menu | Speaks label, "radio menu item", checked state and keyboard shortcut | |
menuitemcheckbox | interactive | Move focus to menuitemcheckbox in sub menu | Speaks label, checked state and keyboard shortcut |
Context | Mode | Action | Information | Notes |
---|---|---|---|---|
menubar and top-level menuitem | interactive | move focus to menubar | "Menu, file, 1 of 7" | TODO |
menuitem[aria-haspopup] | interactive | Move focus to menuitem with a popup or popout menu | "toolbars, sub menu, 1 of 2, T" | TODO |
menuitem | interactive | Move focus to menuitem in sub menu | "downloads, ctrl+J, 1 of 3, D" | |
menuitemradio | interactive | Move focus to menuitemradio in sub menu | "No style, 1 of 2, N", and "Basic page, style, checked, 2 of 2, B" | menuitem radio was not conveyed by the unchecked menuitemradio. |
menuitemradio - change state | interactive | press enter when on the radio | "enter, leaving menus" | new state is not conveyed, instead the menu is closed. |
menuitemcheckbox | interactive | Move focus to menuitemcheckbox in sub menu | "menubar not checked, 1 of 2, M" and "menu bar, checked, 1 of 2, M" | |
menuitcheckbox - change state | interactive | press enter when on the checkbox | "enter, leaving menus" | new state is not conveyed, instead the menu is closed. |
Open menu | interactive | press enter on the 'view' submenu | "enter, view menu, toolbars submenu, 1 of 2, t | |
Open submenu | interactive | press enter on the 'toolbars' submenu | "enter, menubar, not checked, 1 of 2, m | |
Close menu | interactive | press escape when on a menuitem under view | "escape, leaving menus" | |
disabled menuitem | interactive | read disabled menuitem | "recently closed windows, unavailable, sub menu, 3 of 3" |
Note: reading all of the roles resulted in equivalent information being conveyed as when navigating to it.
I used Accessibility Insights to look at the accessibility properties of the Menu and compare them with SR output.
When navigating to the menu bar, the name of the menu bar is not conveyed, and I think the role may be conveyed as "menu". See output above.
I wasn't able to find roles like menuitemradio
or menuitemcheckbox
or haspopup
within the inspector.
Test 1, 2, 3, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26: - NA: reading mode is not supported for native menubars
For Test 27: Read disabled menu item in submenu in interaction mode, for VoiceOver, the second assertion is, "The name 'Smaller' is conveyed." I suspect this is a typo. Since the 'Larger' menu item is selected based on the Scripted Instructions, I think the assertion should read, "The name 'Larger' is conveyed."
For Test 23: Read menu item radio button in submenu in interaction mode, it should be noted that, for VoiceOver (on Safari, at least), while the group role and group name assertions would not be passed by this test, that information is available if the user navigates the menu using VO+Arrow. Not sure if that should be added as a different test, or somehow noted elsewhere, or if we just want to not worry about it, but I worry this may give a misleading impression about support for this structure on VoiceOver.
@jongund @mcking65 I finished my review and comparison to a native menubar for JAWS. See the list of suggestions at the end of https://github.com/w3c/aria-at/issues/54#issuecomment-619153517. I'm happy to answer any questions.
@mfairchild365 commented:
- Remove menubar related assertions from tests for navigating to and reading a menuitem in all modes. This is handled by test 3.
Agree, we only want to make assertions about role and name of the menubar in the tests related to navigating to the menubar itself.
It is worth noting that if you do not have JAWS loading from the system tray, and if you navigate to the native JAWS menubar in the main JAWS window by pressing "Alt", JAWS announces the menubar role. And, in the JAWS menubar, there is no position or setsize information like there is in the Firefox menubar. So, the behavior is different in different apps.
- Add assertion for posinset and setsize to all reading and navigation tests in both modes. Reading mode might be optional for JAWS.
Have this now in #184
- Keep assertions related to state changes when activating a menuitemcheckbox or menuitem radio, even though this does not match the behavior of the native example.
Since the menu closes, we can't really test state changes. Those have been removed. I wish the editor menubar would put focus in the text area when it closes. It would be more realistic that way. But, that is an APG issue.
- Keep assertions related to role and state of the unchecked menuitem radio, even though this does not match the behavior of the native example.
Definitely agree here. Amazing this info is not communicated.
- Keep assertions related to closing a menu, since the behavior of the native example is not the same. (native closes the entire menubar, while APG does not).
Agree that it is OK to keep them. Although, this is one where I can anticipate changing my mind after working with this more.
- Keep optional assertions related role="group", even though this pattern is not found in the native example. The native example uses unnamed separators.
Yes, this is an important feature of ARIA; it would be good if this existed natively.
I am going to go ahead and merge #184. I think it is really important to get the improvements to the ability to format instructions into all the examples.
That said, I still see need to make multiple changes to menubar. I will make them in a separate PR as suggested by @mfairchild365.
The biggest issue is that reading mode in JAWS and NVDA are so different that, in order to respect their perspectives on design, we will need to tailor assertions to each. This is a good test for demonstrating that even with very different approaches, different screen readers can fully support the ARIA necessary to make the widget understandable. It is not our job to say which presentation of accessibility semantics is more understandable; it is our job to find out whether the presentation made by each screen reader sufficiently represents the accessibility semantics in the user interface.
Then, there are some odd little issues like shift+tab does not cause a mode change for JAWS or NVDA because the text area also triggers interaction mode. So, for that test, we will test only Tab, not Shift+Tab. Or, we might be able to test shift+Tab, but it will need its own instructions. I am leaning toward not bothering with shift+Tab.
APG design pattern: https://w3c.github.io/aria-practices/#menu