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

Commit bfad5e4

Browse files
crisbetoThomasBurleson
crisbeto
authored and
ThomasBurleson
committed
fix(menu-item): properly compile when used with ng-repeat
* Fixes the menuItem directive not compiling properly when used together with ngRepeat. This is due to the fact that ngRepeat passes around a documentFragment, instead of a DOM node, which means that it doesn't have a parent and doesn't allow us to properly determine whether the menuItem is inside of a menuBar. This change switches the behavior by making the menuBar directive mark it's children instead. * Minor cleanup in the menuItem directive. * Avoided some repetition in the menuBar unit tests. * Switches to using the prefixer in the menuItem directive. Referencing #8117. Fixes #8697. Closes #8852 Closes #8850
1 parent f85ac4b commit bfad5e4

File tree

3 files changed

+64
-55
lines changed

3 files changed

+64
-55
lines changed

‎src/components/menuBar/js/menuBarDirective.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,13 @@ function MenuBarDirective($mdUtil, $mdTheming) {
123123
}
124124
});
125125

126+
// Mark the child menu items that they're inside a menu bar. This is necessary,
127+
// because mnMenuItem has special behaviour during compilation, depending on
128+
// whether it is inside a mdMenuBar. We can usually figure this out via the DOM,
129+
// however if a directive that uses documentFragment is applied to the child (e.g. ngRepeat),
130+
// the element won't have a parent and won't compile properly.
131+
templateEl.find('md-menu-item').addClass('md-in-menu-bar');
132+
126133
return function postLink(scope, el, attr, ctrl) {
127134
el.addClass('_md'); // private md component indicator for styling
128135
$mdTheming(scope, el);

‎src/components/menuBar/js/menuItemDirective.js

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@ angular
66
/* @ngInject */
77
function MenuItemDirective($mdUtil) {
88
return {
9+
controller: 'MenuItemController',
910
require: ['mdMenuItem', '?ngModel'],
1011
priority: 210, // ensure that our post link runs after ngAria
1112
compile: function(templateEl, templateAttrs) {
13+
var type = templateAttrs.type;
14+
var inMenuBarClass = 'md-in-menu-bar';
1215

1316
// Note: This allows us to show the `check` icon for the md-menu-bar items.
14-
if (isInsideMenuBar() && (templateAttrs.type == 'checkbox' || templateAttrs.type == 'radio')) {
17+
// The `md-in-menu-bar` class is set by the mdMenuBar directive.
18+
if ((type == 'checkbox' || type == 'radio') && templateEl.hasClass(inMenuBarClass)) {
1519
var text = templateEl[0].textContent;
1620
var buttonEl = angular.element('<md-button type="button"></md-button>');
1721
buttonEl.html(text);
@@ -20,10 +24,10 @@ function MenuItemDirective($mdUtil) {
2024
templateEl.html('');
2125
templateEl.append(angular.element('<md-icon md-svg-icon="check"></md-icon>'));
2226
templateEl.append(buttonEl);
23-
templateEl[0].classList.add('md-indent');
27+
templateEl.addClass('md-indent').removeClass(inMenuBarClass);
2428

25-
setDefault('role', (templateAttrs.type == 'checkbox') ? 'menuitemcheckbox' : 'menuitemradio', buttonEl);
26-
angular.forEach(['ng-disabled'], moveAttrToButton);
29+
setDefault('role', type == 'checkbox' ? 'menuitemcheckbox' : 'menuitemradio', buttonEl);
30+
moveAttrToButton('ng-disabled');
2731

2832
} else {
2933
setDefault('role', 'menuitem', templateEl[0].querySelector('md-button, button, a'));
@@ -46,18 +50,17 @@ function MenuItemDirective($mdUtil) {
4650
}
4751
}
4852

49-
function moveAttrToButton(attr) {
50-
if (templateEl[0].hasAttribute(attr)) {
51-
var val = templateEl[0].getAttribute(attr);
52-
buttonEl[0].setAttribute(attr, val);
53-
templateEl[0].removeAttribute(attr);
54-
}
55-
}
53+
function moveAttrToButton(attribute) {
54+
var attributes = $mdUtil.prefixer(attribute);
5655

57-
function isInsideMenuBar() {
58-
return !!$mdUtil.getClosest(templateEl, 'md-menu-bar', true);
56+
angular.forEach(attributes, function(attr) {
57+
if (templateEl[0].hasAttribute(attr)) {
58+
var val = templateEl[0].getAttribute(attr);
59+
buttonEl[0].setAttribute(attr, val);
60+
templateEl[0].removeAttribute(attr);
61+
}
62+
});
5963
}
60-
},
61-
controller: 'MenuItemController'
64+
}
6265
};
6366
}

‎src/components/menuBar/menu-bar.spec.js

Lines changed: 39 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ describe('material.components.menuBar', function() {
2626
});
2727

2828
describe('ARIA', function() {
29-
29+
3030
it('sets role="menubar" on the menubar', function() {
3131
var menuBar = setup();
3232
var ariaRole = menuBar[0].getAttribute('role');
3333
expect(ariaRole).toBe('menubar');
3434
});
35-
35+
3636
it('should set the role on the menu trigger correctly', inject(function($compile, $rootScope) {
3737
var el = $compile(
3838
'<md-menu-bar>' +
@@ -235,7 +235,7 @@ describe('material.components.menuBar', function() {
235235
it('clicks the focused menu', function() {
236236
var opened = false;
237237
spyOn(ctrl, 'getFocusedMenu').and.returnValue({
238-
querySelector: function() { return true }
238+
querySelector: function() { return true; }
239239
});
240240
spyOn(angular, 'element').and.returnValue({
241241
controller: function() { return {
@@ -317,13 +317,24 @@ describe('material.components.menuBar', function() {
317317

318318
describe('md-menu-item directive', function() {
319319
describe('type="checkbox"', function() {
320+
function setup(attrs) {
321+
return setupMenuItem(attrs + ' type="checkbox"');
322+
}
323+
320324
it('compiles', function() {
321325
var menuItem = setup('ng-model="test"')[0];
322326
expect(menuItem.classList.contains('md-indent')).toBe(true);
323327
var children = menuItem.children;
324328
expect(children[0].nodeName).toBe('MD-ICON');
325329
expect(children[1].nodeName).toBe('MD-BUTTON');
326330
});
331+
it('compiles with ng-repeat', function() {
332+
var menuItem = setup('ng-repeat="i in [1, 2, 3]"')[0];
333+
expect(menuItem.classList.contains('md-indent')).toBe(true);
334+
var children = menuItem.children;
335+
expect(children[0].nodeName).toBe('MD-ICON');
336+
expect(children[1].nodeName).toBe('MD-BUTTON');
337+
});
327338
it('sets aria role', function() {
328339
var menuItem = setup()[0].querySelector('md-button');
329340
expect(menuItem.getAttribute('role')).toBe('menuitemcheckbox');
@@ -354,36 +365,27 @@ describe('material.components.menuBar', function() {
354365
expect(menuItem.children[0].style.display).toBe('');
355366
expect(button.getAttribute('aria-checked')).toBe('true');
356367
}));
368+
});
357369

370+
describe('type="radio"', function() {
358371
function setup(attrs) {
359-
attrs = attrs || '';
360-
361-
var template = '<md-menu-item type="checkbox" ' + attrs + '>Test Item</md-menu-item>';
362-
363-
var checkboxMenuItem;
364-
inject(function($compile, $rootScope) {
365-
// We need to have a `md-menu-bar` as a parent of our menu item, because the menu-item
366-
// is only wrapping and indenting the content if it's inside of a menu bar.
367-
var menuBarMock = angular.element('<md-menu-bar>');
368-
var itemEl = angular.element(template);
369-
370-
menuBarMock.append(itemEl);
371-
checkboxMenuItem = $compile(itemEl)($rootScope);
372-
373-
$rootScope.$digest();
374-
});
375-
return checkboxMenuItem;
372+
return setupMenuItem(attrs + ' type="radio"');
376373
}
377-
});
378374

379-
describe('type="radio"', function() {
380375
it('compiles', function() {
381376
var menuItem = setup('ng-model="test"')[0];
382377
expect(menuItem.classList.contains('md-indent')).toBe(true);
383378
var children = menuItem.children;
384379
expect(children[0].nodeName).toBe('MD-ICON');
385380
expect(children[1].nodeName).toBe('MD-BUTTON');
386381
});
382+
it('compiles with ng-repeat', function() {
383+
var menuItem = setup('ng-repeat="i in [1, 2, 3]"')[0];
384+
expect(menuItem.classList.contains('md-indent')).toBe(true);
385+
var children = menuItem.children;
386+
expect(children[0].nodeName).toBe('MD-ICON');
387+
expect(children[1].nodeName).toBe('MD-BUTTON');
388+
});
387389
it('sets aria role', function() {
388390
var menuItem = setup()[0].querySelector('md-button');
389391
expect(menuItem.getAttribute('role')).toBe('menuitemradio');
@@ -417,27 +419,24 @@ describe('material.components.menuBar', function() {
417419
expect(menuItem.children[0].style.display).toBeFalsy();
418420
expect(button.getAttribute('aria-checked')).toBe('true');
419421
}));
422+
});
420423

421-
function setup(attrs) {
422-
attrs = attrs || '';
423-
424-
var template = '<md-menu-item type="radio" ' + attrs + '>Test Item</md-menu-item>';
425-
426-
var radioMenuItem;
427-
inject(function($compile, $rootScope) {
428-
// We need to have a `md-menu-bar` as a parent of our menu item, because the menu-item
429-
// is only wrapping and indenting the content if it's inside of a menu bar.
430-
var menuBarMock = angular.element('<md-menu-bar>');
431-
var itemEl = angular.element(template);
424+
function setupMenuItem(attrs) {
425+
// We need to have a `md-menu-bar` as a parent of our menu item, because the menu-item
426+
// is only wrapping and indenting the content if it's inside of a menu bar.
427+
var menuBar;
428+
var template =
429+
'<md-menu-bar>' +
430+
'<md-menu-item ' + (attrs = attrs || '') + '>Test Item</md-menu-item>' +
431+
'</md-menu-bar>';
432432

433-
menuBarMock.append(itemEl);
434-
radioMenuItem = $compile(itemEl)($rootScope);
433+
inject(function($compile, $rootScope) {
434+
menuBar = $compile(template)($rootScope);
435+
$rootScope.$digest();
436+
});
435437

436-
$rootScope.$digest();
437-
});
438-
return radioMenuItem;
439-
}
440-
});
438+
return menuBar.find('md-menu-item');
439+
}
441440
});
442441

443442
function waitForMenuOpen() {

0 commit comments

Comments
 (0)