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

a11y test #3

Closed
bartdominiak opened this issue Jun 12, 2020 · 4 comments
Closed

a11y test #3

bartdominiak opened this issue Jun 12, 2020 · 4 comments
Labels
enhancement New feature or request

Comments

@bartdominiak
Copy link
Owner

  • accesibility test
  • add necessary tags to manage carousel via keyboard (tab, arrows, space, enter)
@bartdominiak bartdominiak added the enhancement New feature or request label Jul 18, 2020
@plehnen
Copy link

plehnen commented Sep 22, 2020

Hi. I found this page via your comment on the vue-carousel issue page regarding vue3. It's great to see someone taking over!

We are currently using vue-carousel in a big project for a large company which still needs to support IE11 and has to fullfill a11y requirements. The audit forced us to fork the project and improve some features. I was hoping to use pull-requests to get this back to the main project, but as you said it doesn't look like it's still maintained.
So I would like to mention a few things here, which might be worth considering. Probably worth putting in seperate issues but right know it's easier to explain the needs in just a comment:

  • use semantic tags like ul/li, or at least support a "tag" property on wrapper and slide (e.g. <component :is="tag">)
  • support i18n (we need to translate all texts like aria-label="Slide right")
  • currently keyboard-navigation forces you to tab through all slide entries. Maybe the tabindex should only be on the interactive elements (prev/next buttons and pagination in the future). but for this it would need to have aria-hidden attributes, as well as a property passed to Slide's slot exposing if the tab is currently visible (to be able to manually disable interactive elements inside the slide, so keyboard navigation works flawlessly) (note: vue-carousel did also use aria role "tabpanel", but that's not implemented correctly and didn't pass our audit)

PS: currently I can't test vue-snap on my machine since I am using a node version > 14 which apparently is not supported.

@bartdominiak
Copy link
Owner Author

bartdominiak commented Sep 24, 2020

Thanks @plehnen for your feedback! Your information is very helpful :) I am preparing changes, they will be ready soon.

We are currently using vue-carousel in a big project for a large company which still needs to support IE11..

I tested it on IE11, and it's working correctly, but there is one thing on mobile - when user swipe to the next slide, then snap isn't stoping on edge of the current slide. It's not a bug, It's just a limitation of CSS Scroll Snap because is not supported by the older browsers.
And one more thing: AFAIK currently Vue@3 doesnt support IE11.

use semantic tags like ul/li, or at least support a "tag" property on wrapper and slide (e.g. )

Totally agree. I will add them.

support i18n (we need to translate all texts like aria-label="Slide right")

Good idea. Let's add another prop like :i18n and support all translations.

currently keyboard-navigation forces you to tab through all slide entries. Maybe the tabindex should only be on the interactive elements (prev/next buttons and pagination in the future). but for this it would need to have aria-hidden attributes, as well as a property passed to Slide's slot exposing if the tab is currently visible (to be able to manually disable interactive elements inside the slide, so keyboard navigation works flawlessly) (note: vue-carousel did also use aria role "tabpanel", but that's not implemented correctly and didn't pass our audit).

Good eye! I'm working on it.

PS: currently I can't test vue-snap on my machine since I am using a node version > 14 which apparently is not supported.

My bad, I limited node to only LTS version, but I will extend it :)

@plehnen
Copy link

plehnen commented Sep 24, 2020

One thought regarding i18n translations:
When changing the language (e.g. with vue-i18n) all $t('language-specific-text') will be replaced with the correct language.
This works in as well as via computed properties.
Some plugins have their texts implemented via options and need a complete re-render of the component to make the language change visible. (e.g. page reload)
So in case you plan to add such an i18n feature (which I hope you do) please make sure it doesn't require the page to reload when the text changes.

PS: We plan to migrate our project to vue3 (and presumably vue-caroussel to vue-snap) as soon as vue3 supports ie11 and the majority of plugins are migrated too. So not before Q4.

This was referenced Sep 24, 2020
@bartdominiak
Copy link
Owner Author

bartdominiak commented Sep 24, 2020

Thanks @plehnen.
Released vue-snap@0.6.1 with critical changes. I moved other issues to seperate tickets like i18n feature, and aria-* attributes for visible part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants