Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(cdk/menu): unable to close child menu with disabled items when us… #28097

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(cdk/menu): unable to close child menu with disabled items when us… #28097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/cdk/menu/menu-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,13 @@ export abstract class CdkMenuBase
focusFirstItem(focusOrigin: FocusOrigin = 'program') {
this.keyManager.setFocusOrigin(focusOrigin);
this.keyManager.setFirstItemActive();

// If there's no active item at this point, it means that all the items are disabled.
// Move focus to the menuPanel panel so keyboard events like Escape and closing child menus using arrow keys still works.
// Also, this will give _some_ feedback to screen readers.
if (!this.nativeElement.contains(document.activeElement)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of going to the DOM, I think that we can check keyManager.activeItem === null here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delayed response, I tried it but didn't work, the keyManager.activeItem at this point is the child menu trigger button.

this.nativeElement.focus();
}
}

/**
Expand Down
67 changes: 66 additions & 1 deletion src/cdk/menu/menu.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
waitForAsync,
} from '@angular/core/testing';
import {Component, ElementRef, QueryList, ViewChild, ViewChildren} from '@angular/core';
import {TAB} from '@angular/cdk/keycodes';
import {LEFT_ARROW, RIGHT_ARROW, TAB} from '@angular/cdk/keycodes';
import {
createMouseEvent,
dispatchEvent,
Expand Down Expand Up @@ -135,6 +135,8 @@ describe('Menu', () => {

let nativeShareTrigger: HTMLElement | undefined;

let nativeDownloadTrigger: HTMLElement | undefined;

let nativeMenus: HTMLElement[];

beforeEach(waitForAsync(() => {
Expand Down Expand Up @@ -163,6 +165,8 @@ describe('Menu', () => {

nativeShareTrigger = fixture.componentInstance.nativeShareTrigger?.nativeElement;

nativeDownloadTrigger = fixture.componentInstance.nativeDownloadTrigger?.nativeElement;

nativeMenus = fixture.componentInstance.menus.map(m => m.nativeElement);
}

Expand Down Expand Up @@ -315,6 +319,20 @@ describe('Menu', () => {
expect(numEnters).toBeGreaterThan(2);
expect(nativeMenus.length).toBe(1);
}));

it('should close the download menu when it is open and right arrow key is pressed', () => {
openFileMenu();
if (nativeDownloadTrigger) {
dispatchKeyboardEvent(nativeDownloadTrigger, 'keydown', RIGHT_ARROW);
detectChanges();

const downloadMenu = nativeMenus[1];
dispatchKeyboardEvent(downloadMenu, 'keydown', LEFT_ARROW);
detectChanges();

expect(nativeMenus.length).toBe(1);
}
});
});

describe('with rtl layout and menu at bottom of page moving up and left', () => {
Expand All @@ -327,6 +345,8 @@ describe('Menu', () => {

let nativeShareTrigger: HTMLElement | undefined;

let nativeDownloadTrigger: HTMLElement | undefined;

let nativeMenus: HTMLElement[];

beforeEach(waitForAsync(() => {
Expand Down Expand Up @@ -363,6 +383,8 @@ describe('Menu', () => {

nativeShareTrigger = fixture.componentInstance.nativeShareTrigger?.nativeElement;

nativeDownloadTrigger = fixture.componentInstance.nativeDownloadTrigger?.nativeElement;

nativeMenus = fixture.componentInstance.menus.map(m => m.nativeElement);
}

Expand Down Expand Up @@ -504,6 +526,20 @@ describe('Menu', () => {
expect(nativeMenus.length).toBe(2);
expect(nativeMenus[1].id).toBe('edit_menu');
}));

it('should close the download menu when it is open and right arrow key is pressed in rtl', () => {
openFileMenu();
if (nativeDownloadTrigger) {
dispatchKeyboardEvent(nativeDownloadTrigger, 'keydown', LEFT_ARROW);
detectChanges();

const downloadMenu = nativeMenus[1];
dispatchKeyboardEvent(downloadMenu, 'keydown', RIGHT_ARROW);
detectChanges();

expect(nativeMenus.length).toBe(1);
}
});
});
});
});
Expand Down Expand Up @@ -554,6 +590,7 @@ class InlineMenu {}
>
<button #edit_trigger cdkMenuItem [cdkMenuTriggerFor]="edit">Edit</button>
<button #share_trigger cdkMenuItem [cdkMenuTriggerFor]="share">Share</button>
<button #download_trigger cdkMenuItem [cdkMenuTriggerFor]="download">Download</button>
<button cdkMenuItem>Open</button>
<button cdkMenuItem>Rename</button>
<button cdkMenuItem>Print</button>
Expand Down Expand Up @@ -587,12 +624,26 @@ class InlineMenu {}
<button cdkMenuItem>Twitter</button>
</div>
</ng-template>

<ng-template #download>
<div
id="download_menu"
style="display: flex; flex-direction: column;"
cdkMenu
cdkTargetMenuAim
>
<button cdkMenuItem disabled>Via Server 1</button>
<button cdkMenuItem disabled>Via Server 2</button>
</div>
</ng-template>
`,
})
class WithComplexNestedMenus {
@ViewChild('file_trigger', {read: ElementRef}) nativeFileTrigger: ElementRef<HTMLElement>;
@ViewChild('edit_trigger', {read: ElementRef}) nativeEditTrigger?: ElementRef<HTMLElement>;
@ViewChild('share_trigger', {read: ElementRef}) nativeShareTrigger?: ElementRef<HTMLElement>;
@ViewChild('download_trigger', {read: ElementRef})
nativeDownloadTrigger?: ElementRef<HTMLElement>;

@ViewChildren(CdkMenu) menus: QueryList<CdkMenu>;
}
Expand All @@ -615,6 +666,7 @@ class WithComplexNestedMenus {
<button cdkMenuItem>Open</button>
<button #share_trigger cdkMenuItem [cdkMenuTriggerFor]="share">Share</button>
<button #edit_trigger cdkMenuItem [cdkMenuTriggerFor]="edit">Edit</button>
<button #download_trigger cdkMenuItem [cdkMenuTriggerFor]="download">Download</button>
</div>
</ng-template>

Expand Down Expand Up @@ -646,12 +698,25 @@ class WithComplexNestedMenus {
<button cdkMenuItem>Facebook</button>
</div>
</ng-template>
<ng-template #download>
<div
id="download_menu"
style="display: flex; flex-direction: column;"
cdkMenu
cdkTargetMenuAim
>
<button cdkMenuItem disabled>Via Server 1</button>
<button cdkMenuItem disabled>Via Server 2</button>
</div>
</ng-template>
`,
})
class WithComplexNestedMenusOnBottom {
@ViewChild('file_trigger', {read: ElementRef}) nativeFileTrigger: ElementRef<HTMLElement>;
@ViewChild('edit_trigger', {read: ElementRef}) nativeEditTrigger?: ElementRef<HTMLElement>;
@ViewChild('share_trigger', {read: ElementRef}) nativeShareTrigger?: ElementRef<HTMLElement>;
@ViewChild('download_trigger', {read: ElementRef})
nativeDownloadTrigger?: ElementRef<HTMLElement>;

@ViewChildren(CdkMenu) menus: QueryList<CdkMenu>;
}
33 changes: 33 additions & 0 deletions src/cdk/menu/menu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,42 @@ export class CdkMenu extends CdkMenuBase implements AfterContentInit, OnDestroy
const keyManager = this.keyManager;
switch (event.keyCode) {
case LEFT_ARROW:
if (!hasModifierKey(event)) {
event.preventDefault();

if (
this._parentTrigger &&
this.nativeElement === document.activeElement &&
this.dir?.value !== 'rtl'
) {
this.menuStack.close(this, {
focusNextOnEmpty: FocusNext.currentItem,
focusParentTrigger: true,
});
return;
}

keyManager.setFocusOrigin('keyboard');
keyManager.onKeydown(event);
}
break;

case RIGHT_ARROW:
if (!hasModifierKey(event)) {
event.preventDefault();

if (
this._parentTrigger &&
this.nativeElement === document.activeElement &&
this.dir?.value === 'rtl'
) {
this.menuStack.close(this, {
focusNextOnEmpty: FocusNext.currentItem,
focusParentTrigger: true,
});
return;
}

keyManager.setFocusOrigin('keyboard');
keyManager.onKeydown(event);
}
Expand Down
Loading