fix(carousel): Improve accessibility - FRONT-3657 #2566
Conversation
papegaill
added
tag: breaking change
process: review needed
and removed
process: WIP
labels
papegaill
force-pushed
the
fix/FRONT-3657
branch
from
0909509
to
f44486f
Compare
| @@ -7,7 +7,8 @@ | |||
| - "sr_previous" (string) (default: 'Previous slides') screen reader label for previous button | |||
| - "sr_next" (string) (default: 'Next slides') screen reader label for next button | |||
| - "sr_navigation" (string) (default: 'Go to slide %d') screen reader label for navigation buttons | |||
| - "sr_autoplay" (string) (default: 'Carousel auto play') screen reader label for autoplay button | |||
| - "sr_play" (string) (default: 'Play carousel') screen reader label for the play button | |||
There was a problem hiding this comment.
we could add a backward compatibility check, to use the value of the previous "sr_autoplay" by default
similar to what was done here https://github.com/ec-europa/europa-component-library/blob/v3.4.0-dev/src/implementations/twig/components/content-item/content-item.html.twig#L42
| #{map.get(theme.$spacing, 's')} - #{$_outline-width} | ||
| ); | ||
| } | ||
| .ecl-carousel__autoplay { |
There was a problem hiding this comment.
so user no longer has the possibility to play/pause carousel on mobile? Is it on purpose?
There was a problem hiding this comment.
Yes it is, here are the specs for touch screen: https://www.figma.com/file/wAuZXP2e9NUrsT5C1EKNEk/Carousel?node-id=428%3A32040
There was a problem hiding this comment.
They update the spec, so no autoPlay on mobile anymore 👍
| } | ||
|
|
||
| .ecl-carousel__prev { | ||
| left: 0; |
There was a problem hiding this comment.
I guess that it should be inverted in the rtl css
There was a problem hiding this comment.
Already done in RTL CSS resource
| @@ -225,6 +264,37 @@ $_outline-width: null !default; | |||
|
|
|||
| .ecl-carousel__prev, | |||
There was a problem hiding this comment.
these buttons should be hidden on print
| ); | ||
| } | ||
| .ecl-carousel__autoplay { | ||
| display: none; |
There was a problem hiding this comment.
the buttons don't disapear at the the same breakpoint as the mobile version, so there are some cases where you have the mobile display, but still have the button
There was a problem hiding this comment.
Yes that's correct, https://www.figma.com/file/wAuZXP2e9NUrsT5C1EKNEk/Carousel?node-id=428%3A32040
| @@ -31,14 +31,17 @@ export class Carousel { | |||
| constructor( | |||
| element, | |||
| { | |||
| toggleSelector = '.ecl-carousel__toggle', | |||
| playSelector = '.ecl-carousel__play', | |||
| pauseSelector = '.ecl-carousel__pause', | |||
There was a problem hiding this comment.
not sure why we use css class selector here instead of data attributes as on the other components, but that would require some quite big changes...
Probably for next major version of this component
There was a problem hiding this comment.
I think I wanted to lighten the markup in a way... but indeed we'll have to align this as other components
emeryro
added
tag: enhancement
and removed
process: under review
tag: enhancement
labels
inert+tabindex='-1'as a fallback)previous & next buttons are moved in the control bar for mobile format (JS)
todo: