Skip to content

Commit ba441d4

Browse files
andy9775andy9775
authored
refactor(cdk-experimental/menu): refactor MenuStack to ensure submenus are popped off and closed (#20006)
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 bbce2de commit ba441d4

File tree

6 files changed

+167
-21
lines changed

6 files changed

+167
-21
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,11 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit,
154154

155155
/** Subscribe to the MenuStack close and empty observables. */
156156
private _subscribeToMenuStack() {
157-
this._menuStack.close
157+
this._menuStack.closed
158158
.pipe(takeUntil(this._destroyed))
159159
.subscribe((item: MenuStackItem) => this._closeOpenMenu(item));
160160

161-
this._menuStack.empty
161+
this._menuStack.emptied
162162
.pipe(takeUntil(this._destroyed))
163163
.subscribe((event: FocusNext) => this._toggleOpenMenu(event));
164164
}

‎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().closeSubMenuOf(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().close(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().close(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
@@ -145,7 +145,7 @@ export class CdkMenuItem implements FocusableOption {
145145
if (this._isParentVertical() && !this.hasMenu()) {
146146
event.preventDefault();
147147
this._dir?.value === 'rtl'
148-
? this._getMenuStack().closeLatest(FocusNext.previousItem)
148+
? this._getMenuStack().close(this._parentMenu, FocusNext.previousItem)
149149
: this._getMenuStack().closeAll(FocusNext.nextItem);
150150
}
151151
break;
@@ -155,7 +155,7 @@ export class CdkMenuItem implements FocusableOption {
155155
event.preventDefault();
156156
this._dir?.value === 'rtl'
157157
? this._getMenuStack().closeAll(FocusNext.nextItem)
158-
: this._getMenuStack().closeLatest(FocusNext.previousItem);
158+
: this._getMenuStack().close(this._parentMenu, FocusNext.previousItem);
159159
}
160160
break;
161161
}

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
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+
const spy = jasmine.createSpy('menu stack closed spy');
54+
55+
menuStack.closed.subscribe(spy);
56+
menuStack.closeAll();
57+
58+
expect(spy).toHaveBeenCalledTimes(3);
59+
const callArgs = spy.calls.all().map((v: jasmine.CallInfo<jasmine.Func>) => v.args[0]);
60+
expect(callArgs).toEqual(menus.reverse());
61+
expect(menuStack.isEmpty()).toBeTrue();
62+
}
63+
);
64+
65+
it('should close triggering menu and all menus below it', () => {
66+
openAllMenus();
67+
expect(menus.length).toBe(3);
68+
69+
triggers[1].toggle();
70+
getElementsForTesting();
71+
72+
expect(menus.length).toBe(1);
73+
expect(menuStack.length()).withContext('menu stack should only have the single menu').toBe(1);
74+
expect(menuStack.peek()).toEqual(menus[0]);
75+
});
76+
});
77+
78+
@Component({
79+
template: `
80+
<div>
81+
<div cdkMenuBar id="menu_bar">
82+
<button cdkMenuItem [cdkMenuTriggerFor]="file">File</button>
83+
</div>
84+
85+
<ng-template cdkMenuPanel #file="cdkMenuPanel">
86+
<div cdkMenu id="file_menu" [cdkMenuPanel]="file">
87+
<button cdkMenuItem [cdkMenuTriggerFor]="share">Share</button>
88+
</div>
89+
</ng-template>
90+
91+
<ng-template cdkMenuPanel #share="cdkMenuPanel">
92+
<div cdkMenu id="share_menu" [cdkMenuPanel]="share">
93+
<button cdkMenuItem [cdkMenuTriggerFor]="chat">Chat</button>
94+
</div>
95+
</ng-template>
96+
97+
<ng-template cdkMenuPanel #chat="cdkMenuPanel">
98+
<div cdkMenu id="chat_menu" [cdkMenuPanel]="chat">
99+
<button cdkMenuItem>GVC</button>
100+
</div>
101+
</ng-template>
102+
</div>
103+
`,
104+
})
105+
class MultiMenuWithSubmenu {
106+
@ViewChild(CdkMenuBar) menuBar: CdkMenuBar;
107+
108+
@ViewChildren(CdkMenuItemTrigger) triggers: QueryList<CdkMenuItemTrigger>;
109+
@ViewChildren(CdkMenu) menus: QueryList<CdkMenu>;
110+
}

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

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,43 +40,63 @@ export class MenuStack {
4040
private readonly _empty: Subject<FocusNext> = new Subject();
4141

4242
/** Observable which emits the MenuStackItem which has been requested to close. */
43-
readonly close: Observable<MenuStackItem> = this._close;
43+
readonly closed: Observable<MenuStackItem> = this._close.asObservable();
4444

4545
/**
4646
* Observable which emits when the MenuStack is empty after popping off the last element. It
4747
* emits a FocusNext event which specifies the action the closer has requested the listener
4848
* perform.
4949
*/
50-
readonly empty: Observable<FocusNext> = this._empty;
50+
readonly emptied: Observable<FocusNext> = this._empty.asObservable();
5151

5252
/** @param menu the MenuStackItem to put on the stack. */
5353
push(menu: MenuStackItem) {
5454
this._elements.push(menu);
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+
close(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+
closeSubMenuOf(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;
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: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
151151
case ESCAPE:
152152
if (!hasModifierKey(event)) {
153153
event.preventDefault();
154-
this._menuStack.closeLatest(FocusNext.currentItem);
154+
this._menuStack.close(this, FocusNext.currentItem);
155155
}
156156
break;
157157

@@ -219,11 +219,11 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
219219

220220
/** Subscribe to the MenuStack close and empty observables. */
221221
private _subscribeToMenuStack() {
222-
this._menuStack.close
222+
this._menuStack.closed
223223
.pipe(takeUntil(this.closed))
224224
.subscribe((item: MenuStackItem) => this._closeOpenMenu(item));
225225

226-
this._menuStack.empty
226+
this._menuStack.emptied
227227
.pipe(takeUntil(this.closed))
228228
.subscribe((event: FocusNext) => this._toggleMenuFocus(event));
229229
}

0 commit comments

Comments
 (0)