From ef0da76e92ad53c788d2f7a87d8f78e6e1720191 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sun, 7 Feb 2016 23:07:15 +0100 Subject: [PATCH] fix(list): apply ripple only once and fix list-inner alignment for multi-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 #7059 --- src/components/list/list.js | 12 +++++++++--- src/components/list/list.scss | 7 ++++++- src/components/list/list.spec.js | 22 +++++++++++++--------- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/components/list/list.js b/src/components/list/list.js index 3ff398421b9..390a1d55a3e 100644 --- a/src/components/list/list.js +++ b/src/components/list/list.js @@ -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('
'); + container = angular.element( + '' + ); // Button which shows ripple and executes primary action. - var buttonWrap = angular.element(''); + var buttonWrap = angular.element( + '' + ); + buttonWrap[0].setAttribute('aria-label', tEl[0].textContent); copyAttributes(tEl[0], buttonWrap[0]); @@ -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(); diff --git a/src/components/list/list.scss b/src/components/list/list.scss index 4290f34f117..1dfe541cd4e 100644 --- a/src/components/list/list.scss +++ b/src/components/list/list.scss @@ -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; } diff --git a/src/components/list/list.spec.js b/src/components/list/list.spec.js index 1d5066555e7..525183bb7be 100644 --- a/src/components/list/list.spec.js +++ b/src/components/list/list.spec.js @@ -128,17 +128,20 @@ describe('mdListItem directive', function() { it('moves aria-label to primary action', function() { var listItem = setup(''); - 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('

Hello World

'); // 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); @@ -150,7 +153,7 @@ describe('mdListItem directive', function() { var listItem = setup('

Hello World

'); // 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); @@ -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() { @@ -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(); }); });