fix(web-components): fix WCAG 1.3.1 issue in popover-menu

Fix WCAG 1.3.1: Ensures elements with an ARIA role that require child roles contain them by modifying popover and menu-group components structure ref 1897 fix(web-components): fix WCAG 4.1.2 issue in popover-menu Fix WCAG 4.1.2: Ensures all ARIA attributes have valid values in popover by removing aria-controls/aria-owns in ic-menu-item for submenu parent buttons ref 1897
mi6 · Nov 13, 2024 · 53df1ac · 53df1ac
1 parent 211892c
commit 53df1ac
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 81 deletions.
diff --git a/packages/web-components/src/components/ic-menu-group/ic-menu-group.tsx b/packages/web-components/src/components/ic-menu-group/ic-menu-group.tsx
@@ -17,13 +17,13 @@ export class MenuGroup {
     const parentMenu = this.el.closest("ic-popover-menu");
 
     return (
-      <Host aria-label={this.label !== null ? this.label : ""}>
+      <Host role="group" aria-label={this.label !== null ? this.label : ""}>
         {isPropDefined(this.label) && (
           <ic-typography variant="subtitle-small">{this.label}</ic-typography>
         )}
-        <ul role="group">
+        <span class="menu-items-wrapper">
           <slot></slot>
-        </ul>
+        </span>
         {/* The line under the menu group is added on all menu groups except in the case that the menu group is the last item in the popover menu */}
         {this.el !== parentMenu?.querySelector("ic-menu-group:last-child") && (
           <hr />

diff --git a/...mponents/src/components/ic-menu-group/test/basic/__snapshots__/ic-menu-group.spec.ts.snap b/...mponents/src/components/ic-menu-group/test/basic/__snapshots__/ic-menu-group.spec.ts.snap
@@ -1,14 +1,14 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`menu group should render a menu group with a label 1`] = `
-<ic-menu-group aria-label="Menu item label" label="Menu item label">
+<ic-menu-group aria-label="Menu item label" label="Menu item label" role="group">
   <mock:shadow-root>
     <ic-typography variant="subtitle-small">
       Menu item label
     </ic-typography>
-    <ul role="group">
+    <span class="menu-items-wrapper">
       <slot></slot>
-    </ul>
+    </span>
     <hr>
   </mock:shadow-root>
   <ic-menu-item label="Button" variant="default">
@@ -83,11 +83,11 @@ exports[`menu group should render a menu group with a label 1`] = `
 `;
 
 exports[`menu group should render a menu group without a label 1`] = `
-<ic-menu-group>
+<ic-menu-group role="group">
   <mock:shadow-root>
-    <ul role="group">
+    <span class="menu-items-wrapper">
       <slot></slot>
-    </ul>
+    </span>
     <hr>
   </mock:shadow-root>
   <ic-menu-item label="Button" variant="default">

diff --git a/packages/web-components/src/components/ic-menu-item/ic-menu-item.tsx b/packages/web-components/src/components/ic-menu-item/ic-menu-item.tsx
@@ -224,7 +224,11 @@ export class MenuItem {
           role={this.variant === "toggle" ? "menuitemcheckbox" : "menuitem"}
           aria-disabled={`${this.disabled}`}
           aria-checked={
-            this.variant === "toggle" ? this.toggleChecked : undefined
+            this.variant === "toggle"
+              ? this.toggleChecked
+                ? "true"
+                : "false"
+              : undefined
           }
         >
           <ic-button
@@ -240,22 +244,12 @@ export class MenuItem {
             }
             aria-disabled={`${this.disabled}`}
             aria-label={this.getMenuItemAriaLabel()}
-            ariaControlsId={
-              isPropDefined(this.submenuTriggerFor)
-                ? `ic-popover-submenu-${this.submenuTriggerFor}`
-                : false
-            }
             aria-haspopup={
               isPropDefined(this.submenuTriggerFor) ||
               this.el.classList.contains("ic-popover-submenu-back-button")
                 ? "menu"
                 : false
             }
-            ariaOwnsId={
-              isPropDefined(this.submenuTriggerFor)
-                ? `ic-popover-submenu-${this.submenuTriggerFor}`
-                : false
-            }
           >
             <div class="focus-border">
               {isSlotUsed(this.el, "icon") && (

diff --git a/...components/src/components/ic-menu-item/test/basic/__snapshots__/ic-menu-item.spec.ts.snap b/...components/src/components/ic-menu-item/test/basic/__snapshots__/ic-menu-item.spec.ts.snap
@@ -66,7 +66,7 @@ exports[`menu item variants should render the destructive variant 1`] = `
 exports[`menu item variants should render the toggle variant 1`] = `
 <ic-menu-item id="test-menu-item" label="Toggle variant" variant="toggle">
   <mock:shadow-root>
-    <li aria-disabled="false" role="menuitemcheckbox">
+    <li aria-checked="false" aria-disabled="false" role="menuitemcheckbox">
       <ic-button class="button-size-default button-variant-tertiary full-width" exportparts="button">
         <mock:shadow-root>
           <button aria-disabled="false" aria-label="Toggle variant" class="button" part="button" type="button">

diff --git a/packages/web-components/src/components/ic-popover-menu/ic-popover-menu.tsx b/packages/web-components/src/components/ic-popover-menu/ic-popover-menu.tsx
@@ -296,7 +296,9 @@ export class PopoverMenu {
       if (el.tagName === "IC-MENU-ITEM") {
         this.popoverMenuEls.push(el);
       } else if (el.tagName === "IC-MENU-GROUP") {
-        const groupSlotWrapper = el.shadowRoot.querySelector("ul");
+        const groupSlotWrapper = el.shadowRoot.querySelector(
+          ".menu-items-wrapper"
+        );
         const menuGroupElements = getSlotElements(groupSlotWrapper);
 
         this.addMenuItems(menuGroupElements);
@@ -354,43 +356,45 @@ export class PopoverMenu {
           }}
           tabindex={open ? "0" : "-1"}
         >
-          <div
+          <span
             class={{
               "opening-from-parent": this.openingFromParent,
               "opening-from-child": this.openingFromChild,
             }}
           >
             {isPropDefined(this.submenuId) && (
-              <div>
-                <ic-menu-item
-                  class="ic-popover-submenu-back-button"
-                  ref={(el) => (this.backButton = el)}
-                  label="Back"
-                  onClick={this.handleBackButtonClick}
-                  id={`ic-popover-submenu-back-button-${this.submenuLevel}`}
-                >
-                  <svg
-                    slot="icon"
-                    viewBox="0 0 24 24"
-                    fill="none"
-                    xmlns="http://www.w3.org/2000/svg"
-                    class="submenu-back-icon"
+              <span>
+                <span role="menu">
+                  <ic-menu-item
+                    class="ic-popover-submenu-back-button"
+                    ref={(el) => (this.backButton = el)}
+                    label="Back"
+                    onClick={this.handleBackButtonClick}
+                    id={`ic-popover-submenu-back-button-${this.submenuLevel}`}
                   >
-                    <path
-                      d="M20 11H7.83L13.42 5.41L12 4L4 12L12 20L13.41 18.59L7.83 13H20V11Z"
-                      fill="currentColor"
-                    />
-                  </svg>
-                </ic-menu-item>
+                    <svg
+                      slot="icon"
+                      viewBox="0 0 24 24"
+                      fill="none"
+                      xmlns="http://www.w3.org/2000/svg"
+                      class="submenu-back-icon"
+                    >
+                      <path
+                        d="M20 11H7.83L13.42 5.41L12 4L4 12L12 20L13.41 18.59L7.83 13H20V11Z"
+                        fill="currentColor"
+                      />
+                    </svg>
+                  </ic-menu-item>
+                </span>
                 <ic-typography variant="subtitle-small" class="parent-label">
                   {this.parentLabel}
                 </ic-typography>
-              </div>
+              </span>
             )}
             <ul class="button" aria-label={this.getMenuAriaLabel()} role="menu">
               <slot></slot>
             </ul>
-          </div>
+          </span>
         </div>
       </Host>
     );

diff --git a/...ents/src/components/ic-popover-menu/test/basic/__snapshots__/ic-popover-menu.spec.ts.snap b/...ents/src/components/ic-popover-menu/test/basic/__snapshots__/ic-popover-menu.spec.ts.snap
@@ -4,37 +4,39 @@ exports[`ic-popover-menu should render a back button when submenu-id is set: sho
 <ic-popover-menu anchor="#anchorEl" aria-label="popover-menu" submenu-id="submenu">
   <mock:shadow-root>
     <div class="menu" id="ic-popover-submenu-submenu" tabindex="0">
-      <div>
-        <div>
-          <ic-menu-item class="ic-popover-submenu-back-button" id="ic-popover-submenu-back-button-1" variant="default">
-            <mock:shadow-root>
-              <li aria-disabled="false" role="menuitem">
-                <ic-button aria-disabled="false" aria-haspopup="menu" aria-label="Go back to parent menu" fullwidth="" variant="tertiary">
-                  <div class="focus-border">
-                    <span class="icon">
-                      <slot name="icon"></slot>
-                    </span>
-                    <div class="menu-item-info">
-                      <div class="menu-labels">
-                        <ic-typography class="menu-item-label">
-                          Back
-                        </ic-typography>
+      <span>
+        <span>
+          <span role="menu">
+            <ic-menu-item class="ic-popover-submenu-back-button" id="ic-popover-submenu-back-button-1" variant="default">
+              <mock:shadow-root>
+                <li aria-disabled="false" role="menuitem">
+                  <ic-button aria-disabled="false" aria-haspopup="menu" aria-label="Go back to parent menu" fullwidth="" variant="tertiary">
+                    <div class="focus-border">
+                      <span class="icon">
+                        <slot name="icon"></slot>
+                      </span>
+                      <div class="menu-item-info">
+                        <div class="menu-labels">
+                          <ic-typography class="menu-item-label">
+                            Back
+                          </ic-typography>
+                        </div>
                       </div>
                     </div>
-                  </div>
-                </ic-button>
-              </li>
-            </mock:shadow-root>
-            <svg class="submenu-back-icon" fill="none" slot="icon" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
-              <path d="M20 11H7.83L13.42 5.41L12 4L4 12L12 20L13.41 18.59L7.83 13H20V11Z" fill="currentColor"></path>
-            </svg>
-          </ic-menu-item>
+                  </ic-button>
+                </li>
+              </mock:shadow-root>
+              <svg class="submenu-back-icon" fill="none" slot="icon" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
+                <path d="M20 11H7.83L13.42 5.41L12 4L4 12L12 20L13.41 18.59L7.83 13H20V11Z" fill="currentColor"></path>
+              </svg>
+            </ic-menu-item>
+          </span>
           <ic-typography class="parent-label" variant="subtitle-small"></ic-typography>
-        </div>
+        </span>
         <ul aria-label="popover-menu, within nested level 1 undefined submenu," class="button" role="menu">
           <slot></slot>
         </ul>
-      </div>
+      </span>
     </div>
   </mock:shadow-root>
   <ic-menu-item label="Button 1" variant="default">
@@ -78,11 +80,11 @@ exports[`ic-popover-menu should render a menu item and menu group: should render
 <ic-popover-menu anchor="#anchorEl" aria-label="popover-menu">
   <mock:shadow-root>
     <div class="menu" id="ic-popover-submenu-undefined" tabindex="0">
-      <div>
+      <span>
         <ul aria-label="popover-menu" class="button" role="menu">
           <slot></slot>
         </ul>
-      </div>
+      </span>
     </div>
   </mock:shadow-root>
   <ic-menu-item label="Button 1" variant="default">
@@ -102,14 +104,14 @@ exports[`ic-popover-menu should render a menu item and menu group: should render
       </li>
     </mock:shadow-root>
   </ic-menu-item>
-  <ic-menu-group aria-label="Button group" label="Button group">
+  <ic-menu-group aria-label="Button group" label="Button group" role="group">
     <mock:shadow-root>
       <ic-typography variant="subtitle-small">
         Button group
       </ic-typography>
-      <ul role="group">
+      <span class="menu-items-wrapper">
         <slot></slot>
-      </ul>
+      </span>
     </mock:shadow-root>
     <ic-menu-item label="Group button 1" variant="destructive">
       <mock:shadow-root>
@@ -191,11 +193,11 @@ exports[`ic-popover-menu should render on a dialog 1`] = `
   <ic-popover-menu anchor="#anchorEl" aria-label="popover-menu" class="open" data-popper-placement="bottom-start" data-popper-reference-hidden open="" style="position: absolute; left: 0; top: 0; margin: 0; right: auto; bottom: auto; transform: translate(0px, 0px);">
     <mock:shadow-root>
       <div class="menu" id="ic-popover-submenu-undefined" tabindex="0">
-        <div>
+        <span>
           <ul aria-label="popover-menu" class="button" role="menu">
             <slot></slot>
           </ul>
-        </div>
+        </span>
       </div>
     </mock:shadow-root>
     <ic-menu-item label="Button 1" variant="default">
@@ -240,11 +242,11 @@ exports[`ic-popover-menu should render when anchor starts with #: should render
 <ic-popover-menu anchor="#anchorEl" aria-label="popover-menu">
   <mock:shadow-root>
     <div class="menu" id="ic-popover-submenu-undefined" tabindex="0">
-      <div>
+      <span>
         <ul aria-label="popover-menu" class="button" role="menu">
           <slot></slot>
         </ul>
-      </div>
+      </span>
     </div>
   </mock:shadow-root>
   <ic-menu-item label="Button 1" variant="default">
@@ -271,11 +273,11 @@ exports[`ic-popover-menu should render with anchor: should render with anchor 1`
 <ic-popover-menu anchor="anchorEl" aria-label="popover-menu">
   <mock:shadow-root>
     <div class="menu" id="ic-popover-submenu-undefined" tabindex="0">
-      <div>
+      <span>
         <ul aria-label="popover-menu" class="button" role="menu">
           <slot></slot>
         </ul>
-      </div>
+      </span>
     </div>
   </mock:shadow-root>
   <ic-menu-item label="Button 1" variant="default">