fix(sidenav): express hierarchy using list and listitem

* fix(sidenav): [Bug][a11y]: sidenav should express hierarchy using list and listitem #3348
* fix(sidenav): updates per code review #3348

packages/sidenav/src/Sidenav.ts

@@ -23,6 +23,7 @@ import sidenavStyles from './sidenav.css.js';
 import { Focusable } from '@spectrum-web-components/shared';
 import { SideNavItem } from './SidenavItem.js';
 import { SideNavHeading } from './SidenavHeading.js';
+import { ifDefined } from 'lit/directives/if-defined.js';
 
 export interface SidenavSelectDetail {
     value: string;
@@ -54,13 +55,25 @@ export class SideNav extends Focusable {
 
     rovingTabindexController = new RovingTabindexController<SideNavItem>(this, {
         focusInIndex: (elements: SideNavItem[]) => {
-            return elements.findIndex((el) => {
+            let parentSideNavItem: SideNavItem | undefined;
+            let index = elements.findIndex((el) => {
+                // If the selected item's parent is collapsed, save it for later.
+                if (el.value === this.value && this.isDisabledChild(el)) {
+                    parentSideNavItem = el.closest(
+                        'sp-sidenav-item:not([expanded])'
+                    ) as SideNavItem;
+                }
                 return this.value
                     ? !el.disabled &&
                           !this.isDisabledChild(el) &&
                           el.value === this.value
                     : !el.disabled && !this.isDisabledChild(el);
             });
+            // If the selected item's parent is collapsed, focus the collapsed parent.
+            if (index === -1 && parentSideNavItem) {
+                index = elements.findIndex((el) => el === parentSideNavItem);
+            }
+            return index;
         },
         direction: 'vertical',
         elements: () =>
@@ -81,6 +94,15 @@ export class SideNav extends Focusable {
     @property({ reflect: true })
     public variant?: 'multilevel' = undefined;
 
+    /**
+     * An accessible label that describes the component,
+     * so that the side navigation can be distinguished
+     * from other navigation by screen reader users.
+     * It will be applied to aria-label, but not visually rendered.
+     */
+    @property({ reflect: true })
+    public label?: string | undefined = undefined;
+
     private handleSelect(
         event: CustomEvent<SidenavSelectDetail> & { target: SideNavItem }
     ): void {
@@ -162,11 +184,16 @@ export class SideNav extends Focusable {
 
     protected override render(): TemplateResult {
         return html`
-            <nav @sidenav-select=${this.handleSelect}>
-                <slot
-                    name="descendant"
-                    @slotchange=${this.handleSlotchange}
-                ></slot>
+            <nav
+                @sidenav-select=${this.handleSelect}
+                aria-label=${ifDefined(this.label)}
+            >
+                <div role="list">
+                    <slot
+                        name="descendant"
+                        @slotchange=${this.handleSlotchange}
+                    ></slot>
+                </div>
             </nav>
         `;
     }

packages/sidenav/src/SidenavHeading.ts

@@ -45,9 +45,14 @@ export class SideNavHeading extends SpectrumElement {
     protected override render(): TemplateResult {
         return html`
             <h2 id="heading">${this.label}</h2>
-            <div id="list" aria-labelledby="heading">
+            <div id="list" aria-labelledby="heading" role="list">
                 <slot name="descendant"></slot>
             </div>
         `;
     }
+
+    protected override firstUpdated(changed: PropertyValues<this>): void {
+        super.firstUpdated(changed);
+        this.setAttribute('role', 'listitem');
+    }
 }

packages/sidenav/src/SidenavItem.ts

@@ -129,22 +129,37 @@ export class SideNavItem extends LikeAnchor(Focusable) {
                 aria-current=${ifDefined(
                     this.selected && this.href ? 'page' : undefined
                 )}
+                aria-expanded=${ifDefined(
+                    this.hasChildren ? this.expanded : undefined
+                )}
+                aria-controls=${ifDefined(
+                    this.hasChildren && this.expanded ? 'list' : undefined
+                )}
             >
                 <slot name="icon"></slot>
                 ${this.label}
                 <slot></slot>
             </a>
             ${this.expanded
                 ? html`
-                      <slot name="descendant"></slot>
+                      <div id="list" aria-labelledby="item-link" role="list">
+                          <slot name="descendant"></slot>
+                      </div>
                   `
                 : html``}
         `;
     }
 
     protected override updated(changes: PropertyValues): void {
-        if (this.hasChildren && this.expanded && !this.selected) {
+        if (
+            this.hasChildren &&
+            this.expanded &&
+            !this.selected &&
+            this.parentSideNav?.manageTabIndex
+        ) {
             this.focusElement.tabIndex = -1;
+        } else {
+            this.focusElement.removeAttribute('tabindex');
         }
         super.updated(changes);
     }
@@ -186,4 +201,9 @@ export class SideNavItem extends LikeAnchor(Focusable) {
         }
         this._parentSidenav = undefined;
     }
+
+    protected override firstUpdated(changed: PropertyValues<this>): void {
+        super.firstUpdated(changed);
+        this.setAttribute('role', 'listitem');
+    }
 }

projects/documentation/content/_includes/partials/sidenav.njk

@@ -2,7 +2,7 @@
 {% if componentName %}
   {% set value = componentName %}
 {% endif %}
-<sp-sidenav manage-tab-index slot="side-nav" value="{{ value }}">
+<sp-sidenav manage-tab-index slot="side-nav" value="{{ value }}" label="Spectrum Web Components">
     <sp-sidenav-item
         label="Getting started"
         href="/getting-started"