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

fix(ngAria): Apply ARIA attrs correctly #13483

Closed

Conversation

marcysutton
Copy link
Contributor

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. Also supports ngTrueValue and ngFalseValue as part of ngModel.

Closes #13078
Closes #11374
Closes #11830

cc @petebacondarwin (I got this done finally! 馃槃)

@petebacondarwin
Copy link
Member

Well done @marcysutton - thanks. We will get this in after 1.5.0 is released

<h2 id="ngvaluechecked">ngValue and ngChecked</h2>

To ease the transition between native inputs and custom controls, ngAria now supports
[ngValue](https://docs.angularjs.org/api/ng/directive/ngValue) and [ngChecked](https://docs.angularjs.org/api/ng/directive/ngChecked). The original directives were created for native inputs only, so ngAria extends
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These links should be {@link ng/directive/ngValue} and {@link ng/directive/ngChecked}.


The boolean `required` attribute is only valid for native form controls such as `input` and
`textarea`. To properly indicate custom element directives such as `<md-checkbox>` or `<custom-input>`
as required, using ngAria with {@link ng/directive/ngrequired ngRequired} will also add
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if ng/directive/ngrequired (instead of ng/directive/ngRequired) works.

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
});

it('should attach aria-required to textarea', function() {
compileElement('<textarea ng-model="val" required></textarea>');
it('should attach to custom controls with ngModel and required', function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excess leading whitespace :)

@gkalpak
Copy link
Member

gkalpak commented Feb 3, 2016

LGTM (the whitespace can be taken care of upon merging)

@gkalpak gkalpak closed this in d06431e Feb 3, 2016
@gkalpak
Copy link
Member

gkalpak commented Feb 5, 2016

@marcysutton, (a little too late) but I have some questions regarding dropping aria-multiline:

  1. Does it mean that it is not necessary to put it on <textarea> elements or does the developer have to add it themselves ?
  2. Isn't it possible to have custom textboxes (with [role="textbox"]) that accept multiline text. Couldn't/Shouldn't ngAria set aria-multiline in those cases ?

}

function shouldAttachRole(role, elem) {
// if element does not have role attribute
// AND element type is equal to role (if custom element has a type equaling shape) <-- remove?
// AND element is not INPUT
return !elem.attr('role') && (elem.attr('type') === role) && (elem[0].nodeName !== 'INPUT');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcysutton, why are we only checking against input ? Shouldn't all native controls (e.g. textarea, select, button) be excluded ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that line should be replaced with isNodeOneOf(elem, nodeBlackList)). It has already gone through the getShape function but that only checks type and role.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants