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

Commit 5cc492c

Browse files
topherfangiorschmukler
authored andcommitted
fix(list): Don't wrap secondary buttons in a button.
We currently wrap many `.md-secondary` items in an extra button for accessibility purposes. However, there is no need to wrap an existing button inside an additional one as it creates odd behavior and styling. Additionally, fix some styles so that secondary buttons render properly (position was off due to margins). Lastly, add some new demo code to show how to use this functionality. Fixes #3176. Closes #5721
1 parent 4306331 commit 5cc492c

File tree

6 files changed

+91
-4
lines changed

6 files changed

+91
-4
lines changed

src/components/list/demoListControls/index.html

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<md-list ng-controller="ListCtrl" ng-cloak>
2+
23
<md-subheader class="md-no-sticky">Single Action Checkboxes</md-subheader>
34
<md-list-item ng-repeat="topping in toppings">
45
<p> {{ topping.name }} </p>
@@ -7,6 +8,18 @@
78

89
<md-divider></md-divider>
910

11+
<md-subheader class="md-no-sticky">Secondary Buttons</md-subheader>
12+
<md-list-item class="secondary-button-padding">
13+
<p>Clicking the button to the right will fire the secondary action</p>
14+
<md-button class="md-secondary" ng-click="doSecondaryAction($event)">More Info</md-button>
15+
</md-list-item>
16+
<md-list-item class="secondary-button-padding" ng-click="doPrimaryAction($event)">
17+
<p>Click anywhere to fire the primary action, or the button to fire the secondary</p>
18+
<md-button class="md-secondary" ng-click="doSecondaryAction($event)">More Info</md-button>
19+
</md-list-item>
20+
21+
<md-divider></md-divider>
22+
1023
<md-subheader class="md-no-sticky">Clickable Items with Secondary Controls</md-subheader>
1124
<md-list-item ng-click="navigateTo(setting.extraScreen, $event)" ng-repeat="setting in settings">
1225
<md-icon md-svg-icon="{{setting.icon}}"></md-icon>

src/components/list/demoListControls/script.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,17 @@ angular.module('listDemo2', ['ngMaterial'])
5353
);
5454
};
5555

56+
$scope.doPrimaryAction = function(event) {
57+
$mdDialog.show(
58+
$mdDialog.alert()
59+
.title('Primary Action')
60+
.content('Primary actions can be used for one click actions')
61+
.ariaLabel('Primary click demo')
62+
.ok('Awesome!')
63+
.targetEvent(event)
64+
);
65+
};
66+
5667
$scope.doSecondaryAction = function(event) {
5768
$mdDialog.show(
5869
$mdDialog.alert()

src/components/list/demoListControls/style.css

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,8 @@ md-list-item .md-list-item-inner > .md-list-item-inner > p {
1212
-ms-user-select: none; /* IE 10+ */
1313
user-select: none; /* Likely future */
1414
}
15+
16+
/* Add some right padding so that the text doesn't overlap the buttons */
17+
.secondary-button-padding p {
18+
padding-right: 100px;
19+
}

src/components/list/list.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
150150
tEl[0].setAttribute('tabindex', '-1');
151151
tEl.append(container);
152152

153-
if (secondaryItem && secondaryItem.hasAttribute('ng-click')) {
153+
if (secondaryItem && !isButton(secondaryItem) && secondaryItem.hasAttribute('ng-click')) {
154154
$mdAria.expect(secondaryItem, 'aria-label');
155155
var buttonWrapper = angular.element('<md-button class="md-secondary-container md-icon-button">');
156156
buttonWrapper.attr('ng-click', secondaryItem.getAttribute('ng-click'));
@@ -176,6 +176,12 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
176176
return proxiedTypes.indexOf(el.nodeName.toLowerCase()) != -1;
177177
}
178178

179+
function isButton(el) {
180+
var nodeName = el.nodeName.toUpperCase();
181+
182+
return nodeName == "MD-BUTTON" || nodeName == "BUTTON";
183+
}
184+
179185
return postLink;
180186

181187
function postLink($scope, $element, $attr, ctrl) {

src/components/list/list.scss

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,15 +171,15 @@ md-list-item, md-list-item .md-list-item-inner {
171171

172172
.md-secondary-container,
173173
.md-secondary {
174-
margin-left: $list-item-secondary-left-margin;
175174
position: absolute;
176-
right: $list-item-padding-horizontal;
177175
top: 50%;
176+
right: $list-item-padding-horizontal;
177+
margin: 0 0 0 $list-item-secondary-left-margin;
178178
transform: translate3d(0, -50%, 0);
179179
}
180180

181181
& > .md-button.md-secondary-container > .md-secondary {
182-
margin-left: 0px;
182+
margin-left: 0;
183183
position: static;
184184
}
185185

src/components/list/list.spec.js

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,4 +157,56 @@ describe('mdListItem directive', function() {
157157
expect(listItem.hasClass('md-no-proxy')).toBeTruthy();
158158
});
159159

160+
describe('with a clickable item', function() {
161+
162+
it('should wrap secondary icons in a md-button', function() {
163+
var listItem = setup(
164+
'<md-list-item ng-click="something()">' +
165+
' <p>Content Here</p>' +
166+
' <md-icon class="md-secondary" ng-click="heart()">heart</md-icon>' +
167+
'</md-list-item>'
168+
);
169+
170+
var buttons = listItem.find('md-button');
171+
172+
// Check that we wrapped the icon
173+
expect(listItem[0].querySelector('md-button > md-icon')).toBeTruthy();
174+
175+
// Check the button actions
176+
expect(buttons[0].attributes['ng-click'].value).toBe("something()");
177+
expect(buttons[1].attributes['ng-click'].value).toBe("heart()");
178+
});
179+
180+
it('should not wrap secondary buttons in a md-button', function() {
181+
var listItem = setup(
182+
'<md-list-item ng-click="something()">' +
183+
' <p>Content Here</p>' +
184+
' <button class="md-secondary" ng-click="like()">Like</button>' +
185+
'</md-list-item>'
186+
);
187+
188+
// There should only be 1 md-button (the wrapper) and one button (the unwrapped one)
189+
expect(listItem.find('md-button').length).toBe(1);
190+
expect(listItem.find('button').length).toBe(1);
191+
192+
// Check that we didn't wrap the button in an md-button
193+
expect(listItem[0].querySelector('md-button button')).toBeFalsy();
194+
});
195+
196+
it('should not wrap secondary md-buttons in a md-button', function() {
197+
var listItem = setup(
198+
'<md-list-item ng-click="something()">' +
199+
' <p>Content Here</p>' +
200+
' <md-button class="md-secondary" ng-click="like()">Like</md-button>' +
201+
'</md-list-item>'
202+
);
203+
204+
// There should be 2 md-buttons that are siblings
205+
expect(listItem.find('md-button').length).toBe(2);
206+
207+
// Check that we didn't wrap the md-button in an md-button
208+
expect(listItem[0].querySelector('md-button md-button')).toBeFalsy();
209+
});
210+
});
211+
160212
});

0 commit comments

Comments
 (0)