Skip to content

Commit 752ffa9

Browse files
andy9775andy9775
committed
refactor(cdk-experimental/menu): refactor MenuStack to ensure submenus are popped off and closed
When closing a menu from the menu stack this ensures that any sub-menu's are popped off the stack and closed resulting in a stack which represents the state of the UI.
1 parent 49de56c commit 752ffa9

File tree

5 files changed

+159
-15
lines changed

5 files changed

+159
-15
lines changed

‎src/cdk-experimental/menu/menu-item-trigger.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ export class CdkMenuItemTrigger implements OnDestroy {
111111

112112
this._overlayRef!.detach();
113113
}
114+
this._getMenuStack().closeExclusive(this._parentMenu);
114115
}
115116

116117
/** Return true if the trigger has an attached menu */
@@ -150,7 +151,7 @@ export class CdkMenuItemTrigger implements OnDestroy {
150151
if (this._isParentVertical()) {
151152
event.preventDefault();
152153
if (this._directionality?.value === 'rtl') {
153-
this._getMenuStack().closeLatest(FocusNext.currentItem);
154+
this._getMenuStack().closeInclusive(this._parentMenu, FocusNext.currentItem);
154155
} else {
155156
this.openMenu();
156157
this.menuPanel?._menu?.focusFirstItem('keyboard');
@@ -165,7 +166,7 @@ export class CdkMenuItemTrigger implements OnDestroy {
165166
this.openMenu();
166167
this.menuPanel?._menu?.focusFirstItem('keyboard');
167168
} else {
168-
this._getMenuStack().closeLatest(FocusNext.currentItem);
169+
this._getMenuStack().closeInclusive(this._parentMenu, FocusNext.currentItem);
169170
}
170171
}
171172
break;

‎src/cdk-experimental/menu/menu-item.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ export class CdkMenuItem implements FocusableOption {
133133
if (this._isParentVertical() && !this.hasMenu()) {
134134
event.preventDefault();
135135
this._dir?.value === 'rtl'
136-
? this._getMenuStack().closeLatest(FocusNext.previousItem)
136+
? this._getMenuStack().closeInclusive(this._parentMenu, FocusNext.previousItem)
137137
: this._getMenuStack().closeAll(FocusNext.nextItem);
138138
}
139139
break;
@@ -143,7 +143,7 @@ export class CdkMenuItem implements FocusableOption {
143143
event.preventDefault();
144144
this._dir?.value === 'rtl'
145145
? this._getMenuStack().closeAll(FocusNext.nextItem)
146-
: this._getMenuStack().closeLatest(FocusNext.previousItem);
146+
: this._getMenuStack().closeInclusive(this._parentMenu, FocusNext.previousItem);
147147
}
148148
break;
149149
}

‎src/cdk-experimental/menu/menu-stack.spec.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import {QueryList, ViewChild, ViewChildren, Component} from '@angular/core';
2+
import {CdkMenu} from './menu';
3+
import {CdkMenuBar} from './menu-bar';
4+
import {ComponentFixture, TestBed, async} from '@angular/core/testing';
5+
import {CdkMenuItemTrigger} from './menu-item-trigger';
6+
import {MenuStack} from './menu-stack';
7+
import {CdkMenuModule} from './menu-module';
8+
9+
describe('MenuStack', () => {
10+
let fixture: ComponentFixture<MultiMenuWithSubmenu>;
11+
let menuStack: MenuStack;
12+
let triggers: CdkMenuItemTrigger[];
13+
let menus: CdkMenu[];
14+
15+
/** Fetch triggers, menus and the menu stack from the test component. */
16+
function getElementsForTesting() {
17+
fixture.detectChanges();
18+
triggers = fixture.componentInstance.triggers.toArray();
19+
menus = fixture.componentInstance.menus.toArray();
20+
menuStack = fixture.componentInstance.menuBar._menuStack;
21+
}
22+
23+
beforeEach(async(() => {
24+
TestBed.configureTestingModule({
25+
imports: [CdkMenuModule],
26+
declarations: [MultiMenuWithSubmenu],
27+
}).compileComponents();
28+
}));
29+
30+
beforeEach(() => {
31+
fixture = TestBed.createComponent(MultiMenuWithSubmenu);
32+
fixture.detectChanges();
33+
34+
getElementsForTesting();
35+
});
36+
37+
/** Open up all of the menus in the test component. */
38+
function openAllMenus() {
39+
triggers[0].openMenu();
40+
getElementsForTesting();
41+
triggers[1].openMenu();
42+
getElementsForTesting();
43+
triggers[2].openMenu();
44+
getElementsForTesting();
45+
}
46+
47+
it(
48+
'should fill the menu stack with the latest menu at the end of the stack and oldest at' +
49+
' the start of the stack',
50+
() => {
51+
openAllMenus();
52+
expect(menus.length).toBe(3);
53+
54+
menuStack.close.subscribe(item => {
55+
expect(item).toEqual(menus.pop()!);
56+
});
57+
menuStack.closeAll();
58+
expect(menus.length).toBe(0);
59+
expect(menuStack.isEmpty()).toBeTrue();
60+
}
61+
);
62+
63+
it('should close triggering menu and all menus below it', () => {
64+
openAllMenus();
65+
expect(menus.length).toBe(3);
66+
67+
triggers[1].toggle();
68+
getElementsForTesting();
69+
70+
expect(menus.length).toBe(1);
71+
expect(menuStack.length()).withContext('menu stack should only have the single menu').toBe(1);
72+
expect(menuStack.peek()).toEqual(menus[0]);
73+
});
74+
});
75+
76+
@Component({
77+
template: `
78+
<div>
79+
<div cdkMenuBar id="menu_bar">
80+
<button cdkMenuItem [cdkMenuTriggerFor]="file">File</button>
81+
</div>
82+
83+
<ng-template cdkMenuPanel #file="cdkMenuPanel">
84+
<div cdkMenu id="file_menu" [cdkMenuPanel]="file">
85+
<button cdkMenuItem [cdkMenuTriggerFor]="share">Share</button>
86+
</div>
87+
</ng-template>
88+
89+
<ng-template cdkMenuPanel #share="cdkMenuPanel">
90+
<div cdkMenu id="share_menu" [cdkMenuPanel]="share">
91+
<button cdkMenuItem [cdkMenuTriggerFor]="chat">Chat</button>
92+
</div>
93+
</ng-template>
94+
95+
<ng-template cdkMenuPanel #chat="cdkMenuPanel">
96+
<div cdkMenu id="chat_menu" [cdkMenuPanel]="chat">
97+
<button cdkMenuItem>GVC</button>
98+
</div>
99+
</ng-template>
100+
</div>
101+
`,
102+
})
103+
class MultiMenuWithSubmenu {
104+
@ViewChild(CdkMenuBar) menuBar: CdkMenuBar;
105+
106+
@ViewChildren(CdkMenuItemTrigger) triggers: QueryList<CdkMenuItemTrigger>;
107+
@ViewChildren(CdkMenu) menus: QueryList<CdkMenu>;
108+
}

‎src/cdk-experimental/menu/menu-stack.ts

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,28 +55,48 @@ export class MenuStack {
5555
}
5656

5757
/**
58-
* Pop off the top most MenuStackItem and emit it on the close observable.
59-
* @param focusNext the event to emit on the `empty` observable if the method call resulted in an
60-
* empty stack. Does not emit if the stack was initially empty.
58+
* Pop items off of the stack up to and including `lastItem` and emit each on the close
59+
* observable. If the stack is empty or `lastItem` is not on the stack it does nothing.
60+
* @param lastItem the last item to pop off the stack.
61+
* @param focusNext the event to emit on the `empty` observable if the method call resulted in an
62+
* empty stack. Does not emit if the stack was initially empty or if `lastItem` was not on the
63+
* stack.
6164
*/
62-
closeLatest(focusNext?: FocusNext) {
63-
const menuStackItem = this._elements.pop();
64-
if (menuStackItem) {
65-
this._close.next(menuStackItem);
66-
if (this._elements.length === 0) {
65+
closeInclusive(lastItem: MenuStackItem, focusNext?: FocusNext) {
66+
if (this._elements.indexOf(lastItem) >= 0) {
67+
let poppedElement;
68+
do {
69+
poppedElement = this._elements.pop();
70+
this._close.next(poppedElement);
71+
} while (poppedElement !== lastItem);
72+
73+
if (this.isEmpty()) {
6774
this._empty.next(focusNext);
6875
}
6976
}
7077
}
7178

79+
/**
80+
* Pop items off of the stack up to but excluding `lastItem` and emit each on the close
81+
* observable. If the stack is empty or `lastItem` is not on the stack it does nothing.
82+
* @param lastItem the element which should be left on the stack
83+
*/
84+
closeExclusive(lastItem: MenuStackItem) {
85+
if (this._elements.indexOf(lastItem) >= 0) {
86+
while (this.peek() !== lastItem) {
87+
this._close.next(this._elements.pop());
88+
}
89+
}
90+
}
91+
7292
/**
7393
* Pop off all MenuStackItems and emit each one on the `close` observable one by one.
7494
* @param focusNext the event to emit on the `empty` observable once the stack is emptied. Does
7595
* not emit if the stack was initially empty.
7696
*/
7797
closeAll(focusNext?: FocusNext) {
78-
if (this._elements.length) {
79-
while (this._elements.length) {
98+
if (!this.isEmpty()) {
99+
while (!this.isEmpty()) {
80100
const menuStackItem = this._elements.pop();
81101
if (menuStackItem) {
82102
this._close.next(menuStackItem);
@@ -86,4 +106,19 @@ export class MenuStack {
86106
this._empty.next(focusNext);
87107
}
88108
}
109+
110+
/** Return true if this stack is empty. */
111+
isEmpty() {
112+
return this._elements.length === 0;
113+
}
114+
115+
/** Return the length of the stack. */
116+
length() {
117+
return this._elements.length;
118+
}
119+
120+
/** Get the top most element on the stack. */
121+
peek() {
122+
return this._elements[this._elements.length - 1];
123+
}
89124
}

‎src/cdk-experimental/menu/menu.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
146146
case ESCAPE:
147147
if (!hasModifierKey(event)) {
148148
event.preventDefault();
149-
this._menuStack.closeLatest(FocusNext.currentItem);
149+
this._menuStack.closeInclusive(this, FocusNext.currentItem);
150150
}
151151
break;
152152

0 commit comments

Comments
 (0)