-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/. If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits. Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name. |
<md-subheader class="md-no-sticky">Single Action Checkboxes</md-subheader> | ||
<md-item ng-repeat="topping in toppings"> | ||
<p> {{ topping.name }} </p> | ||
<md-checkbox class="md-secondary" ng-model="topping.wanted" aria-label="{{ topping.name }}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder how many people are going to leave off the aria-label
here. I think the way you have it coded will already be covered by the $mdAria
warnings, but I'm wondering if it makes more sense to associate the <p>
with the checkbox rather than duplicating {{topping.name}}
. Either with aria-labelledby
or a more clear element API. Just want to throw it out there though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcysutton completely defer to you in all things aria. How should we do it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Edited) After looking at the spec, I noticed that "a checkbox can either be a primary action or a secondary action." Our challenge is creating a clean, accessible API with so many possible combinations.
We could easily add some CSS to create a right-aligned checkbox for the md-secondary
class, that way it can have real text content. Like this:
<md-item ng-repeat="topping in toppings">
<md-checkbox class="md-secondary" ng-model="topping.wanted">
{{ topping.name }}
</md-checkbox>
</md-item>
Here's another scenario where the primary action is transcluded into a button, alongside a switch component:
<md-item ng-click="navigateTo(setting.extraScreen)" ng-repeat="setting in settings">
<md-icon md-svg-icon="{{setting.icon}}"></md-icon>
<p> {{ setting.name }} </p>
<md-switch class="md-secondary" ng-model="setting.enabled" aria-label="Toggle {{ setting.name }}"></md-switch>
</md-item>
I still think it would be more clear and easier to handle in all respects by just putting the primary action as a child without transcluding. Like this:
<md-item ng-repeat="setting in settings">
<button ng-click="navigateTo(setting.extraScreen)">
<md-icon md-svg-icon="{{setting.icon}}"></md-icon>
{{ setting.name }}
</button>
<md-switch class="md-secondary" ng-model="setting.enabled" aria-label="Toggle {{ setting.name }}"></md-switch>
</md-item>
The same could be done for primary switches:
<md-item ng-repeat="setting in settings">
<md-checkbox ng-model="message.selected">
{{message.title}}
</md-checkbox>
<md-icon class="md-secondary" ng-click="doSecondaryAction()", md-svg-icon="communication:message"></md-icon>
</md-item>
Lastly, and this is very important: existing components like checkboxes also should NOT be wrapped in <div role=button>
. It makes them unusable. By putting the primary action right inline like I have above, we can avoid a bunch of complexity and make it clear to developers what their actions are doing behind the scenes.
Note to self to re-add 38f0423 |
var proxyElement; | ||
if (proxyElement = tEl[0].querySelector(type)) { | ||
hasProxiedElement = true; | ||
proxyElement.setAttribute('tabindex', '-1'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This causes problems on checkboxes and switches, because those elements have to be focused in order for their semantics to be announced. They also shouldn't be nested in role=button
...can we just get rid of <div role=button>
if it's a switch?
Looking good so far, is this going to be part of 0.9? |
@ilanbiala we hope so. |
|
8c225f9
to
c8f3b82
Compare
Greatly simplify list API. This commit also adds support for various forms of list controls. See the demos for some examples.
8ca934c
to
d922365
Compare
@@ -4,7 +4,7 @@ var fs = require('fs'); | |||
var versionFile = __dirname + '/../dist/commit'; | |||
|
|||
module.exports = { | |||
ngVersion: '1.3.2', | |||
ngVersion: '1.3.15', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 👍 this will also upgrade ngAria to a better version
Closes angular#2021. Greatly simplify list API. This commit also adds support for various forms of list controls. See the demos for some examples.
No description provided.