Skip to content

Commit

Permalink
fix(list): apply ripple only once and fix list-inner alignment for mu…
Browse files Browse the repository at this point in the history
…lti-line lists.

- Currently the list applied two ripples to list items with a `ng-click` / `proxy element`.
- The actual list-item content wasn't aligned correctly when using a multi-line list.

Fixes angular#7059
  • Loading branch information
devversion committed Feb 7, 2016
1 parent beceb39 commit ef0da76
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 13 deletions.
12 changes: 9 additions & 3 deletions src/components/list/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,15 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
tEl.addClass('md-proxy-focus');
} else {
// Element which holds the default list-item content.
container = angular.element('<div class="md-button md-no-style"><div class="md-list-item-inner"></div></div>');
container = angular.element(
'<button class="md-button md-no-style"><div class="md-list-item-inner"></div></button>'
);

// Button which shows ripple and executes primary action.
var buttonWrap = angular.element('<md-button class="md-no-style" md-no-focus-style></md-button>');
var buttonWrap = angular.element(
'<md-button class="md-no-style" md-no-focus-style></md-button>'
);

buttonWrap[0].setAttribute('aria-label', tEl[0].textContent);
copyAttributes(tEl[0], buttonWrap[0]);

Expand Down Expand Up @@ -226,7 +231,8 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {

var proxies = [],
firstChild = $element[0].firstElementChild,
hasClick = firstChild && hasClickEvent(firstChild);
hasClick = firstChild && firstChild.firstElementChild &&
hasClickEvent(firstChild.firstElementChild);

computeProxies();
computeClickable();
Expand Down
7 changes: 6 additions & 1 deletion src/components/list/list.scss
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,18 @@ md-list-item {
border: medium none;

> .md-button:first-child {
height: 100%;
position: absolute;
top: 0;
left: 0;

margin: 0;
padding: 0;
height: 100%;
}

.md-list-item-inner {
width: 100%;
height: 100%;
padding: 0 16px;
}

Expand Down
22 changes: 13 additions & 9 deletions src/components/list/list.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,20 @@ describe('mdListItem directive', function() {

it('moves aria-label to primary action', function() {
var listItem = setup('<md-list-item ng-click="sayHello()" aria-label="Hello"></md-list-item>');
var listItemChildren = listItem.children();
expect(listItemChildren[0].nodeName).toBe('DIV');
expect(listItemChildren).toHaveClass('md-button');
expect(listItemChildren.children()[0].getAttribute('aria-label')).toBe('Hello');
var listButtonWrap = listItem.children();
// The actual click button will be a child of the button.md-no-style wrapper.
var listItemButton = listButtonWrap.children();

expect(listButtonWrap).toHaveClass('md-button');
expect(listItemButton[0].nodeName).toBe('MD-BUTTON');
expect(listItemButton[0].getAttribute('aria-label')).toBe('Hello');
});

it('moves md-secondary items outside of the button', function() {
var listItem = setup('<md-list-item ng-click="sayHello()"><p>Hello World</p><md-icon class="md-secondary" ng-click="goWild()"></md-icon></md-list-item>');
// First child is our button and content holder
var firstChild = listItem.children().eq(0);
expect(firstChild[0].nodeName).toBe('DIV');
expect(firstChild[0].nodeName).toBe('BUTTON');
// It should contain two elements, the button overlay and the actual content
expect(firstChild.children().length).toBe(2);
var secondChild = listItem.children().eq(1);
Expand All @@ -150,7 +153,7 @@ describe('mdListItem directive', function() {
var listItem = setup('<md-list-item ng-click="sayHello()"><p>Hello World</p><md-icon class="md-secondary" ng-click="goWild()"><md-icon class="md-secondary" ng-click="goWild2()"></md-icon></md-list-item>');
// First child is our button and content holder
var firstChild = listItem.children().eq(0);
expect(firstChild[0].nodeName).toBe('DIV');
expect(firstChild[0].nodeName).toBe('BUTTON');
// It should contain two elements, the button overlay and the actual content
expect(firstChild.children().length).toBe(2);
var secondChild = listItem.children().eq(1);
Expand Down Expand Up @@ -226,10 +229,11 @@ describe('mdListItem directive', function() {

// There should only be 1 md-button (the wrapper) and one button (the unwrapped one)
expect(listItem.find('md-button').length).toBe(1);
expect(listItem.find('button').length).toBe(1);
// There will be two buttons, because of the button.md-no-style.md-button wrapper.
expect(listItem.find('button').length).toBe(2);

// Check that we didn't wrap the button in an md-button
expect(listItem[0].querySelector('md-button button')).toBeFalsy();
expect(listItem[0].querySelector('md-button button.md-secondary')).toBeFalsy();
});

it('should not wrap secondary md-buttons in a md-button', function() {
Expand All @@ -244,7 +248,7 @@ describe('mdListItem directive', function() {
expect(listItem.find('md-button').length).toBe(2);

// Check that we didn't wrap the md-button in an md-button
expect(listItem[0].querySelector('md-button md-button')).toBeFalsy();
expect(listItem[0].querySelector('md-button md-button.md-secondary')).toBeFalsy();
});
});

Expand Down

0 comments on commit ef0da76

Please sign in to comment.