Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accessibility #68

Closed
callumacrae opened this issue Jun 13, 2017 · 39 comments
Closed

Accessibility #68

callumacrae opened this issue Jun 13, 2017 · 39 comments

Comments

@callumacrae
Copy link

Hey!

This component currently doesn't work with screen readers - because it's made of divs, the screen reader doesn't know it's an input element and goes straight over it. I'd recommend duplicating the state to a hidden range input for screenreaders to use.

Also, are you fine if I use this library as an example in a talk of an input library which doesn't work with screen readers? It 100% isn't intended as an attack and it won't be the only library I talk about - most libraries in the Vue ecosystem have the same problem!

@NightCatSama
Copy link
Owner

NightCatSama commented Jun 13, 2017

Because I do not have a screen readers, so it is difficult to reproduce the problem.

About add a hidden range input element to compatibility, is refers to the events would only on the range input trigger on a screen reader?

If yes, that would be very complicated to modify.

@callumacrae
Copy link
Author

It wouldn't be tricky. Just add an <input type="range"> slider with a screen reader only class, and put aria-hidden="true" on the current HTML. Then you can bind the range slider to the variable with v-model and they would reflect each other's state.

@NightCatSama
Copy link
Owner

NightCatSama commented Jun 28, 2017

Whether only need to add a hidden input(<input type="range" aria-hidden="true" style="opacity: 0">), and then synchronize the value of the component to the input value can be completed.

@callumacrae
Copy link
Author

you want the aria-hidden attribute on the current HTML for vue-slider-component, or it will hide the range slider from the screenreader - exactly the opposite of what you want to do!

@NightCatSama
Copy link
Owner

I get it, I will support this feature.

thank for your issue, this will be a meaningful update.

@callumacrae
Copy link
Author

amazing, thank you!

@NightCatSama
Copy link
Owner

NightCatSama commented Jun 28, 2017

I added some accessibility attributes, but I do not have a device to test, so can you please help me test it?

I did not update the new version because I did not know if it was available.

test url: https://nightcatsama.github.io/vue-slider-component/example/

this is my references ant's slider.

@callumacrae
Copy link
Author

Still doesn't work, because the screenreader doesn't know it's a slider. trying setting role="slider": https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_slider_role

@NightCatSama
Copy link
Owner

This attribute exists.

<div 
  //...
  role="slider"
  :aria-valuemin="min"
  :aria-valuemax="max"
  :aria-valuenow="val"
  aria-disabled="false"
  aria-hidden="false"
>
..
</div>

@callumacrae
Copy link
Author

I'm not able to change it using a screen reader :(

@NightCatSama
Copy link
Owner

Can you tell me whether the slider on the ant can be used? slider

@callumacrae
Copy link
Author

Nope, unable to use that with VoiceOver on Safari.

@NightCatSama
Copy link
Owner

It may still have to use the <input type="range">, but I don't know how to hide the input tag in the common equipment.

@callumacrae
Copy link
Author

See the article I linked to before: http://www.coolfields.co.uk/2016/05/text-for-screen-readers-only-updated/

.screen-reader-text { 
   clip: rect(1px, 1px, 1px, 1px); 
   height: 1px; 
   width: 1px; 
   overflow: hidden; 
   position: absolute !important;
}

stuff like that is pretty bulletproof

@NightCatSama
Copy link
Owner

I thought that this applies only to the text.

I'll give it a try.

@callumacrae
Copy link
Author

nah, works for everything - it's more commonly implemented as .sr-only

@NightCatSama
Copy link
Owner

NightCatSama commented Jun 28, 2017

I have already corrected. demo

Then I found that there is a problem with this scheme, not support for the range mode and custom data mode.

@callumacrae
Copy link
Author

huh? not sure i follow

@NightCatSama
Copy link
Owner

Don't work?

@callumacrae
Copy link
Author

Oh, right, I misunderstood.

That works, mostly! It reads out the existing value and it's possible to change it. The only problem is that the visually displayed slider, while the number displayed changes, the position of the handle does not.

@NightCatSama
Copy link
Owner

Do you mean that the slider position does not move when the value of input changes?

@callumacrae
Copy link
Author

Yes, exactly. The value has changed, but it isn't reflected in the UI. Not a huge issue as it won't really affect anyone (the people using screenreaders generally won't see the UI anyway), but worth fixing if it's an easy fix.

@NightCatSama
Copy link
Owner

I do not know where the problem is, and it's hard for me to debug.

@callumacrae
Copy link
Author

callumacrae commented Jun 29, 2017

¯\_(ツ)_/¯

I think what you've done is enough - it works for people using screen readers now. Good job, thanks!

@aphofstede
Copy link

Range mode is currently not supported for accessibility. Would it make sense to add two inputs in that case? (from/to) Not sure how screen readers will pick that up.

@spatras
Copy link

spatras commented Aug 3, 2019

Hello,
I see that the slider doesn't support the screen reader. It has aria-hidden=true. Do you intend to change this? Or at least to give the option to change the aria-hidden value?
Thanks

@NightCatSama
Copy link
Owner

@spatras

So aria-hidden can't be set to true? I don't understand very well, I wrote it according to the previous version (v2).

<div ref="elem" aria-hidden="true" class="vue-slider" :style="[elemStyles, bgStyle]">

I can't debug on the screen reader, if you can, please mention pr perfect components. Thanks

@spatras
Copy link

spatras commented Aug 4, 2019

aria-hidden si set to trueby default. I would like to be customisable to set it false. If I add aria-valuemax, and other aria properties on the handler, it doesn't work becasue the parent has aria-hidden true. It would be great if you could add a prop for aria-hidden value.
Thanks!

@NightCatSama
Copy link
Owner

@spatras

So is aria-hidden= "true" a remained problems? If so, then I set it to false by default to have other effects.

@spatras
Copy link

spatras commented Aug 4, 2019

Yes. aria-hidden="true" is hiding the element and all it's childrens from the screen reader. You could remove the attribute entierly.

@NightCatSama
Copy link
Owner

@spatras Cancel this attribute in v3.0.34, and support adding custom attributes directly.

@spatras
Copy link

spatras commented Aug 4, 2019

Awesome! Thanks!

@spatras
Copy link

spatras commented Aug 9, 2019

Hi again!
Your last update helped a lot and many thanks for that but I found that there is an hidden input .vue-slider-sr-only which appear only when there is 1 slider handle and is read by the screen reader but can't interact with it. Can you remove that?

@NightCatSama
Copy link
Owner

@spatras This is the code that was previously contributed by others. Can it be compatible with screen readers if it is removed?

@spatras
Copy link

spatras commented Aug 11, 2019

@NightCatSama That code doesn't make the slider screen reader compatible. It only appears when there is only 1 handle and also, it cannot be changed from the screen reader. Maybe was doing more previously, but right doesn't do anything. And previously, when you had aria-hidden it was not read by the sr. For example users can add aria-valuenow, aria-label... and so on, to a custom handle and then you achieve fully sr compatibility.

@NightCatSama
Copy link
Owner

@spatras Sorry, I don't know much about the compatibility of screen readers. Is it only necessary to delete <input class="vue-slider-sr-only" />?

@spatras
Copy link

spatras commented Aug 11, 2019 via email

@NightCatSama
Copy link
Owner

@spatras Already removed, please update to v3.0.35.

@spatras
Copy link

spatras commented Aug 12, 2019

Works great! Thank you!

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

No branches or pull requests

4 participants