fix(list): empty aria-label attributes for list-items with interpolation #10218

Merged
merged 2 commits into from Jan 11, 2017
@DevVersion
Member
  • The md-list-item component determines the aria-label from the content if no aria-label is specified.
    Right now the md-list-item is not able to determine the label from interpolated content and shows a lot of $mdAria warnings (See the warnings issue here)

  • Fixes that the md-list-item complains twice about a missing aria-label, because the label was expected on the list and on the underlaying button element. (See the issue here)

  • Fixes a potential issue in the $mdAriaProvider where the provider class instance is applied to the service.

@ThomasBurleson This should go into 1.1.2, because otherwise developers would have a lot of warnings in their applications.

@DevVersion DevVersion fix(list): empty aria-label attributes for list-items with interpolation
* The `md-list-item` component determines the aria-label from the content if no `aria-label` is specified.
  Right now the `md-list-item` is not able to determine the label from interpolated content and shows a lot of $mdAria warnings

* Fixes that the `md-list-item` complains twice about a missing `aria-label`, because the label was expected on the list and on the underlaying button element.

* Fixes a potential issue in the `$mdAriaProvider` where the provider class instance is applied to the service.
46471e7
@googlebot googlebot added the cla: yes label Jan 4, 2017
@ErinCoughlan

LGTM

- * By default the warnings are enabled
- */
- self.showWarnings = true;
+ var config = {
@ThomasBurleson
ThomasBurleson Jan 5, 2017 Contributor

Do we have an specs for these Provider changes ?

@DevVersion
DevVersion Jan 5, 2017 edited Member

Since the changes won't be noticeable from outside, testing will be be difficult and IMO not necessary.

src/core/services/aria/aria.js
return {
disableWarnings: disableWarnings,
- $get: function($$rAF, $log, $window, $interpolate) {
- return MdAriaService.apply(self, arguments);
+ $get: function($$rAF, $log, $q, $window, $interpolate) {
@ThomasBurleson
ThomasBurleson Jan 5, 2017 Contributor

Why did you inject $q when I do not see it being used [in these new changes].

@DevVersion
DevVersion Jan 5, 2017 Member

Good catch. I had that for debugging the expectAsync function.

@DevVersion DevVersion Remove $q injection
ae69093
@ThomasBurleson
Contributor

LGTM

@ThomasBurleson ThomasBurleson modified the milestone: 1.1.2, 1.1.3 Jan 5, 2017
@ThomasBurleson ThomasBurleson modified the milestone: 1.1.3, 1.1.2 Jan 6, 2017
@ThomasBurleson
Contributor
ThomasBurleson commented Jan 6, 2017 edited

@jelbourn - Sample Console errors BEFORE this PR is merged:

screen shot 2017-01-06 at 8 17 08 am

Above output is for this view: https://material.angularjs.org/HEAD/demo/list

@tinayuangao tinayuangao merged commit 3556d57 into angular:master Jan 11, 2017

2 checks passed

cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@DevVersion DevVersion deleted the DevVersion:fix/list-aria-label-error branch Jan 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment