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

fix(menu): avoid runtime errors when menu-content isn't set. #10198

Merged
merged 2 commits into from
Jan 3, 2017
Merged

fix(menu): avoid runtime errors when menu-content isn't set. #10198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aac6363
fix(menu): avoid runtime errors when menu-content isn't set.
devversion Jan 1, 2017
3921908
Remove fit
devversion Jan 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 19 additions & 12 deletions src/components/menu/js/menuDirective.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,26 +177,33 @@ function MenuDirective($mdUtil) {

function compile(templateElement) {
templateElement.addClass('md-menu');
var triggerElement = templateElement.children()[0];

var triggerEl = templateElement.children()[0];
var contentEl = templateElement.children()[1];

var prefixer = $mdUtil.prefixer();

if (!prefixer.hasAttribute(triggerElement, 'ng-click')) {
triggerElement = triggerElement
.querySelector(prefixer.buildSelector(['ng-click', 'ng-mouseenter'])) || triggerElement;
if (!prefixer.hasAttribute(triggerEl, 'ng-click')) {
triggerEl = triggerEl
.querySelector(prefixer.buildSelector(['ng-click', 'ng-mouseenter'])) || triggerEl;
}

var isButtonTrigger = triggerEl.nodeName === 'MD-BUTTON' || triggerEl.nodeName === 'BUTTON';

if (triggerEl && isButtonTrigger && !triggerEl.hasAttribute('type')) {
triggerEl.setAttribute('type', 'button');
}
if (triggerElement && (
triggerElement.nodeName == 'MD-BUTTON' ||
triggerElement.nodeName == 'BUTTON'
) && !triggerElement.hasAttribute('type')) {
triggerElement.setAttribute('type', 'button');

if (!triggerEl) {
throw Error(INVALID_PREFIX + 'Expected the menu to have a trigger element.');
}

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

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

var nestedMenus = templateElement[0].querySelectorAll('md-menu');
var nestingDepth = parseInt(templateElement[0].getAttribute('md-nest-level'), 10) || 0;
Expand Down
69 changes: 56 additions & 13 deletions src/components/menu/js/menuServiceProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ function MenuProvider($$interimElementProvider) {
});

/* @ngInject */
function menuDefaultOptions($mdUtil, $mdTheming, $mdConstant, $document, $window, $q, $$rAF, $animateCss, $animate) {
function menuDefaultOptions($mdUtil, $mdTheming, $mdConstant, $document, $window, $q, $$rAF,
$animateCss, $animate, $log) {

var prefixer = $mdUtil.prefixer();
var animator = $mdUtil.dom.animator;

Expand Down Expand Up @@ -74,10 +76,14 @@ function MenuProvider($$interimElementProvider) {
* and backdrop
*/
function onRemove(scope, element, opts) {
opts.cleanupInteraction && opts.cleanupInteraction();
opts.cleanupInteraction();
opts.cleanupBackdrop();
opts.cleanupResizing();
opts.hideBackdrop();

// Before the menu is closing remove the clickable class.
element.removeClass('md-clickable');

// For navigation $destroy events, do a quick, non-animated removal,
// but for normal closes (from clicks, etc) animate the removal

Expand Down Expand Up @@ -109,9 +115,17 @@ function MenuProvider($$interimElementProvider) {
function onShow(scope, element, opts) {
sanitizeAndConfigure(opts);

// Wire up theming on our menu element
$mdTheming.inherit(opts.menuContentEl, opts.target);

if (opts.menuContentEl[0]) {
// Inherit the theme from the target element.
$mdTheming(opts.menuContentEl, opts.target);
} else {
$log.warn(
'$mdMenu: Menu elements should always contain a `md-menu-content` element,' +
'otherwise interactivity features will not work properly.',
element
);
}

// Register various listeners to move menu on resize/orientation change
opts.cleanupResizing = startRepositioningOnResize();
opts.hideBackdrop = showBackdrop(scope, element, opts);
Expand All @@ -121,6 +135,11 @@ function MenuProvider($$interimElementProvider) {
.then(function(response) {
opts.alreadyOpen = true;
opts.cleanupInteraction = activateInteraction();
opts.cleanupBackdrop = setupBackdrop();

// Since the menu finished its animation, mark the menu as clickable.
element.addClass('md-clickable');

return response;
});

Expand Down Expand Up @@ -194,14 +213,40 @@ function MenuProvider($$interimElementProvider) {
}

/**
* Activate interaction on the menu. Wire up keyboard listerns for
* clicks, keypresses, backdrop closing, etc.
* Sets up the backdrop and listens for click elements.
* Once the backdrop will be clicked, the menu will automatically close.
* @returns {!Function} Function to remove the backdrop.
*/
function activateInteraction() {
element.addClass('md-clickable');
function setupBackdrop() {
if (!opts.backdrop) return angular.noop;

opts.backdrop.on('click', onBackdropClick);

// close on backdrop click
if (opts.backdrop) opts.backdrop.on('click', onBackdropClick);
return function() {
opts.backdrop.off('click', onBackdropClick);
}
}

/**
* Function to be called whenever the backdrop is clicked.
* @param {!MouseEvent} event
*/
function onBackdropClick(event) {
event.preventDefault();
event.stopPropagation();

scope.$apply(function() {
opts.mdMenuCtrl.close(true, { closeAll: true });
});
}

/**
* Activate interaction on the menu. Resolves the focus target and closes the menu on
* escape or option click.
* @returns {!Function} Function to deactivate the interaction listeners.
*/
function activateInteraction() {
if (!opts.menuContentEl[0]) return angular.noop;

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

return function cleanupInteraction() {
element.removeClass('md-clickable');
if (opts.backdrop) opts.backdrop.off('click', onBackdropClick);
opts.menuContentEl.off('keydown', onMenuKeyDown);
opts.menuContentEl[0].removeEventListener('click', captureClickListener, true);
};
Expand Down
77 changes: 73 additions & 4 deletions src/components/menu/menu.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,28 @@ describe('material.components.menu', function() {
expect(element.hasClass('_md')).toBe(true);
});

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

expect(buildBadMenu).toThrow();
expect(createInvalidMenu).toThrow();
}));

it('should throw when md-menu-content is missing', inject(function($compile, $rootScope) {
function createInvalidMenu() {
$compile(
'<md-menu>' +
' <button ng-click="null">Trigger Element</button>' +
'</md-menu>'
)($rootScope);
}

expect(createInvalidMenu).toThrow();
}));

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

describe('with $mdMenu service', function() {

var $mdMenu, $rootScope, $compile, $timeout, $log = null;

beforeEach(inject(function($injector) {
$mdMenu = $injector.get('$mdMenu');
$rootScope = $injector.get('$rootScope');
$compile = $injector.get('$compile');
$timeout = $injector.get('$timeout');
$log = $injector.get('$log');
}));

it('should warn when the md-menu-content element is missing', function() {
spyOn($log, 'warn');

var parent = angular.element('<div>');
var menuEl = angular.element(
'<md-menu>' +
' <button ng-click="null">Trigger</button>' +
'</md-menu>'
);

expect($log.warn).not.toHaveBeenCalled();

$mdMenu.show({
scope: $rootScope,
mdMenuCtrl: createFakeMenuController(),
element: menuEl,
target: document.body,
preserveElement: true,
parent: parent
});

$timeout.flush();

expect($log.warn).toHaveBeenCalledTimes(1);

// Close the menu and remove the parent container
$mdMenu.hide();
parent.remove();
});

function createFakeMenuController() {
return {
open: function() {},
close: function() { $mdMenu.hide(); },
positionMode: function() { return { left: 'left', top: 'target' }; },
offsets: function() { return { top: 0, left: 0 }; }
}
}

});


// ********************************************
// Internal methods
Expand Down