Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

[POC] Render MenuItem as li > button #85

Closed
wants to merge 1 commit into from

Conversation

miroslavstastny
Copy link
Member

Menu

!!! DO NOT MERGE !!!

For accessibility reasons, Menu renders as ul > li > a with onClick handler on a, see #44.
This still does not work as expected, see #61.

This is a Proof of Concept which renders the menu as ul > li > button to test if it helps with the accessibility.

There should be no visual regression, however UTs are failing at the moment.

TODO

  • Conformance test
  • Minimal doc site example
  • Stardust base theme
  • Teams Light theme
  • Teams Dark theme
  • Teams Contrast theme
  • Confirm RTL usage
  • W3 accessibility check
  • Stardust accessibility check
  • Update glossary props table
  • Update the CHANGELOG.md

@@ -93,7 +93,7 @@ class MenuItem extends UIComponent<any, any> {
{childrenExist(children) ? (
children
) : (
<a
<button
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tested with the change as button and now it works as expected for each case.
Space and Enter trigger onclick method.
Tested with:
Chrome > JAWS with and without virtual cursor
Edge > Narrator when scan mode is on/off
Firefox > NVDA > when browse mode is on/off
OSX > Chrome > VoiceOver > without and with VoiceOver keys

Copy link
Collaborator

Choose a reason for hiding this comment

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

Today we have been testing more using "a" tag for menu and figure out following:

When added on menu "a" handleKeydown method:
handleKeydown = e => {
if (e.which === 32 || e.which === 13) {
this.handleClick(e)
// _.invoke(this.props, 'onClick', e, this.props)
e.preventDefault()
}
}

  1. Then space/enter was working properly

  2. And JAWS was not listed link in link view... What is fine...

So in the end I think we could go with "a" tag in menu.

@jurokapsiar @miroslavstastny

@@ -93,7 +93,7 @@ class MenuItem extends UIComponent<any, any> {
{childrenExist(children) ? (
children
) : (
<a
Copy link
Member

@notandrew notandrew Aug 13, 2018

Choose a reason for hiding this comment

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

Do we really want to replace the a tag with a button? I think that might have its own set of issues. I don't understand the root issue though. Is it that the onclick handler is interfering with the default link?

@miroslavstastny
Copy link
Member Author

After discussion with @jurokapsiar we decided to stick with ul > li > a for now and handle Enter/Space in Menu Behavior

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants