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

bug(ngAria): update aria-valuemin/max when min/max change #11770

Closed
gkalpak opened this issue Apr 30, 2015 · 4 comments
Closed

bug(ngAria): update aria-valuemin/max when min/max change #11770

gkalpak opened this issue Apr 30, 2015 · 4 comments

Comments

@gkalpak
Copy link
Member

gkalpak commented Apr 30, 2015

Currently, ngAria checks attr.min/max in a 200-priority directive's post-linking function and sets aria-valuemin/aria-valuemax accordingly.

There are 2 issues with this approach:

  1. ngMin/ngMax will also result in attr.min/max being updated, but only after ngAria has set aria-valuemin/max. As a result, ngAria is effectively not taking into account ngMin/ngMax (while I think it should).
  2. If min/max change later on, aria-valuemin/max are not updated, resulting in misleading info (accessibility loses 😢).

Luckily, solving (2) will automatically fix (1) as well.

So, I believe that ngAria should attr.$observe certain attributes and update aria values accordingly.
This is probably true for other attributes besides min/max (@marcy should know better).

@gkalpak gkalpak self-assigned this Apr 30, 2015
@gkalpak gkalpak removed their assignment Apr 30, 2015
gkalpak added a commit to gkalpak/angular.js that referenced this issue Apr 30, 2015
As a result of thi fix, `ngMin/Max` also set `aria-valuemin/max` on
"range"-shaped elements.

Fixes angular#11770
@Narretz Narretz added this to the 1.4.x - jaracimrman-existence milestone Apr 30, 2015
@marcysutton
Copy link
Contributor

@gkalpak ngAria already binds $watch for many of the attributes it sets, including aria-valuenow, however this is only if the element has type=range, role="slider" or role="progressbar". It doesn't yet bind to date inputs, but I don't know that it needs to. If people are creating date pickers out of DIVs (which is completely likely due to lack of native browser support), then probably yes it should.

It's worth noting that ARIA generally isn't necessary on native inputs, only custom controls like <some-slider> or <div class="someSlider">. ngAria is currently undergoing a refactor to prune out redundant attributes, like aria-valuenow on <input type="range">.

Here is a sandbox where you can see ngAria in action, see the first "Test Slider" component, which has min and max, as well as aria-valuemin, aria-valuemax and aria-valuenow. It doesn't need all of those to be accessible, so it's only adding code bloat. http://codepen.io/marcysutton/pen/wBwVqM

I don't necessarily think there is an action item right now, I just wanted you to be aware of what ngAria currently does and what it should/shouldn't do. :)

@gkalpak
Copy link
Member Author

gkalpak commented May 6, 2015

@marcysutton, thx for the insight. Note, though, that my 2 points above are still valid (aka action items), since:

  1. is about ngMin/ngMax (which assistive technologies know nothing about) and
  2. is about updating aria-valuemin/max when min/max changes dynamically (not on native input, but on elements that ngAria already sets aria-valuemin/max).

I just realized there is another issue (which #11774 does not address): aria-valuemin/max (and possibly other properties) should be set on native inputs when using ngMin/ngMax (which do not set min/max).

@marcysutton
Copy link
Contributor

Oh yes, you're totally right. Although your last point about aria-valuemin/max sounds the same as item 1.

To clear up what needs to happen (at least for me):

  • ngAria should add ARIA support to new directives ngMin and ngMax (native or custom elements)
    • Caveat: Is there any potential for aria-valuemin to conflict with min, for example, if the developer is using a different value in min than ngMin? Which takes priority?
  • ngAria should watch min and max for changes on custom range elements to update aria-valuemin/max in addition to aria-valuenow
    • Caveat: ngAria currently only binds those properties to input[type=range], role="slider" and role="progressbar", not input[type=date].

@gkalpak
Copy link
Member Author

gkalpak commented May 8, 2015

My item (1) was that ngMin/ngMax should be taken into account on elements on which ngAria already cares for min/max (i.e. role="slider" etc).
My item (3) is that ngMin/ngMax should be taken into account even on native elements (where ngAria does not care about min/max, since they are accessible by default).

But, you are right, if I had thought of (3) from the beginning, it would be merged into (1).
The practical difference is that my PR only fixes (1) not (3) (yet).

if the developer is using a different value in min than ngMin? Which takes priority?

It seems that ngMin takes priority as far as Angular is concerned. The HTML5 validation only cares about min though.

ngAria should watch min and max for changes on custom range elements to update aria-valuemin/max in addition to aria-valuenow

Just to be clear, it should watch (rather observe) attrs.min/attrs.max (not the HTML attributes min/max) in order to also account for ngMin/ngMax.

gkalpak added a commit to gkalpak/angular.js that referenced this issue May 12, 2015
As a result of thi fix, `ngMin/Max` also set `aria-valuemin/max` on
"range"-shaped elements.

Fixes angular#11770
@gkalpak gkalpak closed this as completed in ebaa0f5 Jun 3, 2015
netman92 pushed a commit to netman92/angular.js that referenced this issue Aug 8, 2015
As a result of thi fix, `ngMin/Max` also set `aria-valuemin/max` on
"range"-shaped elements.

Fixes angular#11770

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

Successfully merging a pull request may close this issue.

3 participants