fix(context-menu): fix keyboard focus (#408)

allianz · Nov 15, 2021 · f48e824 · f48e824
1 parent 9febc8c
commit f48e824
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 86 deletions.
diff --git a/projects/ng-aquila/src/context-menu/context-menu-item.component.scss b/projects/ng-aquila/src/context-menu/context-menu-item.component.scss
@@ -84,7 +84,7 @@
     }
   }
 
-  &.cdk-keyboard-focused .nx-context-menu-item__content-wrapper {
+  &.cdk-keyboard-focused:focus .nx-context-menu-item__content-wrapper {
     border-radius: nx-border-radius(s);
     outline: 0;
     @include focus-style;

diff --git a/projects/ng-aquila/src/context-menu/context-menu-item.component.ts b/projects/ng-aquila/src/context-menu/context-menu-item.component.ts
@@ -10,9 +10,8 @@ import {
 } from '@angular/core';
 import { Subject } from 'rxjs';
 import { DOCUMENT } from '@angular/common';
-import { NxContextMenuComponent } from './context-menu.component';
 import { coerceBooleanProperty, BooleanInput } from '@angular/cdk/coercion';
-import { FocusMonitor } from '@angular/cdk/a11y';
+import { FocusMonitor, FocusOrigin } from '@angular/cdk/a11y';
 
 /**
  * This directive is intended to be used inside an nx-context-menu tag.
@@ -71,29 +70,23 @@ export class NxContextMenuItemComponent implements OnDestroy {
     private _elementRef: ElementRef<HTMLElement>,
     @Inject(DOCUMENT) document: any,
     private _changeDetectorRef: ChangeDetectorRef,
-    @Inject(NxContextMenuComponent)
-    @Optional() private _parentMenu: NxContextMenuComponent,
     private _focusMonitor: FocusMonitor
   ) {
 
-    if (_parentMenu && _parentMenu.addItem) {
-      _parentMenu.addItem(this);
-    }
-
     this._document = document;
     this._focusMonitor.monitor(this._elementRef);
   }
 
   /** Focuses this context menu item. */
-  focus(): void {
-    this._getHostElement().focus();
+  focus(origin?: FocusOrigin): void {
+    if (origin) {
+      this._focusMonitor.focusVia(this._getHostElement(), origin);
+    } else {
+      this._getHostElement().focus();
+    }
   }
 
   ngOnDestroy() {
-    if (this._parentMenu && this._parentMenu.removeItem) {
-      this._parentMenu.removeItem(this);
-    }
-
     this._hovered.complete();
     this._focusMonitor.stopMonitoring(this._elementRef);
   }

diff --git a/projects/ng-aquila/src/context-menu/context-menu-trigger.directive.ts b/projects/ng-aquila/src/context-menu/context-menu-trigger.directive.ts
@@ -1,4 +1,6 @@
 import { Direction, Directionality } from '@angular/cdk/bidi';
+import { FocusOrigin } from '@angular/cdk/a11y';
+
 import { LEFT_ARROW, RIGHT_ARROW } from '@angular/cdk/keycodes';
 import {
   ConnectedPosition,
@@ -40,11 +42,6 @@ export const MENU_PANEL_OFFSET_X = 8;
 
 export type NxContextMenuScrollStrategy = 'close' | 'reposition';
 
-/** Options for binding a passive event listener. */
-const passiveEventListenerOptions = normalizePassiveListenerOptions({
-  passive: true
-});
-
 /**
  * This directive is intended to be used in conjunction with an nx-context-menu tag.
  * It is responsible for toggling the display of the provided context menu instance.
@@ -180,14 +177,14 @@ export class NxContextMenuTriggerDirective
   }
 
   /** Toggles the context menu between the open and closed states. */
-  toggleContextMenu(): void {
+  toggleContextMenu(origin?: FocusOrigin): void {
     return this.contextMenuOpen
       ? this.closeContextMenu()
-      : this.openContextMenu();
+      : this.openContextMenu(origin);
   }
 
   /** Opens the context menu. */
-  openContextMenu(): void {
+  openContextMenu(origin?: FocusOrigin): void {
     if (this.contextMenuOpen) {
       return;
     }
@@ -209,7 +206,8 @@ export class NxContextMenuTriggerDirective
     this._closingActionsSubscription = this._contextMenuClosingActions().subscribe(
       () => this.closeContextMenu()
     );
-    this._initContextMenu();
+
+    this._initContextMenu(origin);
 
     if (this.contextMenu instanceof NxContextMenuComponent) {
       this.contextMenu._startAnimation();
@@ -265,13 +263,13 @@ export class NxContextMenuTriggerDirective
    * This method sets the context menu state to open and focuses the first item if
    * the context menu was opened via the keyboard.
    */
-  private _initContextMenu(): void {
+  private _initContextMenu(origin?: FocusOrigin): void {
     this.contextMenu.parentMenu = this.triggersSubmenu()
       ? this._parentMenu
       : undefined;
     this.contextMenu.direction = this.dir;
     this._setIsContextMenuOpen(true);
-    this.contextMenu.focusFirstItem();
+    this.contextMenu.focusFirstItem(origin);
   }
 
   /**
@@ -452,18 +450,20 @@ export class NxContextMenuTriggerDirective
       ((keyCode === RIGHT_ARROW && this.dir === 'ltr') ||
         (keyCode === LEFT_ARROW && this.dir === 'rtl'))
     ) {
-      this.openContextMenu();
+      this.openContextMenu('keyboard');
     }
   }
 
   /** Handles click events on the trigger. */
   _handleClick(event: MouseEvent): void {
+    const origin: FocusOrigin = event.detail ? 'program' : 'keyboard';
+
     if (this.triggersSubmenu()) {
       // Stop event propagation to avoid closing the parent menu.
       event.stopPropagation();
-      this.openContextMenu();
+      this.openContextMenu(origin);
     } else {
-      this.toggleContextMenu();
+      this.toggleContextMenu(origin);
     }
   }
 
@@ -508,9 +508,9 @@ export class NxContextMenuTriggerDirective
               delay(0, asapScheduler),
               takeUntil(this._parentMenu._hovered())
             )
-            .subscribe(() => this.openContextMenu());
+            .subscribe(() => this.openContextMenu('mouse'));
         } else {
-          this.openContextMenu();
+          this.openContextMenu('mouse');
         }
       });
   }

diff --git a/projects/ng-aquila/src/context-menu/context-menu.component.ts b/projects/ng-aquila/src/context-menu/context-menu.component.ts
@@ -1,4 +1,4 @@
-import { FocusKeyManager } from '@angular/cdk/a11y';
+import { FocusKeyManager, FocusOrigin } from '@angular/cdk/a11y';
 import { Direction } from '@angular/cdk/bidi';
 import {
   ESCAPE,
@@ -19,8 +19,10 @@ import {
   Output,
   TemplateRef,
   ViewChild,
+  ContentChildren,
+  QueryList
 } from '@angular/core';
-import { merge, Observable, Subject, Subscription } from 'rxjs';
+import { merge, Observable, ReplaySubject, Subject, Subscription } from 'rxjs';
 import { startWith, switchMap, take } from 'rxjs/operators';
 import { nxContextMenuAnimations } from './context-menu-animations';
 import { NxContextMenuContentDirective } from './context-menu-content.directive';
@@ -41,15 +43,14 @@ export class NxContextMenuComponent
   implements AfterContentInit, OnDestroy {
   private _keyManager!: FocusKeyManager<NxContextMenuItemComponent>;
 
-  /** Menu items inside the current menu. */
-  private _items: NxContextMenuItemComponent[] = [];
-
-  /** Emits whenever the amount of menu items changes. */
-  private _itemChanges = new Subject<NxContextMenuItemComponent[]>();
+  @ContentChildren(NxContextMenuItemComponent)
+  private _items!: QueryList<NxContextMenuItemComponent>;
 
   /** Subscription to tab events on the menu panel */
   private _tabSubscription = Subscription.EMPTY;
 
+  private _init: ReplaySubject<void> = new ReplaySubject(1);
+
   /** Config object to be passed into the menu's ngClass */
   _classList: { [key: string]: boolean } = {};
 
@@ -92,22 +93,27 @@ export class NxContextMenuComponent
         this._items
       )
       .withWrap()
-      .withTypeAhead();
+      .withTypeAhead()
+      .setFocusOrigin('keyboard');
     this._tabSubscription = this._keyManager.tabOut.subscribe(() =>
       this.closed.emit('tab')
     );
+    this._init.next();
   }
 
   ngOnDestroy() {
     this._tabSubscription.unsubscribe();
     this.closed.complete();
+    this._init.complete();
   }
 
   /** Stream that emits whenever the hovered menu item changes. */
   _hovered(): Observable<NxContextMenuItemComponent> {
-    return this._itemChanges.pipe(
-      startWith(this._items),
-      switchMap(items => merge(...items.map(item => item._hovered)))
+    return this._init.pipe(
+      switchMap(() => this._items.changes.pipe(startWith(this._items))),
+      switchMap((items: QueryList<NxContextMenuItemComponent>) =>
+        merge(...items.map(item => item._hovered))
+      )
     );
   }
 
@@ -150,18 +156,15 @@ export class NxContextMenuComponent
   /**
    * Focus the first item in the menu.
    */
-  focusFirstItem(): void {
+  focusFirstItem(origin?: FocusOrigin): void {
     // When the content is rendered lazily, it takes a bit before the items are inside the DOM.
-    if (this.lazyContent) {
-      this._ngZone.onStable
-        .asObservable()
-        .pipe(take(1))
-        .subscribe(() =>
-          this._keyManager.setFirstItemActive()
-        );
-    } else {
-      this._keyManager.setFirstItemActive();
-    }
+    this._ngZone.onStable
+      .asObservable()
+      .pipe(take(1))
+      .subscribe(() => {
+        this._keyManager.setFirstItemActive();
+        this._keyManager.activeItem?.focus(origin);
+      });
   }
 
   /**
@@ -172,35 +175,6 @@ export class NxContextMenuComponent
     this._keyManager.setActiveItem(-1);
   }
 
-  /**
-   * Registers a menu item with the context menu.
-   * @docs-private
-   */
-  addItem(item: NxContextMenuItemComponent) {
-    // We register the items through this method, rather than picking them up through
-    // `ContentChildren`, because we need the items to be picked up by their closest
-    // `nx-context-menu` ancestor. If we used `@ContentChildren(NxContextMenuItem, {descendants: true})`,
-    // all descendant items will bleed into the top-level menu in the case where the consumer
-    // has `nx-context-menu` instances nested inside each other.
-    if (this._items.indexOf(item) === -1) {
-      this._items.push(item);
-      this._itemChanges.next(this._items);
-    }
-  }
-
-  /**
-   * Removes an item from the context menu.
-   * @docs-private
-   */
-  removeItem(item: NxContextMenuItemComponent) {
-    const index = this._items.indexOf(item);
-
-    if (this._items.indexOf(item) > -1) {
-      this._items.splice(index, 1);
-      this._itemChanges.next(this._items);
-    }
-  }
-
   /** Starts the enter animation. */
   _startAnimation() {
     this._panelAnimationState = 'enter';

diff --git a/projects/ng-aquila/src/context-menu/context-menu.spec.ts b/projects/ng-aquila/src/context-menu/context-menu.spec.ts
@@ -27,7 +27,6 @@ import {
 
 import { Subject } from 'rxjs';
 import { ScrollDispatcher } from '@angular/cdk/scrolling';
-import { FocusMonitor } from '@angular/cdk/a11y';
 import { NxIconModule } from '../icon/public-api';
 import {
   dispatchKeyboardEvent,
@@ -72,7 +71,7 @@ describe('nxContextMenu', () => {
       providers
     }).compileComponents();
 
-    inject([OverlayContainer, FocusMonitor], (oc: OverlayContainer, fm: FocusMonitor) => {
+    inject([OverlayContainer], (oc: OverlayContainer) => {
       overlayContainer = oc;
       overlayContainerElement = oc.getContainerElement();
     })();
@@ -1221,7 +1220,10 @@ describe('nxContextMenu', () => {
       const repeaterFixture = createComponent(NestedMenuRepeater);
       overlay = overlayContainerElement;
 
-      expect(() => repeaterFixture.detectChanges()).not.toThrow();
+      expect(() => {
+        repeaterFixture.detectChanges();
+        flush();
+      }).not.toThrow();
 
       repeaterFixture.componentInstance.rootTriggerEl.nativeElement.click();
       repeaterFixture.detectChanges();
@@ -1333,6 +1335,7 @@ describe('nxContextMenu', () => {
       const trigger = fixture.componentInstance.trigger;
       trigger.openContextMenu();
       fixture.detectChanges();
+      flush();
 
       const direction = (trigger as any)._overlayRef.getDirection();
       expect(direction).toBe('ltr');
@@ -1349,6 +1352,7 @@ describe('nxContextMenu', () => {
       const trigger = fixture.componentInstance.trigger;
       trigger.openContextMenu();
       fixture.detectChanges();
+      flush();
       const direction = (trigger as any)._overlayRef.getDirection();
       expect(direction).toBe('rtl');
     }));
@@ -1367,6 +1371,7 @@ describe('nxContextMenu', () => {
       spyOn(trigger, 'closeContextMenu');
       trigger.openContextMenu();
       fixture.detectChanges();
+      flush();
       changeEmitter.emit('ltr');
       expect(trigger.closeContextMenu).toHaveBeenCalledTimes(1);
       changeEmitter.complete();

diff --git a/projects/ng-aquila/src/table/sort-header/sort.spec.ts b/projects/ng-aquila/src/table/sort-header/sort.spec.ts
@@ -1,5 +1,5 @@
 import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
-import { ViewChild, Component, Type, Directive } from '@angular/core';
+import { ViewChild, Component, Type, Directive, Injectable } from '@angular/core';
 import { inject } from '@angular/core/testing';
 import { SortDirection, SortEvent } from './sort.directive';
 import { NxSortHeaderComponent } from './sort-header.component';
@@ -13,6 +13,7 @@ interface DataStructure {
   count: number;
 }
 
+@Injectable()
 class MyIntl extends NxSortHeaderIntl {
   sortAscendingAriaLabel = 'aufsteigend sortieren';
   sortDescendingAriaLabel = 'absteigend sortieren';