Skip to content

Commit baeda3e

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 baeda3e

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
@@ -150,11 +150,11 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit,
150150

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

157-
this._menuStack.empty
157+
this._menuStack.emptied
158158
.pipe(takeUntil(this._destroyed))
159159
.subscribe((event: FocusNext) => this._toggleOpenMenu(event));
160160
}

‎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
@@ -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().close(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().close(this._parentMenu, FocusNext.previousItem);
147147
}
148148
break;
149149
}

‎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.asObservable();
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.asObservable();
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
@@ -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.close(this, FocusNext.currentItem);
150150
}
151151
break;
152152

@@ -214,11 +214,11 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
214214

215215
/** Subscribe to the MenuStack close and empty observables. */
216216
private _subscribeToMenuStack() {
217-
this._menuStack.close
217+
this._menuStack.closed
218218
.pipe(takeUntil(this.closed))
219219
.subscribe((item: MenuStackItem) => this._closeOpenMenu(item));
220220

221-
this._menuStack.empty
221+
this._menuStack.emptied
222222
.pipe(takeUntil(this.closed))
223223
.subscribe((event: FocusNext) => this._toggleMenuFocus(event));
224224
}

0 commit comments

Comments
 (0)