Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

[ngAria] aria-required only applied when ng-model has $error.required #11374

Closed
dpogue opened this issue Mar 19, 2015 · 7 comments
Closed

[ngAria] aria-required only applied when ng-model has $error.required #11374

dpogue opened this issue Mar 19, 2015 · 7 comments

Comments

@dpogue
Copy link

dpogue commented Mar 19, 2015

Based on a Twitter conversation with @marcysutton: https://twitter.com/dpogue/status/569999089967849472

If I have <input type="text" ng-model="username" name="username" required> and I'm using ngAria then it should have the aria-required attribute added to it.

This currently only happens when the field doesn't have content (i.e., when username.$error.required is true):
https://github.com/angular/angular.js/blob/master/src/ngAria/aria.js#L292-L298

@marcysutton
Copy link
Contributor

A native input doesn't need aria-required. Custom elements do, though. If it was <md-checkbox role="checkbox" required>, aria-required would need to be added. That said, I'm already seeing ngAria correctly add the attribute on a custom element with required: http://codepen.io/marcysutton/pen/wBwVqM

@Narretz
Copy link
Contributor

Narretz commented Mar 22, 2015

So this works as expected @marcysutton?

@marcysutton
Copy link
Contributor

Unless I'm missing something, it looked like it worked fine to me. @dpogue can you elaborate? Is the issue happening when ng-model is set up in a specific way? Can you provide a Codepen or Plunker that shows the issue?

@marcysutton
Copy link
Contributor

@dpogue can you reply to this issue? Otherwise we'll close it as not a bug.

@dpogue
Copy link
Author

dpogue commented Apr 5, 2015

My concern was just that the aria-required attribute was being added and removed based on the error state of the ngModel, instead of being added once and maintained. If that's not an issue, then this can be closed.

@Narretz
Copy link
Contributor

Narretz commented Aug 7, 2015

According to this mdn example, it looks like aria-required is always present: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-required_attribute

So, for non-native inputs (controls) with ng-model that have a required attribute should this mean?

  • if they have only the required attribute, ngAria should add aria-required once.
  • if they have ngRequired, they should watch that and toggle aria-required appropriately.

@marcysutton
Copy link
Contributor

That sounds right. Upon looking closer, I found that aria-required in ngAria is super buggy for expected use cases (custom elements with roles). Depending on whether the custom control has type="checkbox" (or other form type) or role="checkbox" (which it needs for assistive tech), it can get out of sync with required. For example:

<some-checkbox type="checkbox" ng-required="true" ng-model="data">
 Checkbox
</some-checkbox>
{
  "type": "checkbox",
  "ng-model": "data",
  "ng-required": "true",
  "tabindex": "0",
  "required": "required",
  "aria-checked": "false",
  "aria-required": "true"
}

By including type=checkbox, aria-required is maintained for both truthy and falsey values. Great, except role=checkbox is needed here, not type, and ngAria should not require both type and role on a custom element: role should suffice.

With role="checkbox" only, for falsey values, aria-required="false" is included correctly from the start. For truthy values, aria-required has a false value. This is a bug with the validator logic in ngAria.

<some-checkbox role="checkbox" show-attrs ng-model="data" required>
  Checkbox
</some-checkbox>
{
  "role": "checkbox",
  "ng-model": "data",
  "ng-required": "true",
  "class": "ng-pristine ng-untouched ng-valid",
  "tabindex": "0",
  "required": "required",
  "aria-checked": "true",
  "aria-required": "false"
}

Pretty much exactly the same thing happens with required–with role=checkbox and required, ngAria adds aria-required="false" when it should be true. With type, it works as expected.

marcysutton added a commit to marcysutton/angular.js that referenced this issue Dec 9, 2015
BREAKING CHANGE: Where appropriate, ngAria now applies ARIA to custom controls only, not native inputs. Because of this, support for `aria-multiline` on textareas has been removed. 

New support added for ngValue, ngChecked, and ngRequired, along with updated documentation.

Closes angular#13078
Closes angular#11374
Closes angular#11830
marcysutton added a commit to marcysutton/angular.js that referenced this issue Jan 18, 2016
BREAKING CHANGE: Where appropriate, ngAria now applies ARIA to custom controls only, not native inputs. Because of this, support for `aria-multiline` on textareas has been removed. 

New support added for ngValue, ngChecked, and ngRequired, along with updated documentation.

Closes angular#13078
Closes angular#11374
Closes angular#11830
marcysutton added a commit to marcysutton/angular.js that referenced this issue Feb 2, 2016
BREAKING CHANGE: Where appropriate, ngAria now applies ARIA to custom controls only, not native inputs. Because of this, support for `aria-multiline` on textareas has been removed. 

New support added for ngValue, ngChecked, and ngRequired, along with updated documentation.

Closes angular#13078
Closes angular#11374
Closes angular#11830
marcysutton added a commit to marcysutton/angular.js that referenced this issue Feb 3, 2016
BREAKING CHANGE: Where appropriate, ngAria now applies ARIA to custom controls only, not native inputs. Because of this, support for `aria-multiline` on textareas has been removed. 

New support added for ngValue, ngChecked, and ngRequired, along with updated documentation.

Closes angular#13078
Closes angular#11374
Closes angular#11830
marcysutton added a commit to marcysutton/angular.js that referenced this issue Feb 3, 2016
BREAKING CHANGE: Where appropriate, ngAria now applies ARIA to custom controls only, not native inputs. Because of this, support for `aria-multiline` on textareas has been removed. 

New support added for ngValue, ngChecked, and ngRequired, along with updated documentation.

Closes angular#13078
Closes angular#11374
Closes angular#11830
@gkalpak gkalpak closed this as completed in d06431e Feb 3, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants