Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

Commit 0b65e08

Browse files
devversionkara
devversion
authored and
kara
committed
fix(menu): avoid runtime errors when menu-content isn't set. (#10198)
Fixes #9709.
1 parent 9dbdf7e commit 0b65e08

File tree

3 files changed

+148
-29
lines changed

3 files changed

+148
-29
lines changed

‎src/components/menu/js/menuDirective.js

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -177,26 +177,33 @@ function MenuDirective($mdUtil) {
177177

178178
function compile(templateElement) {
179179
templateElement.addClass('md-menu');
180-
var triggerElement = templateElement.children()[0];
180+
181+
var triggerEl = templateElement.children()[0];
182+
var contentEl = templateElement.children()[1];
183+
181184
var prefixer = $mdUtil.prefixer();
182185

183-
if (!prefixer.hasAttribute(triggerElement, 'ng-click')) {
184-
triggerElement = triggerElement
185-
.querySelector(prefixer.buildSelector(['ng-click', 'ng-mouseenter'])) || triggerElement;
186+
if (!prefixer.hasAttribute(triggerEl, 'ng-click')) {
187+
triggerEl = triggerEl
188+
.querySelector(prefixer.buildSelector(['ng-click', 'ng-mouseenter'])) || triggerEl;
189+
}
190+
191+
var isButtonTrigger = triggerEl.nodeName === 'MD-BUTTON' || triggerEl.nodeName === 'BUTTON';
192+
193+
if (triggerEl && isButtonTrigger && !triggerEl.hasAttribute('type')) {
194+
triggerEl.setAttribute('type', 'button');
186195
}
187-
if (triggerElement && (
188-
triggerElement.nodeName == 'MD-BUTTON' ||
189-
triggerElement.nodeName == 'BUTTON'
190-
) && !triggerElement.hasAttribute('type')) {
191-
triggerElement.setAttribute('type', 'button');
196+
197+
if (!triggerEl) {
198+
throw Error(INVALID_PREFIX + 'Expected the menu to have a trigger element.');
192199
}
193200

194-
if (templateElement.children().length != 2) {
195-
throw Error(INVALID_PREFIX + 'Expected two children elements.');
201+
if (!contentEl || contentEl.nodeName !== 'MD-MENU-CONTENT') {
202+
throw Error(INVALID_PREFIX + 'Expected the menu to contain a `md-menu-content` element.');
196203
}
197204

198205
// Default element for ARIA attributes has the ngClick or ngMouseenter expression
199-
triggerElement && triggerElement.setAttribute('aria-haspopup', 'true');
206+
triggerEl && triggerEl.setAttribute('aria-haspopup', 'true');
200207

201208
var nestedMenus = templateElement[0].querySelectorAll('md-menu');
202209
var nestingDepth = parseInt(templateElement[0].getAttribute('md-nest-level'), 10) || 0;

‎src/components/menu/js/menuServiceProvider.js

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ function MenuProvider($$interimElementProvider) {
2222
});
2323

2424
/* @ngInject */
25-
function menuDefaultOptions($mdUtil, $mdTheming, $mdConstant, $document, $window, $q, $$rAF, $animateCss, $animate) {
25+
function menuDefaultOptions($mdUtil, $mdTheming, $mdConstant, $document, $window, $q, $$rAF,
26+
$animateCss, $animate, $log) {
27+
2628
var prefixer = $mdUtil.prefixer();
2729
var animator = $mdUtil.dom.animator;
2830

@@ -74,10 +76,14 @@ function MenuProvider($$interimElementProvider) {
7476
* and backdrop
7577
*/
7678
function onRemove(scope, element, opts) {
77-
opts.cleanupInteraction && opts.cleanupInteraction();
79+
opts.cleanupInteraction();
80+
opts.cleanupBackdrop();
7881
opts.cleanupResizing();
7982
opts.hideBackdrop();
8083

84+
// Before the menu is closing remove the clickable class.
85+
element.removeClass('md-clickable');
86+
8187
// For navigation $destroy events, do a quick, non-animated removal,
8288
// but for normal closes (from clicks, etc) animate the removal
8389

@@ -109,9 +115,17 @@ function MenuProvider($$interimElementProvider) {
109115
function onShow(scope, element, opts) {
110116
sanitizeAndConfigure(opts);
111117

112-
// Wire up theming on our menu element
113-
$mdTheming.inherit(opts.menuContentEl, opts.target);
114-
118+
if (opts.menuContentEl[0]) {
119+
// Inherit the theme from the target element.
120+
$mdTheming(opts.menuContentEl, opts.target);
121+
} else {
122+
$log.warn(
123+
'$mdMenu: Menu elements should always contain a `md-menu-content` element,' +
124+
'otherwise interactivity features will not work properly.',
125+
element
126+
);
127+
}
128+
115129
// Register various listeners to move menu on resize/orientation change
116130
opts.cleanupResizing = startRepositioningOnResize();
117131
opts.hideBackdrop = showBackdrop(scope, element, opts);
@@ -121,6 +135,11 @@ function MenuProvider($$interimElementProvider) {
121135
.then(function(response) {
122136
opts.alreadyOpen = true;
123137
opts.cleanupInteraction = activateInteraction();
138+
opts.cleanupBackdrop = setupBackdrop();
139+
140+
// Since the menu finished its animation, mark the menu as clickable.
141+
element.addClass('md-clickable');
142+
124143
return response;
125144
});
126145

@@ -194,14 +213,40 @@ function MenuProvider($$interimElementProvider) {
194213
}
195214

196215
/**
197-
* Activate interaction on the menu. Wire up keyboard listerns for
198-
* clicks, keypresses, backdrop closing, etc.
216+
* Sets up the backdrop and listens for click elements.
217+
* Once the backdrop will be clicked, the menu will automatically close.
218+
* @returns {!Function} Function to remove the backdrop.
199219
*/
200-
function activateInteraction() {
201-
element.addClass('md-clickable');
220+
function setupBackdrop() {
221+
if (!opts.backdrop) return angular.noop;
222+
223+
opts.backdrop.on('click', onBackdropClick);
202224

203-
// close on backdrop click
204-
if (opts.backdrop) opts.backdrop.on('click', onBackdropClick);
225+
return function() {
226+
opts.backdrop.off('click', onBackdropClick);
227+
}
228+
}
229+
230+
/**
231+
* Function to be called whenever the backdrop is clicked.
232+
* @param {!MouseEvent} event
233+
*/
234+
function onBackdropClick(event) {
235+
event.preventDefault();
236+
event.stopPropagation();
237+
238+
scope.$apply(function() {
239+
opts.mdMenuCtrl.close(true, { closeAll: true });
240+
});
241+
}
242+
243+
/**
244+
* Activate interaction on the menu. Resolves the focus target and closes the menu on
245+
* escape or option click.
246+
* @returns {!Function} Function to deactivate the interaction listeners.
247+
*/
248+
function activateInteraction() {
249+
if (!opts.menuContentEl[0]) return angular.noop;
205250

206251
// Wire up keyboard listeners.
207252
// - Close on escape,
@@ -232,8 +277,6 @@ function MenuProvider($$interimElementProvider) {
232277
focusTarget && focusTarget.focus();
233278

234279
return function cleanupInteraction() {
235-
element.removeClass('md-clickable');
236-
if (opts.backdrop) opts.backdrop.off('click', onBackdropClick);
237280
opts.menuContentEl.off('keydown', onMenuKeyDown);
238281
opts.menuContentEl[0].removeEventListener('click', captureClickListener, true);
239282
};

‎src/components/menu/menu.spec.js

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,28 @@ describe('material.components.menu', function() {
2727
expect(element.hasClass('_md')).toBe(true);
2828
});
2929

30-
it('errors on invalid markup', inject(function($compile, $rootScope) {
31-
function buildBadMenu() {
32-
$compile('<md-menu></md-menu>')($rootScope);
30+
it('should throw when trigger element is missing', inject(function($compile, $rootScope) {
31+
function createInvalidMenu() {
32+
$compile(
33+
'<md-menu>' +
34+
' <md-menu-content>Menu Content</md-menu-content>' +
35+
'</md-menu>'
36+
)($rootScope);
3337
}
3438

35-
expect(buildBadMenu).toThrow();
39+
expect(createInvalidMenu).toThrow();
40+
}));
41+
42+
it('should throw when md-menu-content is missing', inject(function($compile, $rootScope) {
43+
function createInvalidMenu() {
44+
$compile(
45+
'<md-menu>' +
46+
' <button ng-click="null">Trigger Element</button>' +
47+
'</md-menu>'
48+
)($rootScope);
49+
}
50+
51+
expect(createInvalidMenu).toThrow();
3652
}));
3753

3854
it('specifies button type', inject(function($compile, $rootScope) {
@@ -341,6 +357,59 @@ describe('material.components.menu', function() {
341357
}
342358
});
343359

360+
describe('with $mdMenu service', function() {
361+
362+
var $mdMenu, $rootScope, $compile, $timeout, $log = null;
363+
364+
beforeEach(inject(function($injector) {
365+
$mdMenu = $injector.get('$mdMenu');
366+
$rootScope = $injector.get('$rootScope');
367+
$compile = $injector.get('$compile');
368+
$timeout = $injector.get('$timeout');
369+
$log = $injector.get('$log');
370+
}));
371+
372+
it('should warn when the md-menu-content element is missing', function() {
373+
spyOn($log, 'warn');
374+
375+
var parent = angular.element('<div>');
376+
var menuEl = angular.element(
377+
'<md-menu>' +
378+
' <button ng-click="null">Trigger</button>' +
379+
'</md-menu>'
380+
);
381+
382+
expect($log.warn).not.toHaveBeenCalled();
383+
384+
$mdMenu.show({
385+
scope: $rootScope,
386+
mdMenuCtrl: createFakeMenuController(),
387+
element: menuEl,
388+
target: document.body,
389+
preserveElement: true,
390+
parent: parent
391+
});
392+
393+
$timeout.flush();
394+
395+
expect($log.warn).toHaveBeenCalledTimes(1);
396+
397+
// Close the menu and remove the parent container
398+
$mdMenu.hide();
399+
parent.remove();
400+
});
401+
402+
function createFakeMenuController() {
403+
return {
404+
open: function() {},
405+
close: function() { $mdMenu.hide(); },
406+
positionMode: function() { return { left: 'left', top: 'target' }; },
407+
offsets: function() { return { top: 0, left: 0 }; }
408+
}
409+
}
410+
411+
});
412+
344413

345414
// ********************************************
346415
// Internal methods

0 commit comments

Comments
 (0)