fix(ui-shell): remove role="menuitem" from header links

* feat(ui-shell): switch header nav role for side nav This change allows users of `<HeaderMenu>` and `<HeaderMenuItem>` to specify their ARIA roles so the a11y structure won't be broken when they are put in side nav for narrow screen. Fixes #3572. * fix(ui-shell): remove role in a tag * chore(UIShell): update snapshot * fix(HeaderMenuItem): style fix * Update packages/components/src/components/ui-shell/_side-nav.scss Co-Authored-By: emyarod <[email protected]> * Update packages/components/src/components/ui-shell/_side-nav.scss Co-Authored-By: emyarod <[email protected]> * fix(HeaderMenu): more style fix Co-authored-by: Josh Black <[email protected]> Co-authored-by: TJ Egan <[email protected]> Co-authored-by: emyarod <[email protected]>
carbon-design-system · Jan 23, 2020 · 63c950e · 63c950e
1 parent c4e8bcd
commit 63c950e
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 52 deletions.
diff --git a/packages/components/src/components/ui-shell/_header.scss b/packages/components/src/components/ui-shell/_header.scss
@@ -175,7 +175,7 @@
     margin: 0;
   }
 
-  a.#{$prefix}--header__menu-item[role='menuitem'] {
+  a.#{$prefix}--header__menu-item {
     display: flex;
     align-items: center;
     color: $shell-header-text-02;
@@ -196,46 +196,46 @@
       border-color $duration--fast-02, color $duration--fast-02;
   }
 
-  a.#{$prefix}--header__menu-item[role='menuitem']:hover {
+  a.#{$prefix}--header__menu-item:hover {
     background-color: $shell-header-bg-02;
     color: $shell-header-text-01;
   }
 
   .#{$prefix}--header__action:active,
-  a.#{$prefix}--header__menu-item[role='menuitem']:active {
+  a.#{$prefix}--header__menu-item:active {
     background-color: $shell-header-bg-03;
     color: $shell-header-text-01;
   }
 
-  a.#{$prefix}--header__menu-item[role='menuitem']:focus {
+  a.#{$prefix}--header__menu-item:focus {
     border-color: $shell-header-focus;
     color: $shell-header-text-01;
     outline: none;
   }
 
-  a.#{$prefix}--header__menu-item[role='menuitem']:hover > svg,
-  a.#{$prefix}--header__menu-item[role='menuitem']:active > svg,
-  a.#{$prefix}--header__menu-item[role='menuitem']:focus > svg {
+  a.#{$prefix}--header__menu-item:hover > svg,
+  a.#{$prefix}--header__menu-item:active > svg,
+  a.#{$prefix}--header__menu-item:focus > svg {
     fill: $shell-header-icon-01;
   }
 
   .#{$prefix}--header__submenu {
     position: relative;
   }
 
-  .#{$prefix}--header__menu-title[role='menuitem'][aria-haspopup='true'] {
+  .#{$prefix}--header__menu-title[aria-haspopup='true'] {
     position: relative;
   }
 
-  .#{$prefix}--header__menu-title[role='menuitem'][aria-expanded='true'] {
+  .#{$prefix}--header__menu-title[aria-expanded='true'] {
     background-color: $shell-header-bg-06;
     color: $shell-header-focus;
     // Note: needs to be higher than menu. Adding 1 here instead of moving to
     // the next level.
     z-index: #{z('header') + 1};
   }
 
-  .#{$prefix}--header__menu-title[role='menuitem'][aria-expanded='true']
+  .#{$prefix}--header__menu-title[aria-expanded='true']
     > .#{$prefix}--header__menu-arrow {
     transform: rotate(180deg);
   }
@@ -247,7 +247,7 @@
     margin: 0;
   }
 
-  .#{$prefix}--header__menu-title[role='menuitem'][aria-expanded='true']
+  .#{$prefix}--header__menu-title[aria-expanded='true']
     + .#{$prefix}--header__menu {
     position: absolute;
     bottom: 0;
@@ -261,24 +261,23 @@
     z-index: z('header');
   }
 
-  .#{$prefix}--header__menu-title[role='menuitem'][aria-expanded='true']
+  .#{$prefix}--header__menu-title[aria-expanded='true']
     + .#{$prefix}--header__menu
     .#{$prefix}--header__menu-item:hover {
     background-color: $shell-header-bg-04;
   }
 
-  .#{$prefix}--header__menu-title[role='menuitem'][aria-expanded='true']
+  .#{$prefix}--header__menu-title[aria-expanded='true']
     + .#{$prefix}--header__menu
     .#{$prefix}--header__menu-item:active {
     background-color: $shell-header-bg-03;
   }
 
-  .#{$prefix}--header__menu .#{$prefix}--header__menu-item[role='menuitem'] {
+  .#{$prefix}--header__menu .#{$prefix}--header__menu-item {
     height: mini-units(6);
   }
 
-  .#{$prefix}--header__menu
-    .#{$prefix}--header__menu-item[role='menuitem']:hover {
+  .#{$prefix}--header__menu .#{$prefix}--header__menu-item:hover {
     background-color: $shell-header-bg-06;
     color: $shell-header-text-01;
   }

diff --git a/packages/components/src/components/ui-shell/_side-nav.scss b/packages/components/src/components/ui-shell/_side-nav.scss
@@ -330,9 +330,9 @@
     > .#{$prefix}--side-nav__link:hover,
   .#{$prefix}--side-nav__menu[role='menu']
     a.#{$prefix}--side-nav__link[role='menuitem']:not(.#{$prefix}--side-nav__link--current):not([aria-current='page']):hover,
-  .#{$prefix}--side-nav a.#{$prefix}--header__menu-item[role='menuitem']:hover,
+  .#{$prefix}--side-nav a.#{$prefix}--header__menu-item:hover,
   .#{$prefix}--side-nav
-    .#{$prefix}--header__menu-title[role='menuitem'][aria-expanded='true']:hover {
+    .#{$prefix}--header__menu-title[aria-expanded='true']:hover {
     // TODO: sync color
     background-color: $shell-side-nav-bg-04;
     color: $ibm-color__gray-100;
@@ -474,9 +474,9 @@
   // Side-nav > Link
   //----------------------------------------------------------------------------
   a.#{$prefix}--side-nav__link,
-  .#{$prefix}--side-nav a.#{$prefix}--header__menu-item[role='menuitem'],
+  .#{$prefix}--side-nav a.#{$prefix}--header__menu-item,
   .#{$prefix}--side-nav
-    .#{$prefix}--header__menu-title[role='menuitem'][aria-expanded='true']
+    .#{$prefix}--header__menu-title[aria-expanded='true']
     + .#{$prefix}--header__menu {
     @include focus-outline('reset');
     @include type-style('productive-heading-01');
@@ -498,7 +498,7 @@
 
   a.#{$prefix}--side-nav__link > .#{$prefix}--side-nav__link-text,
   .#{$prefix}--side-nav
-    a.#{$prefix}--header__menu-item[role='menuitem']
+    a.#{$prefix}--header__menu-item
     .#{$prefix}--text-truncate-end {
     @include text-overflow();
     color: $shell-side-nav-text-01;
@@ -509,7 +509,7 @@
   }
 
   a.#{$prefix}--side-nav__link:focus,
-  .#{$prefix}--side-nav a.#{$prefix}--header__menu-item[role='menuitem']:focus {
+  .#{$prefix}--side-nav a.#{$prefix}--header__menu-item:focus {
     @include focus-outline('outline');
   }
 
@@ -619,7 +619,7 @@
   }
 
   //header menu items overrides
-  .#{$prefix}--side-nav a.#{$prefix}--header__menu-item[role='menuitem'] {
+  .#{$prefix}--side-nav a.#{$prefix}--header__menu-item {
     color: $shell-side-nav-text-01;
     white-space: nowrap;
     justify-content: space-between;
@@ -630,7 +630,7 @@
   }
 
   .#{$prefix}--side-nav
-    .#{$prefix}--header__menu-title[role='menuitem'][aria-expanded='true']
+    .#{$prefix}--header__menu-title[aria-expanded='true']
     + .#{$prefix}--header__menu {
     bottom: inherit;
     width: 100%;
@@ -643,28 +643,28 @@
       width: 100%;
     }
 
-    & a.#{$prefix}--header__menu-item[role='menuitem'] {
+    a.#{$prefix}--header__menu-item {
       padding-left: 4.25rem;
       font-weight: 400;
     }
 
-    & a.#{$prefix}--header__menu-item[role='menuitem']:hover {
+    a.#{$prefix}--header__menu-item:hover {
       background-color: $shell-side-nav-bg-04;
       color: $ibm-color__gray-100;
     }
   }
 
   .#{$prefix}--side-nav
     .#{$prefix}--header__menu
-    a.#{$prefix}--header__menu-item[role='menuitem'] {
+    a.#{$prefix}--header__menu-item {
     height: inherit;
   }
 
   .#{$prefix}--side-nav
-    a.#{$prefix}--header__menu-item[role='menuitem']:hover
+    a.#{$prefix}--header__menu-item:hover
     .#{$prefix}--header__menu-arrow,
   .#{$prefix}--side-nav
-    a.#{$prefix}--header__menu-item[role='menuitem']:focus
+    a.#{$prefix}--header__menu-item:focus
     .#{$prefix}--header__menu-arrow,
   .#{$prefix}--side-nav .#{$prefix}--header__menu-arrow {
     fill: $shell-side-nav-text-01;

diff --git a/packages/react/src/components/UIShell/HeaderMenu.js b/packages/react/src/components/UIShell/HeaderMenu.js
@@ -194,7 +194,6 @@ class HeaderMenu extends React.Component {
           href="#"
           onKeyDown={this.handleOnKeyDown}
           ref={this.handleMenuButtonRef}
-          role="menuitem"
           tabIndex={0}
           {...accessibilityLabel}>
           {menuLinkName}

diff --git a/packages/react/src/components/UIShell/HeaderMenuItem.js b/packages/react/src/components/UIShell/HeaderMenuItem.js
@@ -22,7 +22,6 @@ const HeaderMenuItem = React.forwardRef(function HeaderMenuItem(
         {...rest}
         className={`${prefix}--header__menu-item`}
         ref={ref}
-        role="menuitem"
         tabIndex={0}>
         <span className={`${prefix}--text-truncate--end`}>{children}</span>
       </Link>

diff --git a/packages/react/src/components/UIShell/UIShell-story.js b/packages/react/src/components/UIShell/UIShell-story.js
@@ -182,10 +182,19 @@ storiesOf('UI Shell', module)
                 isPersistent={false}>
                 <SideNavItems>
                   <HeaderSideNavItems>
-                    <HeaderMenuItem href="#">Link 1</HeaderMenuItem>
-                    <HeaderMenuItem href="#">Link 2</HeaderMenuItem>
-                    <HeaderMenuItem href="#">Link 3</HeaderMenuItem>
-                    <HeaderMenu aria-label="Link 4" menuLinkName="Link 4">
+                    <HeaderMenuItem linkRole="link" href="#">
+                      Link 1
+                    </HeaderMenuItem>
+                    <HeaderMenuItem linkRole="link" href="#">
+                      Link 2
+                    </HeaderMenuItem>
+                    <HeaderMenuItem linkRole="link" href="#">
+                      Link 3
+                    </HeaderMenuItem>
+                    <HeaderMenu
+                      linkRole="link"
+                      aria-label="Link 4"
+                      menuLinkName="Link 4">
                       <HeaderMenuItem href="#">Sub-link 1</HeaderMenuItem>
                       <HeaderMenuItem href="#">Sub-link 2</HeaderMenuItem>
                       <HeaderMenuItem href="#">Sub-link 3</HeaderMenuItem>
@@ -306,10 +315,19 @@ storiesOf('UI Shell', module)
                 isPersistent={false}>
                 <SideNavItems>
                   <HeaderSideNavItems>
-                    <HeaderMenuItem href="#">Link 1</HeaderMenuItem>
-                    <HeaderMenuItem href="#">Link 2</HeaderMenuItem>
-                    <HeaderMenuItem href="#">Link 3</HeaderMenuItem>
-                    <HeaderMenu aria-label="Link 4" menuLinkName="Link 4">
+                    <HeaderMenuItem linkRole="link" href="#">
+                      Link 1
+                    </HeaderMenuItem>
+                    <HeaderMenuItem linkRole="link" href="#">
+                      Link 2
+                    </HeaderMenuItem>
+                    <HeaderMenuItem linkRole="link" href="#">
+                      Link 3
+                    </HeaderMenuItem>
+                    <HeaderMenu
+                      linkRole="link"
+                      aria-label="Link 4"
+                      menuLinkName="Link 4">
                       <HeaderMenuItem href="#">Sub-link 1</HeaderMenuItem>
                       <HeaderMenuItem href="#">Sub-link 2</HeaderMenuItem>
                       <HeaderMenuItem href="#">Sub-link 3</HeaderMenuItem>
@@ -371,10 +389,19 @@ storiesOf('UI Shell', module)
                 expanded={isSideNavExpanded}>
                 <SideNavItems>
                   <HeaderSideNavItems hasDivider={true}>
-                    <HeaderMenuItem href="#">Link 1</HeaderMenuItem>
-                    <HeaderMenuItem href="#">Link 2</HeaderMenuItem>
-                    <HeaderMenuItem href="#">Link 3</HeaderMenuItem>
-                    <HeaderMenu aria-label="Link 4" menuLinkName="Link 4">
+                    <HeaderMenuItem linkRole="link" href="#">
+                      Link 1
+                    </HeaderMenuItem>
+                    <HeaderMenuItem linkRole="link" href="#">
+                      Link 2
+                    </HeaderMenuItem>
+                    <HeaderMenuItem linkRole="link" href="#">
+                      Link 3
+                    </HeaderMenuItem>
+                    <HeaderMenu
+                      linkRole="link"
+                      aria-label="Link 4"
+                      menuLinkName="Link 4">
                       <HeaderMenuItem href="#">Sub-link 1</HeaderMenuItem>
                       <HeaderMenuItem href="#">Sub-link 2</HeaderMenuItem>
                       <HeaderMenuItem href="#">Sub-link 3</HeaderMenuItem>

diff --git a/packages/react/src/components/UIShell/__tests__/__snapshots__/HeaderMenu-test.js.snap b/packages/react/src/components/UIShell/__tests__/__snapshots__/HeaderMenu-test.js.snap
@@ -18,7 +18,6 @@ exports[`HeaderMenu should render 1`] = `
         className="bx--header__menu-item bx--header__menu-title"
         href="#"
         onKeyDown={[Function]}
-        role="menuitem"
         tabIndex={0}
       >
         <defaultRenderMenuContent>
@@ -72,13 +71,11 @@ exports[`HeaderMenu should render 1`] = `
               className="bx--header__menu-item"
               element="a"
               href="/a"
-              role="menuitem"
               tabIndex={0}
             >
               <a
                 className="bx--header__menu-item"
                 href="/a"
-                role="menuitem"
                 tabIndex={0}
               >
                 <span
@@ -102,13 +99,11 @@ exports[`HeaderMenu should render 1`] = `
               className="bx--header__menu-item"
               element="a"
               href="/b"
-              role="menuitem"
               tabIndex={0}
             >
               <a
                 className="bx--header__menu-item"
                 href="/b"
-                role="menuitem"
                 tabIndex={0}
               >
                 <span
@@ -132,13 +127,11 @@ exports[`HeaderMenu should render 1`] = `
               className="bx--header__menu-item"
               element="a"
               href="/c"
-              role="menuitem"
               tabIndex={0}
             >
               <a
                 className="bx--header__menu-item"
                 href="/c"
-                role="menuitem"
                 tabIndex={0}
               >
                 <span

diff --git a/packages/react/src/components/UIShell/__tests__/__snapshots__/HeaderMenuItem-test.js.snap b/packages/react/src/components/UIShell/__tests__/__snapshots__/HeaderMenuItem-test.js.snap
@@ -12,12 +12,10 @@ exports[`HeaderMenuItem should render 1`] = `
     <Link
       className="bx--header__menu-item"
       element="a"
-      role="menuitem"
       tabIndex={0}
     >
       <a
         className="bx--header__menu-item"
-        role="menuitem"
         tabIndex={0}
       >
         <span