fix(material): recover keyboard navigation trough menus

klemenoslaj · Jan 24, 2021 · fe033f4 · fe033f4
1 parent fc3ae86
commit fe033f4
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 109 deletions.
diff --git a/projects/material/src/lib/action-mat-anchor.component.ts b/projects/material/src/lib/action-mat-anchor.component.ts
@@ -1,38 +1,24 @@
-import { Component, Input, ChangeDetectionStrategy, ViewEncapsulation, ElementRef, HostBinding, Inject } from '@angular/core';
+import { Component, Input, ChangeDetectionStrategy, ViewEncapsulation, ElementRef, Inject, HostBinding } from '@angular/core';
 import { ActionAnchor, ActionAnchorComponentImpl } from '@ng-action-outlet/core';
 import { FocusableOption, FocusMonitor, FocusOrigin } from '@angular/cdk/a11y';
 
-import { isMenuItem } from './common';
 import { actionMatButtonTemplate } from './action-mat-button.template';
 import { ICON_TYPE, ACTION_ICON_TYPE_TOKEN } from './action-icon-type-token';
 
-export const AnchorType = <const>{
-  Link: 0,
-  MenuLink: 1
-};
-
 @Component({
   selector: 'action-mat-anchor',
   template: `
-    <ng-container *ngIf="(_action?.visible$ | async) && _action; let action" [ngSwitch]="_getAnchorType(action!)">
+    <ng-container *ngIf="(_action?.visible$ | async) && _action; let action">
       <ng-container *ngIf="(action.href$ | async); let href" [ngTemplateOutlet]="action.isExternalLink() ? external : internal" [ngTemplateOutletContext]="{ $implicit: href }"></ng-container>
 
       <ng-template #external let-href>
-        <a *ngSwitchCase="AnchorType.Link" mat-button [actionMatButton]="action" [href]="href" [attr.target]="action.target$ | async">
-          <ng-container *ngTemplateOutlet="content; context: { $implicit: action }"></ng-container>
-        </a>
-
-        <a *ngSwitchCase="AnchorType.MenuLink" mat-menu-item [actionMatButton]="action" [href]="href" [attr.target]="action.target$ | async">
+        <a mat-button [actionMatButton]="action" [href]="href" [attr.target]="action.target$ | async">
           <ng-container *ngTemplateOutlet="content; context: { $implicit: action }"></ng-container>
         </a>
       </ng-template>
 
       <ng-template #internal let-href>
-        <a *ngSwitchCase="AnchorType.Link" mat-button [actionMatButton]="action" [routerLink]="href" [target]="action.target$ | async">
-          <ng-container *ngTemplateOutlet="content; context: { $implicit: action }"></ng-container>
-        </a>
-
-        <a *ngSwitchCase="AnchorType.MenuLink" mat-menu-item [actionMatButton]="action" [routerLink]="href" [target]="action.target$ | async">
+        <a mat-button [actionMatButton]="action" [routerLink]="href" [target]="action.target$ | async">
           <ng-container *ngTemplateOutlet="content; context: { $implicit: action }"></ng-container>
         </a>
       </ng-template>
@@ -45,13 +31,11 @@ export const AnchorType = <const>{
   encapsulation: ViewEncapsulation.None,
 })
 export class ActionMatAnchorComponent implements ActionAnchorComponentImpl, FocusableOption {
+  @HostBinding('class.action-contents') _contentsClass = true;
+
   @Input('action')
   readonly _action?: ActionAnchor | null;
 
-  @HostBinding('class')
-  readonly _classname = 'action-mat-button';
-
-  AnchorType = AnchorType;
 
   constructor(
     @Inject(ACTION_ICON_TYPE_TOKEN)
@@ -67,8 +51,4 @@ export class ActionMatAnchorComponent implements ActionAnchorComponentImpl, Focu
       this._elementRef.nativeElement.focus(options);
     }
   }
-
-  _getAnchorType(action: ActionAnchor) {
-    return isMenuItem(action.getParent()) ? AnchorType.MenuLink : AnchorType.Link;
-  }
 }
diff --git a/projects/material/src/lib/action-mat-button.component.scss b/projects/material/src/lib/action-mat-button.component.scss
@@ -1,12 +1,12 @@
-.action-mat-button {
+.action-contents {
   display: contents;
 }
 
-.action-mat-button .mat-button,
-.action-mat-button .mat-icon-button {
+.action-mat-button.mat-button,
+.action-mat-button.mat-icon-button {
   vertical-align: top;
 }
 
-.action-mat-button a:hover {
+a.action-mat-button:hover {
   text-decoration: underline;
 }
diff --git a/projects/material/src/lib/action-mat-button.component.ts b/projects/material/src/lib/action-mat-button.component.ts
@@ -1,25 +1,14 @@
-import { Component, Input, ChangeDetectionStrategy, ViewEncapsulation, ElementRef, HostBinding, Inject } from '@angular/core';
+import { Component, Input, ChangeDetectionStrategy, ViewEncapsulation, HostBinding, Inject } from '@angular/core';
 import { ActionButton, ActionButtonComponentImpl } from '@ng-action-outlet/core';
-import { FocusableOption, FocusMonitor, FocusOrigin } from '@angular/cdk/a11y';
 
-import { isMenuItem } from './common';
 import { actionMatButtonTemplate } from './action-mat-button.template';
 import { ICON_TYPE, ACTION_ICON_TYPE_TOKEN } from './action-icon-type-token';
 
-export const ButtonType = <const>{
-  Button: 0,
-  MenuItem: 1
-};
-
 @Component({
   selector: 'action-mat-button',
   template: `
-    <ng-container *ngIf="_action && _action.visible$ | async" [ngSwitch]="_getButtonType(_action!)">
-      <button *ngSwitchCase="ButtonType.Button" mat-button [actionMatButton]="_action">
-        <ng-container *ngTemplateOutlet="content; context: { $implicit: _action }"></ng-container>
-      </button>
-
-      <button *ngSwitchCase="ButtonType.MenuItem" mat-menu-item [actionMatButton]="_action">
+    <ng-container *ngIf="_action && _action.visible$ | async">
+      <button mat-button [actionMatButton]="_action">
         <ng-container *ngTemplateOutlet="content; context: { $implicit: _action }"></ng-container>
       </button>
     </ng-container>
@@ -30,32 +19,14 @@ export const ButtonType = <const>{
   changeDetection: ChangeDetectionStrategy.OnPush,
   encapsulation: ViewEncapsulation.None,
 })
-export class ActionMatButtonComponent implements ActionButtonComponentImpl, FocusableOption {
+export class ActionMatButtonComponent implements ActionButtonComponentImpl {
+  @HostBinding('class.action-contents') _contentsClass = true;
+
   @Input('action')
   readonly _action?: ActionButton | null;
 
-  @HostBinding('class')
-  readonly _classname = 'action-mat-button';
-
-  ButtonType = ButtonType;
-
   constructor(
     @Inject(ACTION_ICON_TYPE_TOKEN)
     readonly _iconType: ICON_TYPE,
-    private readonly _elementRef: ElementRef,
-    private readonly _focusMonitor?: FocusMonitor,
   ) {}
-
-  focus(origin: FocusOrigin = 'program', options?: FocusOptions): void {
-    if (this._focusMonitor) {
-      this._focusMonitor.focusVia(this._elementRef.nativeElement, origin, options);
-    } else {
-      this._elementRef.nativeElement.focus(options);
-    }
-  }
-
-  _getButtonType(action: ActionButton) {
-    const isButtonMenuItem = isMenuItem(action.getParent());
-    return isButtonMenuItem ? ButtonType.MenuItem : ButtonType.Button;
-  }
 }
diff --git a/projects/material/src/lib/action-mat-button.directive.ts b/projects/material/src/lib/action-mat-button.directive.ts
@@ -2,7 +2,7 @@ import { Directive, Input, OnDestroy, ElementRef, Renderer2, Optional, Inject, C
 import { MatMenuTrigger, MatMenuItem } from '@angular/material/menu';
 import { MatButton } from '@angular/material/button';
 import { ActionButton, ActionGroup, ActionAnchor } from '@ng-action-outlet/core';
-import { Subject, fromEvent, ReplaySubject, EMPTY } from 'rxjs';
+import { Subject, fromEvent, ReplaySubject } from 'rxjs';
 import { takeUntil, switchMap, filter } from 'rxjs/operators';
 
 @Directive({
@@ -28,18 +28,26 @@ export class ActionMatButtonDirective implements OnDestroy {
     renderer: Renderer2,
     cdRef: ChangeDetectorRef,
   ) {
-    this._action$.pipe(
-      switchMap(action => action instanceof ActionAnchor ? EMPTY : action.disabled$),
-      takeUntil(this._unsubscribe$),
-    ).subscribe(disabled => {
-      // Either MatButton, either MatMenuItem should always be present.
-      // tslint:disable-next-line: no-non-null-assertion
-      (matButton ?? matMenuItem)!.disabled = disabled;
-      cdRef.markForCheck();
-    });
-
+    renderer.addClass(nativeElement, 'action-mat-button');
     if (nativeElement.tagName === 'BUTTON') {
       renderer.setAttribute(nativeElement, 'type', 'button');
+
+      if (!matMenuTrigger) {
+        fromEvent(nativeElement, 'click').pipe(
+          switchMap(() => this._action$),
+          takeUntil(this._unsubscribe$),
+        ).subscribe(action => action.trigger());
+      }
+
+      this._action$.pipe(
+        switchMap(action => action.disabled$),
+        takeUntil(this._unsubscribe$),
+      ).subscribe(disabled => {
+        // Either MatButton, either MatMenuItem should always be present.
+        // tslint:disable-next-line: no-non-null-assertion
+        (matButton ?? matMenuItem)!.disabled = disabled;
+        cdRef.markForCheck();
+      });
     }
 
     const ariaLabel$ = this._action$.pipe(switchMap(action => action.ariaLabel$));
@@ -69,13 +77,6 @@ export class ActionMatButtonDirective implements OnDestroy {
         renderer.addClass(nativeElement, 'mat-icon-button');
       });
     }
-
-    if (!matMenuTrigger) {
-      fromEvent(nativeElement, 'click').pipe(
-        switchMap(() => this._action$),
-        takeUntil(this._unsubscribe$),
-      ).subscribe(action => action.trigger());
-    }
   }
 
   ngOnDestroy() {

diff --git a/projects/material/src/lib/action-mat-menu.component.ts b/projects/material/src/lib/action-mat-menu.component.ts
@@ -23,8 +23,18 @@ import { ACTION_ICON_TYPE_TOKEN, ICON_TYPE } from './action-icon-type-token';
 
         <ng-template #actionOutlet let-action>
           <ng-container *ngIf="action.visible$ | async" [ngSwitch]="_isAnchor(action)">
-            <action-mat-anchor *ngSwitchCase="true" [action]="action"></action-mat-anchor>
-            <action-mat-button *ngSwitchDefault [action]="action"></action-mat-button>
+            <ng-container *ngSwitchCase="true">
+              <a *ngIf="action.isExternalLink()" mat-menu-item [actionMatButton]="action" [href]="action.href$ | async" [attr.target]="action.target$ | async">
+                <ng-container *ngTemplateOutlet="content; context: { $implicit: action }"></ng-container>
+              </a>
+              <a *ngIf="!action.isExternalLink()" mat-menu-item [actionMatButton]="action" [routerLink]="action.href$ | async" [target]="action.target$ | async">
+                <ng-container *ngTemplateOutlet="content; context: { $implicit: action }"></ng-container>
+              </a>
+            </ng-container>
+
+            <button *ngSwitchDefault mat-menu-item [actionMatButton]="action">
+              <ng-container *ngTemplateOutlet="content; context: { $implicit: action }"></ng-container>
+            </button>
           </ng-container>
         </ng-template>
 

diff --git a/projects/playground/src/app/app.component.ts b/projects/playground/src/app/app.component.ts
@@ -11,7 +11,6 @@ export class AppComponent {
   button1 = new ActionButton({
     title: 'Button 1',
     callback: this.callback,
-    disabled: true
   });
   link1 = new ActionAnchor({
     title: 'Hello route (this tab)',
@@ -53,30 +52,35 @@ export class AppComponent {
     ariaLabel: 'Menu for more actions',
     children: [
       this.menuItem1,
-      new ActionAnchor({
-        title: 'Hello route (new tab)',
-        href: ['hello'],
-        target: '_blank',
-      }),
-      new ActionAnchor({
-        title: 'Home route (this tab)',
-        href: ['home'],
-      }),
-      new ActionAnchor({
-        title: 'Home route (new tab)',
-        href: ['home'],
-        target: '_blank',
-      }),
-      new ActionAnchor({
-        title: 'Google (this tab)',
-        href: 'http://www.google.com',
-      }),
-      new ActionAnchor({
-        title: 'Google (new tab)',
-        href: 'http://www.google.com',
-        target: '_blank',
+      new ActionGroup({
+        children: [
+          new ActionAnchor({
+            title: 'Hello route (new tab)',
+            href: ['hello'],
+            target: '_blank',
+          }),
+          new ActionAnchor({
+            title: 'Home route (this tab)',
+            href: ['home'],
+          }),
+          new ActionAnchor({
+            title: 'Home route (new tab)',
+            href: ['home'],
+            target: '_blank',
+          }),
+          new ActionAnchor({
+            title: 'Google (this tab)',
+            href: 'http://www.google.com',
+          }),
+          new ActionAnchor({
+            title: 'Google (new tab)',
+            href: 'http://www.google.com',
+            target: '_blank',
+          }),]
       }),
-      this.dropdown2,
+      new ActionGroup({
+        children: [this.dropdown2]
+      })
     ]
   });
   group1 = new ActionGroup({