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

Review editor menubar example revisions made through June 2017 #409

Closed
3 tasks done
mcking65 opened this issue Jun 21, 2017 · 8 comments
Closed
3 tasks done

Review editor menubar example revisions made through June 2017 #409

mcking65 opened this issue Jun 21, 2017 · 8 comments
Assignees
Labels
Example Page Related to a page containing an example implementation of a pattern

Comments

@mcking65
Copy link
Contributor

mcking65 commented Jun 21, 2017

The editor menubar example has been significantly revised and is ready for additional review.

Known issues

Reviews requested as of June 20, 2017

@mcking65 mcking65 added Example Page Related to a page containing an example implementation of a pattern Needs Review labels Jun 21, 2017
@mcking65 mcking65 added this to the 1.1 Rec milestone Jun 21, 2017
@mcking65 mcking65 modified the milestones: 1.1 Rec, 3Q17 Working Draft Oct 10, 2017
mcking65 added a commit that referenced this issue Oct 11, 2017
Related to issue #409:

1. Add aria-disabled to menuitems for for increasing and decreasing font size. Disable larger when size is already largest and smaller when size is already smallest.
2. Add documentation to roles, states, and properties for aria-disabled.
3. Add documentation to roles, states, and properties for names calculated from content.
@shirsha
Copy link

shirsha commented Oct 31, 2017

Below are my observations:

  1. When the focus is on the “Font” menu, use down arrow ( you will see “Sans-seriff” has a check mark next to it) and go the last submenu item “Fantasy” and select enter. The Font changes accordingly. Select tab to go the next focusable item. The whole menubar is missing now.
    Please see the screenshot Move issues to new repo #1.

2)When I selected Fantasy, now I go to the next item (Style/Color) in the menubar by clicking the right arrow.
Then I will go back to the previous item(Font) to see my selection and none of them have a checkmark next to it.

Screen reader NVDA in FF browser is not announcing that the different font is selected (on selecting enter) and when I go back to that font type by using up and down arrow, NVDA is reading the font type is not checked but the font is changed in the tab panel according to the selection made.
Please see the attachment #2

In Style/color menu. Upon selecting only first two submenu items got check mark next to them. Not for the remaining selections.
Please see screen shot#3

Editor menu bar.docx

@StommePoes
Copy link

StommePoes commented Nov 15, 2017

I ran the example through aXe, since it had some stuff that looked fishy (li with aria-expanded which never changes, then inside a direct-child span with aria-expanded which DOES change... confusing for a developer) and aXe claimed a menuitem role'd thing can't have aria-expanded.

Indeed I don't see aria-expanded in the list for menuitems and related types, though I do see aria-haspopup. So what's the secret way of showing the expanded/collapsed state of the submenu "popup"? Should the menuitem NOT have a haspopup and instead have a button inside with haspopup and expanded?
Or should the menuitem have haspopup and a button child inside with expanded?
Or should the menuitem just have haspopup and users need to simply expect there is an expanded popup merely because they've encountered the menuitem in the first place (no explicit state)?

Are menuitems allowed to have controls inside them other than what they themselves do? If it has a menu child, can it also have a button child? Should it?

Whatever it is I'm doing at work, it's never going to be following the spec entirely, because I'm too dumb to be able to read it right.

@jnurthen
Copy link
Member

@StommePoes see w3c/aria#454

@mcking65
Copy link
Contributor Author

mcking65 commented Nov 16, 2017

@shirsha, thanks. It looks like this example was broken by commit 0de93bd.

It took some digging, but I found if I checkout one of the parents of that commit, commit a6d74df, the aria-checked properties are getting set correctly.
Then, if I checkout commit 0de93bd,
in several of the menus, the aria-checked state is not getting set correctly.

That is the cause of the NVDA behavior you are observing.

I could not reproduce the first issue you observed.

I also observed that that the Enter key behavior does not match what is documented in the pattern.

I raised issue #524.

@a11ydoer
Copy link
Contributor

@mcking65 I have reviewed the menubar and @shirsha seems to do a good job to review the example.

@mcking65
Copy link
Contributor Author

@StommePoes commented:

Are menuitems allowed to have controls inside them other than what they themselves do? If it has a menu child, can it also have a button child? Should it?

No, menuitems are not composite widgets. They cannot contain other widgets.

@mcking65
Copy link
Contributor Author

OK; it appears all requested reviews are complete and comments have been addressed now that issue #524 is closed. Thank you all for your help with this! Special thanks to @sh0ji who speedily handled issue #524. Closing!

@wzhonggo
Copy link

No refer to menubar-2-init.js in the Javascript and CSS Source Code paragraph of menubar-2 example.

CSS and js in menubar-2.html

 <!--  CSS and javascript for this example.  -->
  <link href="css/menubarAction.css" rel="stylesheet">
  <script src="js/styleManager.js"    type="text/javascript"></script>
  <script src="js/MenubarAction.js"       type="text/javascript"></script>
  <script src="js/MenubarItemAction.js"   type="text/javascript"></script>
  <script src="js/PopupMenuAction.js"     type="text/javascript"></script>
  <script src="js/PopupMenuItemAction.js" type="text/javascript"></script>
  <script src="js/menubar-2-init.js" type="text/javascript"></script>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Example Page Related to a page containing an example implementation of a pattern
Development

No branches or pull requests

6 participants