Skip to content

Commit 189fda0

Browse files
kseamonandy9775
authored andcommitted
perf(tooltip): Defer hooking up events until there's a message and the tooltip is not disabled (angular#19764)
* perf(tooltip): Defer hooking up events until there's a message and the tooltip is not disabled * comment update
1 parent 223caaa commit 189fda0

File tree

7 files changed

+235
-27
lines changed

7 files changed

+235
-27
lines changed

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

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ import {
1818
import {Directionality} from '@angular/cdk/bidi';
1919
import {FocusKeyManager, FocusOrigin} from '@angular/cdk/a11y';
2020
import {LEFT_ARROW, RIGHT_ARROW, UP_ARROW, DOWN_ARROW, ESCAPE, TAB} from '@angular/cdk/keycodes';
21+
import {Subject} from 'rxjs';
22+
import {takeUntil, mapTo, mergeAll, startWith, mergeMap} from 'rxjs/operators';
2123
import {CdkMenuGroup} from './menu-group';
2224
import {CDK_MENU, Menu} from './menu-interface';
2325
import {CdkMenuItem} from './menu-item';
2426
import {MenuStack, MenuStackItem, FocusNext} from './menu-stack';
25-
import {Subject} from 'rxjs';
26-
import {takeUntil} from 'rxjs/operators';
2727

2828
/**
2929
* Directive applied to an element which configures it as a MenuBar by setting the appropriate
@@ -64,6 +64,9 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit,
6464
@ContentChildren(CdkMenuItem, {descendants: true})
6565
private readonly _allItems: QueryList<CdkMenuItem>;
6666

67+
/** The Menu Item which opened the active submenu. */
68+
private _openItem?: CdkMenuItem;
69+
6770
constructor(readonly _menuStack: MenuStack, @Optional() private readonly _dir?: Directionality) {
6871
super();
6972
}
@@ -73,6 +76,8 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit,
7376

7477
this._setKeyManager();
7578
this._subscribeToMenuStack();
79+
this._subscribeToMenuOpen();
80+
this._allItems.changes.subscribe(() => this._subscribeToMenuOpen());
7681
}
7782

7883
/** Place focus on the first MenuItem in the menu and set the focus origin. */
@@ -163,12 +168,13 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit,
163168
* Close the open menu if the current active item opened the requested MenuStackItem.
164169
* @param item the MenuStackItem requested to be closed.
165170
*/
166-
private _closeOpenMenu(item: MenuStackItem) {
171+
private _closeOpenMenu(menu: MenuStackItem) {
172+
const trigger = this._openItem;
167173
const keyManager = this._keyManager;
168-
if (item === keyManager.activeItem?.getMenu()) {
169-
keyManager.activeItem.getMenuTrigger()?.closeMenu();
174+
if (menu === trigger?.getMenuTrigger()?.getMenu()) {
175+
trigger.getMenuTrigger()?.closeMenu();
170176
keyManager.setFocusOrigin('keyboard');
171-
keyManager.setActiveItem(keyManager.activeItem);
177+
keyManager.setActiveItem(trigger);
172178
}
173179
}
174180

@@ -200,6 +206,24 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit,
200206
}
201207
}
202208

209+
/**
210+
* Subscribe to the menu triggers open events in order to set the trigger which opened the menu.
211+
*/
212+
private _subscribeToMenuOpen() {
213+
this._allItems.changes
214+
.pipe(
215+
startWith(this._allItems),
216+
mergeMap((list: QueryList<CdkMenuItem>) =>
217+
list
218+
.filter(item => item.hasMenu())
219+
.map(item => item.getMenuTrigger()!.opened.pipe(mapTo(item)))
220+
),
221+
mergeAll(),
222+
takeUntil(this._destroyed)
223+
)
224+
.subscribe(item => (this._openItem = item));
225+
}
226+
203227
/**
204228
* @return true if the menu bar is configured to be horizontal.
205229
*/

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, FocusNext.currentItem);
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
}
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 first 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.peek()).toEqual(menus[0]);
72+
expect(menuStack.length()).withContext('menu stack should only have the single menu').toBe(1);
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: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,43 @@ export class MenuStack {
5555
}
5656

5757
/**
58-
* Pop off the top most MenuStackItem and emit it on the close observable.
58+
* Pop off the top most MenuStackItem and emit it on the close observable if the stack is not
59+
* empty. If `lastElement` is provided it will pop off until this element (inclusive). If
60+
* `lastElement` is not in the stack, it will pop off the topmost element only.
61+
* @param lastElement the last item to pop off the stack.
5962
* @param focusNext the event to emit on the `empty` observable if the method call resulted in an
6063
* empty stack. Does not emit if the stack was initially empty.
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(lastElement?: MenuStackItem, focusNext?: FocusNext) {
66+
if (!this.isEmpty()) {
67+
const containsElement = lastElement ? this._elements.indexOf(lastElement) >= 0 : false;
68+
let poppedElement;
69+
do {
70+
poppedElement = this._elements.pop();
71+
this._close.next(poppedElement);
72+
} while (containsElement && poppedElement !== lastElement && !this.isEmpty());
73+
74+
if (this.isEmpty()) {
75+
this._empty.next(focusNext);
76+
}
77+
}
78+
}
79+
80+
/**
81+
* Pop off the top most MenuStackItem and emit it on the close observable if the stack is not
82+
* empty. If `lastElement` is provided it will pop off until this element is the last one on the
83+
* stack. If `lastElement` is not in the stack, it will pop off the topmost element only.
84+
* @param lastElement the element which should be left on the stack
85+
* @param focusNext the event to emit on the `empty` observable if the method call resulted in an
86+
* empty stack. Does not emit if the stack was initially empty.
87+
*/
88+
closeExclusive(lastElement?: MenuStackItem, focusNext?: FocusNext) {
89+
if (!this.isEmpty() && this.peek() !== lastElement) {
90+
do {
91+
this._close.next(this._elements.pop());
92+
} while (lastElement && !this.isEmpty() && this.peek() !== lastElement);
93+
94+
if (this.isEmpty()) {
6795
this._empty.next(focusNext);
6896
}
6997
}
@@ -75,8 +103,8 @@ export class MenuStack {
75103
* not emit if the stack was initially empty.
76104
*/
77105
closeAll(focusNext?: FocusNext) {
78-
if (this._elements.length) {
79-
while (this._elements.length) {
106+
if (!this.isEmpty()) {
107+
while (!this.isEmpty()) {
80108
const menuStackItem = this._elements.pop();
81109
if (menuStackItem) {
82110
this._close.next(menuStackItem);
@@ -86,4 +114,19 @@ export class MenuStack {
86114
this._empty.next(focusNext);
87115
}
88116
}
117+
118+
/** Return true if this stack is empty. */
119+
isEmpty() {
120+
return this._elements.length === 0;
121+
}
122+
123+
/** Return the length of the stack. */
124+
length() {
125+
return this._elements.length;
126+
}
127+
128+
/** Get the top most element on the stack. */
129+
peek() {
130+
return this._elements[this._elements.length - 1];
131+
}
89132
}

src/cdk-experimental/menu/menu.ts

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ import {
2929
hasModifierKey,
3030
} from '@angular/cdk/keycodes';
3131
import {Directionality} from '@angular/cdk/bidi';
32-
import {take, takeUntil} from 'rxjs/operators';
32+
import {take, takeUntil, mergeAll, mapTo} from 'rxjs/operators';
33+
import {merge} from 'rxjs';
3334
import {CdkMenuGroup} from './menu-group';
3435
import {CdkMenuPanel} from './menu-panel';
3536
import {Menu, CDK_MENU} from './menu-interface';
@@ -81,6 +82,9 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
8182
@ContentChildren(CdkMenuItem, {descendants: true})
8283
private readonly _allItems: QueryList<CdkMenuItem>;
8384

85+
/** The Menu Item which opened the active submenu. */
86+
private _openTrigger?: CdkMenuItem;
87+
8488
/**
8589
* A reference to the enclosing parent menu panel.
8690
*
@@ -107,6 +111,8 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
107111
this._completeChangeEmitter();
108112
this._setKeyManager();
109113
this._subscribeToMenuStack();
114+
this._subscribeToMenuOpen();
115+
this._allItems.changes.subscribe(() => this._subscribeToMenuOpen());
110116
}
111117

112118
/** Place focus on the first MenuItem in the menu and set the focus origin. */
@@ -146,7 +152,7 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
146152
case ESCAPE:
147153
if (!hasModifierKey(event)) {
148154
event.preventDefault();
149-
this._menuStack.closeLatest(FocusNext.currentItem);
155+
this._menuStack.closeInclusive(this, FocusNext.currentItem);
150156
}
151157
break;
152158

@@ -227,12 +233,12 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
227233
* Close the open menu if the current active item opened the requested MenuStackItem.
228234
* @param item the MenuStackItem requested to be closed.
229235
*/
230-
private _closeOpenMenu(item: MenuStackItem) {
231-
const keyManager = this._keyManager;
232-
if (item === keyManager.activeItem?.getMenu()) {
233-
keyManager.activeItem.getMenuTrigger()?.closeMenu();
234-
keyManager.setFocusOrigin('keyboard');
235-
keyManager.setActiveItem(keyManager.activeItem);
236+
private _closeOpenMenu(menu: MenuStackItem) {
237+
const trigger = this._openTrigger;
238+
if (menu === trigger?.getMenuTrigger()?.getMenu()) {
239+
trigger.getMenuTrigger()?.closeMenu();
240+
this._keyManager.setFocusOrigin('keyboard');
241+
this._keyManager.setActiveItem(trigger);
236242
}
237243
}
238244

@@ -259,6 +265,19 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
259265
}
260266
}
261267

268+
/**
269+
* Subscribe to the menu triggers open events in order to set the trigger which opened the menu.
270+
*/
271+
private _subscribeToMenuOpen() {
272+
merge(
273+
this._allItems
274+
.filter(item => item.hasMenu())
275+
.map((item: CdkMenuItem) => item.getMenuTrigger()!.opened.pipe(mapTo(item)))
276+
)
277+
.pipe(mergeAll(), takeUntil(merge(this._allItems.changes, this.closed)))
278+
.subscribe(trigger => (this._openTrigger = trigger));
279+
}
280+
262281
/** Return true if this menu has been configured in a horizontal orientation. */
263282
private _isHorizontal() {
264283
return this.orientation === 'horizontal';

0 commit comments

Comments
 (0)