[CL-807] Improve aria a11y of nav group (#17078)

libs/components/src/navigation/nav-group.component.html

@@ -8,7 +8,8 @@
     [routerLinkActiveOptions]="routerLinkActiveOptions()"
     (mainContentClicked)="handleMainContentClicked()"
     [ariaLabel]="ariaLabel()"
-    [hideActiveStyles]="parentHideActiveStyles"
+    [hideActiveStyles]="parentHideActiveStyles()"
+    [ariaCurrentWhenActive]="ariaCurrent()"
   >
     <ng-template #button>
       <button
@@ -18,7 +19,6 @@
         [buttonType]="'nav-contrast'"
         (click)="toggle($event)"
         size="small"
-        aria-haspopup="true"
         [attr.aria-expanded]="open().toString()"
         [attr.aria-controls]="contentId"
         [label]="['toggleCollapse' | i18n, text()].join(' ')"
@@ -30,7 +30,7 @@
     </ng-container>
   </bit-nav-item>
   <!-- [attr.aria-controls] of the above button expects a unique ID on the controlled element -->
-  @if (sideNavService.open$ | async) {
+  @if (sideNavOpen()) {
     @if (open()) {
       <div
         [attr.id]="contentId"

libs/components/src/navigation/nav-group.component.ts

@@ -9,7 +9,10 @@ import {
   input,
   model,
   contentChildren,
+  computed,
 } from "@angular/core";
+import { toSignal } from "@angular/core/rxjs-interop";
+import { RouterLinkActive } from "@angular/router";
 
 import { I18nPipe } from "@bitwarden/ui-common";
 
@@ -33,10 +36,33 @@ import { SideNavService } from "./side-nav.service";
 export class NavGroupComponent extends NavBaseComponent {
   readonly nestedNavComponents = contentChildren(NavBaseComponent, { descendants: true });
 
+  readonly sideNavOpen = toSignal(this.sideNavService.open$);
+
+  readonly sideNavAndGroupOpen = computed(() => {
+    return this.open() && this.sideNavOpen();
+  });
+
   /** When the side nav is open, the parent nav item should not show active styles when open. */
-  protected get parentHideActiveStyles(): boolean {
+  readonly parentHideActiveStyles = computed(() => {
-    return this.hideActiveStyles() || (this.open() && this.sideNavService.open);
+    return this.hideActiveStyles() || this.sideNavAndGroupOpen();
-  }
+  });
+
+  /**
+   * Allow overriding of the RouterLink['ariaCurrentWhenActive'] property.
+   *
+   * By default, assuming that the nav group navigates to its first child page instead of its
+   * own page, the nav group will be `current` when the side nav is collapsed or the nav group
+   * is collapsed (since child pages don't show in either collapsed view) and not `current`
+   * when the side nav and nav group are open (since the child page will show as `current`).
+   *
+   * If the nav group navigates to its own page, use this property to always set it to announce
+   * as `current` by passing in `"page"`.
+   */
+  readonly ariaCurrentWhenActive = input<RouterLinkActive["ariaCurrentWhenActive"]>();
+
+  readonly ariaCurrent = computed(() => {
+    return this.ariaCurrentWhenActive() ?? (this.sideNavAndGroupOpen() ? undefined : "page");
+  });
 
   /**
    * UID for `[attr.aria-controls]`

libs/components/src/navigation/nav-item.component.html

@@ -11,7 +11,7 @@
       ]"
     >
       <div class="tw-relative tw-flex tw-items-center tw-h-full">
-        <ng-container *ngIf="route; then isAnchor; else isButton"></ng-container>
+        <ng-container *ngIf="route(); then isAnchor; else isButton"></ng-container>
 
         <!-- Main content of `NavItem` -->
         <ng-template #anchorAndButtonContent>
@@ -31,7 +31,7 @@
           </div>
         </ng-template>
 
-        <!-- Show if a value was passed to `this.to` -->
+        <!-- Show if a value was passed to `this.route` -->
         <ng-template #isAnchor>
           <!-- The `data-fvw` attribute passes focus to `this.focusVisibleWithin$` -->
           <!-- The following `class` field should match the `#isButton` class field below -->
@@ -43,7 +43,7 @@
             [attr.aria-label]="ariaLabel() || text()"
             routerLinkActive
             [routerLinkActiveOptions]="routerLinkActiveOptions()"
-            [ariaCurrentWhenActive]="'page'"
+            [ariaCurrentWhenActive]="ariaCurrentWhenActive()"
             (isActiveChange)="setIsActive($event)"
             (click)="mainContentClicked.emit()"
           >
@@ -51,12 +51,13 @@
           </a>
         </ng-template>
 
-        <!-- Show if `this.to` is falsy -->
+        <!-- Show if `this.route` is falsy -->
         <ng-template #isButton>
           <!-- Class field should match `#isAnchor` class field above -->
           <button
             type="button"
-            class="tw-size-full tw-px-4 tw-pe-3 tw-truncate tw-border-none tw-bg-transparent tw-text-start !tw-text-alt2 hover:tw-text-alt2 hover:tw-no-underline focus:tw-outline-none [&_i]:tw-leading-[1.5rem]"
+            class="tw-size-full tw-px-4 tw-truncate tw-border-none tw-bg-transparent tw-text-start !tw-text-alt2 hover:tw-text-alt2 hover:tw-no-underline focus:tw-outline-none [&_i]:tw-leading-[1.5rem]"
+            [ngClass]="open ? 'tw-pe-3' : 'tw-pe-4'"
             data-fvw
             (click)="mainContentClicked.emit()"
           >

libs/components/src/navigation/nav-item.component.ts

@@ -1,6 +1,6 @@
 import { CommonModule } from "@angular/common";
 import { Component, HostListener, Optional, input } from "@angular/core";
-import { RouterModule } from "@angular/router";
+import { RouterLinkActive, RouterModule } from "@angular/router";
 import { BehaviorSubject, map } from "rxjs";
 
 import { IconButtonModule } from "../icon-button";
@@ -39,6 +39,14 @@ export class NavItemComponent extends NavBaseComponent {
     return this.forceActiveStyles() || (this._isActive && !this.hideActiveStyles());
   }
 
+  /**
+   * Allow overriding of the RouterLink['ariaCurrentWhenActive'] property.
+   *
+   * Useful for situations like nav-groups that navigate to their first child page and should
+   * not be marked `current` while the child page is marked as `current`
+   */
+  readonly ariaCurrentWhenActive = input<RouterLinkActive["ariaCurrentWhenActive"]>("page");
+
   /**
    * The design spec calls for the an outline to wrap the entire element when the template's
    * anchor/button has :focus-visible. Usually, we would use :focus-within for this. However, that