From a1f442336cb7664c5f6360528dff9b89c1a17e75 Mon Sep 17 00:00:00 2001 From: Pavel Kovirshin <45898580+PahaN47@users.noreply.github.com> Date: Fri, 27 Sep 2024 17:18:32 +0300 Subject: [PATCH] fix: slider a11y (#982) * fix: slider a11y * fix: more slider a11y features * fix: requested changes and fixes * fix: removed extra attribute * fix: old slider changes * fix: tests * fix: inproper in * fix: less code * fix: even less code * fix: removed auto focus on new slider * fix: requested changes * fix: spaces * fix: tests * fix: visual tests --- src/blocks/Slider/Slider.scss | 2 +- src/blocks/Slider/Slider.tsx | 218 ++++++++++++++------ src/blocks/Slider/__tests__/Slider.test.tsx | 6 +- src/blocks/Slider/i18n/en.json | 4 +- src/blocks/Slider/i18n/ru.json | 4 +- src/blocks/Slider/utils.ts | 110 ++++++++++ src/blocks/SliderNew/Arrow/Arrow.tsx | 10 +- src/blocks/SliderNew/Slider.tsx | 60 ++++-- src/blocks/SliderNew/i18n/en.json | 4 +- src/blocks/SliderNew/i18n/ru.json | 4 +- src/blocks/SliderNew/useSlider.tsx | 20 +- src/blocks/SliderNew/useSliderPagination.ts | 49 +++++ src/blocks/SliderNew/utils.ts | 20 ++ 13 files changed, 409 insertions(+), 102 deletions(-) create mode 100644 src/blocks/SliderNew/useSliderPagination.ts diff --git a/src/blocks/Slider/Slider.scss b/src/blocks/Slider/Slider.scss index 3adb743ad..2ef57c2e1 100644 --- a/src/blocks/Slider/Slider.scss +++ b/src/blocks/Slider/Slider.scss @@ -54,7 +54,7 @@ $block: '.#{$ns}SliderBlock'; } .slick-slide > div { - height: 100%; + display: flex; } } diff --git a/src/blocks/Slider/Slider.tsx b/src/blocks/Slider/Slider.tsx index 64f59da51..c5473ca1e 100644 --- a/src/blocks/Slider/Slider.tsx +++ b/src/blocks/Slider/Slider.tsx @@ -8,6 +8,7 @@ import React, { useState, } from 'react'; +import {useUniqId} from '@gravity-ui/uikit'; import debounce from 'lodash/debounce'; import get from 'lodash/get'; import noop from 'lodash/noop'; @@ -32,12 +33,15 @@ import { import {block} from '../../utils'; import Arrow, {ArrowType} from './Arrow/Arrow'; +import {i18n} from './i18n'; import {SliderBreakpointParams} from './models'; import { getSliderResponsiveParams, getSlidesCountByBreakpoint, getSlidesToShowCount, getSlidesToShowWithDefaults, + isFocusable, + useRovingTabIndex, } from './utils'; import './Slider.scss'; @@ -71,7 +75,7 @@ export const SliderBlock = (props: React.PropsWithChildren) => { anchorId, arrows = true, adaptive, - autoplay = undefined, + autoplay: autoplaySpeed, dots = true, dotsClassName, disclaimer, @@ -87,11 +91,14 @@ export const SliderBlock = (props: React.PropsWithChildren) => { const {isServer} = useContext(SSRContext); const isMobile = useContext(MobileContext); const [breakpoint, setBreakpoint] = useState(BREAKPOINTS.xl); + const sliderId = useUniqId(); const disclosedChildren = useMemo( - () => discloseAllNestedChildren(children as React.ReactElement[]), - [children], + () => discloseAllNestedChildren(children as React.ReactElement[], sliderId), + [children, sliderId], ); const childrenCount = disclosedChildren.length; + const isAutoplayEnabled = autoplaySpeed !== undefined && autoplaySpeed > 0; + const isUserInteractionRef = useRef(false); const [slidesToShow] = useState( getSlidesToShowWithDefaults({ @@ -109,9 +116,17 @@ export const SliderBlock = (props: React.PropsWithChildren) => { const [currentIndex, setCurrentIndex] = useState(0); const [childStyles, setChildStyles] = useState({}); const [slider, setSlider] = useState(); + const prevIndexRef = useRef(0); const autoplayTimeId = useRef(); const {hasFocus, unsetFocus} = useFocus(slider?.innerSlider?.list); + const asUserInteraction = + (fn: (...args: T) => R) => + (...args: T): R => { + isUserInteractionRef.current = true; + return fn(...args); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps const onResize = useCallback( debounce(() => { @@ -135,7 +150,7 @@ export const SliderBlock = (props: React.PropsWithChildren) => { (current: number) => { const lastSlide = childrenCount - slidesToShowCount; - if (autoplay && lastSlide === current) { + if (isAutoplayEnabled && lastSlide === current) { // Slick doesn't support autoplay with no infinity scroll autoplayTimeId.current = setTimeout(() => { if (slider) { @@ -147,10 +162,10 @@ export const SliderBlock = (props: React.PropsWithChildren) => { slider.slickPlay(); } }, 500); - }, autoplay); + }, autoplaySpeed); } }, - [autoplay, childrenCount, slider, slidesToShowCount], + [autoplaySpeed, childrenCount, isAutoplayEnabled, slider, slidesToShowCount], ); useEffect(() => { @@ -169,24 +184,21 @@ export const SliderBlock = (props: React.PropsWithChildren) => { return () => window.removeEventListener('resize', onResize); }, [onResize]); - const handleArrowClick = useCallback( - (direction: ArrowType) => { - let nextIndex; + const handleArrowClick = (direction: ArrowType) => { + let nextIndex; - if (direction === 'right') { - nextIndex = - currentIndex === childrenCount - slidesCountByBreakpoint ? 0 : currentIndex + 1; - } else { - nextIndex = - currentIndex === 0 ? childrenCount - slidesCountByBreakpoint : currentIndex - 1; - } + if (direction === 'right') { + nextIndex = + currentIndex === childrenCount - slidesCountByBreakpoint ? 0 : currentIndex + 1; + } else { + nextIndex = + currentIndex === 0 ? childrenCount - slidesCountByBreakpoint : currentIndex - 1; + } - if (slider) { - slider.slickGoTo(nextIndex); - } - }, - [childrenCount, currentIndex, slider, slidesCountByBreakpoint], - ); + if (slider) { + slider.slickGoTo(nextIndex); + } + }; const onBeforeChange = useCallback( (current: number, next: number) => { @@ -194,6 +206,8 @@ export const SliderBlock = (props: React.PropsWithChildren) => { handleBeforeChange(current, next); } + prevIndexRef.current = current; + setCurrentIndex(Math.ceil(next)); }, [handleBeforeChange], @@ -212,25 +226,46 @@ export const SliderBlock = (props: React.PropsWithChildren) => { if (!hasFocus) { scrollLastSlide(current); } - }, - [handleAfterChange, hasFocus, scrollLastSlide], - ); - - const handleDotClick = useCallback( - (index: number) => { - const nextIndex = index > currentIndex ? index + 1 - slidesCountByBreakpoint : index; - if (slider) { - slider.slickGoTo(nextIndex); + if (isUserInteractionRef.current) { + const focusIndex = + prevIndexRef.current >= current + ? current + : Math.max(current, prevIndexRef.current + slidesCountByBreakpoint); + + const firstNewSlide = document.getElementById(getSlideId(sliderId, focusIndex)); + if (firstNewSlide) { + const focusableChild = Array.from(firstNewSlide.querySelectorAll('*')).find( + isFocusable, + ) as HTMLElement | undefined; + focusableChild?.focus(); + } } + + isUserInteractionRef.current = false; }, - [slider, currentIndex, slidesCountByBreakpoint], + [handleAfterChange, hasFocus, scrollLastSlide, sliderId, slidesCountByBreakpoint], ); - const barSlidesCount = childrenCount - slidesToShowCount + 1; + const handleDotClick = (index: number) => { + const nextIndex = index > currentIndex ? index + 1 - slidesCountByBreakpoint : index; + + if (slider) { + slider.slickGoTo(nextIndex); + } + }; + + const barSlidesCount = childrenCount - slidesCountByBreakpoint + 1; const barPosition = (DOT_GAP + DOT_WIDTH) * currentIndex; const barWidth = DOT_WIDTH + (DOT_GAP + DOT_WIDTH) * (slidesCountByBreakpoint - 1); + const {getRovingItemProps, rovingListProps} = useRovingTabIndex({ + itemCount: barSlidesCount, + activeIndex: currentIndex + 1, + firstIndex: 1, + uniqId: sliderId, + }); + const renderBar = () => { return ( slidesCountByBreakpoint > 1 && ( @@ -240,7 +275,7 @@ export const SliderBlock = (props: React.PropsWithChildren) => { left: barPosition, width: barWidth, }} - > + /> ) ); }; @@ -253,13 +288,18 @@ export const SliderBlock = (props: React.PropsWithChildren) => { {slidesCountByBreakpoint > 0 && (
  • + {...getRovingItemProps(currentIndex + 1)} + /> )} ); @@ -269,10 +309,10 @@ export const SliderBlock = (props: React.PropsWithChildren) => { const currentIndexDiff = index - currentIndex; let currentSlideNumber; - if (0 <= currentIndexDiff && currentIndexDiff < slidesToShowCount) { + if (0 <= currentIndexDiff && currentIndexDiff < slidesCountByBreakpoint) { currentSlideNumber = currentIndex + 1; - } else if (currentIndexDiff >= slidesToShowCount) { - currentSlideNumber = index - slidesToShowCount + 2; + } else if (currentIndexDiff >= slidesCountByBreakpoint) { + currentSlideNumber = index - slidesCountByBreakpoint + 2; } else { currentSlideNumber = index + 1; } @@ -281,22 +321,38 @@ export const SliderBlock = (props: React.PropsWithChildren) => { const isVisibleSlide = (index: number) => { const currentIndexDiff = index - currentIndex; - return ( + const result = slidesCountByBreakpoint > 0 && 0 <= currentIndexDiff && - currentIndexDiff < slidesToShowCount - ); + currentIndexDiff < slidesCountByBreakpoint; + return result; }; const renderDot = (index: number) => { + const isVisible = isVisibleSlide(index); + const currentSlideNumber = getCurrentSlideNumber(index); + const rovingItemProps = isVisible ? undefined : getRovingItemProps(currentSlideNumber); return (
  • handleDotClick(index)} - aria-hidden={isVisibleSlide(index) ? true : undefined} - aria-label={`Slide ${getCurrentSlideNumber(index)} of ${barSlidesCount}`} - >
    • + onClick={asUserInteraction(() => handleDotClick(index))} + onKeyDown={(e) => { + const key = e.key.toLowerCase(); + if (key === 'space' || key === 'enter') { + e.currentTarget.click(); + } + }} + role="menuitemradio" + aria-checked={false} + tabIndex={-1} + aria-hidden={isVisible} + aria-label={i18n('dot-label', { + index: currentSlideNumber, + count: barSlidesCount, + })} + {...rovingItemProps} + /> ); }; @@ -311,7 +367,12 @@ export const SliderBlock = (props: React.PropsWithChildren) => { return (
    -
      +
        {renderBar()} {dotsList}
      @@ -338,17 +399,30 @@ export const SliderBlock = (props: React.PropsWithChildren) => { infinite: false, speed: 1000, adaptiveHeight: adaptive, - autoplay: Boolean(autoplay), - autoplaySpeed: autoplay, + autoplay: isAutoplayEnabled, + autoplaySpeed, slidesToShow: slidesToShowCount, slidesToScroll: 1, responsive: getSliderResponsiveParams(slidesToShow), beforeChange: onBeforeChange, afterChange: onAfterChange, initialSlide: 0, - nextArrow: , - prevArrow: , + nextArrow: ( + + ), + prevArrow: ( + + ), lazyLoad, + accessibility: false, }; return ( @@ -390,29 +464,49 @@ export const SliderBlock = (props: React.PropsWithChildren) => { ); }; +function getSlideId(sliderId: string, index: number) { + return `slider-${sliderId}-child-${index}`; +} + // TODO remove this and rework PriceDetailed CLOUDFRONT-12230 -function discloseAllNestedChildren(children: React.ReactElement[]): React.ReactElement[] { +function discloseAllNestedChildren( + children: React.ReactElement[], + sliderId: string, +): React.ReactElement[] { if (!children) { return []; } + let childIndex = 0; + const wrapped = (child: React.ReactElement) => { + const id = getSlideId(sliderId, childIndex++); + + return ( +
      + {child} +
      + ); + }; + return React.Children.map(children, (child) => { if (child) { // TODO: if child has 'items' then 'items' determinate like nested children for Slider. const nestedChildren = child.props.data?.items; if (nestedChildren) { - return nestedChildren.map((nestedChild: React.ReactElement) => - React.cloneElement(child, { - data: { - ...child.props.data, - items: [nestedChild], - }, - }), - ); + return nestedChildren.map((nestedChild: React.ReactElement) => { + return wrapped( + React.cloneElement(child, { + data: { + ...child.props.data, + items: [nestedChild], + }, + }), + ); + }); } } - return child; + return child && wrapped(child); }).filter(Boolean); } diff --git a/src/blocks/Slider/__tests__/Slider.test.tsx b/src/blocks/Slider/__tests__/Slider.test.tsx index 9c0a3e7d5..043dd9dee 100644 --- a/src/blocks/Slider/__tests__/Slider.test.tsx +++ b/src/blocks/Slider/__tests__/Slider.test.tsx @@ -44,9 +44,9 @@ describe('Slider', () => { // There we have a bar covering `slidesToShow` dots // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access const accessibleBarElement = container.querySelector('.pc-SliderBlock__accessible-bar'); - expect(accessibleBarElement?.getAttribute('aria-label')).toBe(`Slide 1 of ${barDotsCount}`); + expect(accessibleBarElement?.getAttribute('aria-label')).toBe(`Page 1 of ${barDotsCount}`); expect( - queryHelpers.queryAllByAttribute('aria-label', container, `Slide 1 of ${barDotsCount}`), + queryHelpers.queryAllByAttribute('aria-label', container, `Page 1 of ${barDotsCount}`), ).toHaveLength(slidesToShow + 1); // Checking labels for the slides starting from 2 @@ -57,7 +57,7 @@ describe('Slider', () => { queryHelpers.queryAllByAttribute( 'aria-label', container, - `Slide ${index + 2} of ${barDotsCount}`, + `Page ${index + 2} of ${barDotsCount}`, ), ).toHaveLength(1); }); diff --git a/src/blocks/Slider/i18n/en.json b/src/blocks/Slider/i18n/en.json index 976c4a199..c72f11a9e 100644 --- a/src/blocks/Slider/i18n/en.json +++ b/src/blocks/Slider/i18n/en.json @@ -1,4 +1,6 @@ { "arrow-right": "Next", - "arrow-left": "Previous" + "arrow-left": "Previous", + "dot-label": "Page {{index}} of {{count}}", + "pagination-label": "Pages" } diff --git a/src/blocks/Slider/i18n/ru.json b/src/blocks/Slider/i18n/ru.json index 95873004f..fb16cd7bc 100644 --- a/src/blocks/Slider/i18n/ru.json +++ b/src/blocks/Slider/i18n/ru.json @@ -1,4 +1,6 @@ { "arrow-right": "Дальше", - "arrow-left": "Назад" + "arrow-left": "Назад", + "dot-label": "Страница {{index}} из {{count}}", + "pagination-label": "Страницы" } diff --git a/src/blocks/Slider/utils.ts b/src/blocks/Slider/utils.ts index 5b39cffc0..23c0ec8e6 100644 --- a/src/blocks/Slider/utils.ts +++ b/src/blocks/Slider/utils.ts @@ -1,3 +1,5 @@ +import {useEffect, useRef, useState} from 'react'; + import pickBy from 'lodash/pickBy'; import {BREAKPOINTS} from '../../constants'; @@ -20,6 +22,45 @@ export interface GetSlidesToShowParams { breakpoints?: SlidesToShow; mobileFullscreen?: boolean; } + +export const isFocusable = (element: Element): boolean => { + if (!(element instanceof HTMLElement)) { + return false; + } + const tabIndexAttr = element.getAttribute('tabindex'); + const hasTabIndex = tabIndexAttr !== null; + const tabIndex = Number(tabIndexAttr); + if (element.ariaHidden === 'true' || (hasTabIndex && tabIndex < 0)) { + return false; + } + if (hasTabIndex && tabIndex >= 0) { + return true; + } + + // without this jest fails here for some reason + let htmlElement: + | HTMLAnchorElement + | HTMLInputElement + | HTMLSelectElement + | HTMLTextAreaElement + | HTMLButtonElement; + switch (true) { + case element instanceof HTMLAnchorElement: + htmlElement = element as HTMLAnchorElement; + return Boolean(htmlElement.href); + case element instanceof HTMLInputElement: + htmlElement = element as HTMLInputElement; + return htmlElement.type !== 'hidden' && !htmlElement.disabled; + case element instanceof HTMLSelectElement: + case element instanceof HTMLTextAreaElement: + case element instanceof HTMLButtonElement: + htmlElement = element as HTMLSelectElement | HTMLTextAreaElement | HTMLButtonElement; + return !htmlElement.disabled; + default: + return false; + } +}; + export function getSlidesToShowWithDefaults({ contentLength, breakpoints, @@ -62,3 +103,72 @@ export function getSlidesCountByBreakpoint( export function getSlidesToShowCount(breakpoints: SliderBreakpointParams) { return Math.floor(Math.max(...Object.values(breakpoints))); } + +const getRovingListItemId = (uniqId: string, index: number) => + `${uniqId}-roving-tabindex-item-${index}`; +export function useRovingTabIndex(props: { + itemCount: number; + activeIndex: number; + firstIndex?: number; + uniqId: string; +}) { + const {itemCount, activeIndex, firstIndex = 0, uniqId} = props; + const [currentIndex, setCurrentIndex] = useState(firstIndex); + const hasFocusRef = useRef(false); + const lastIndex = itemCount + firstIndex - 1; + + const getRovingItemProps = ( + index: number, + ): Pick, 'id' | 'tabIndex' | 'onFocus'> => { + return { + id: getRovingListItemId(uniqId, index), + tabIndex: index === activeIndex ? 0 : -1, + onFocus: () => { + setCurrentIndex(index); + hasFocusRef.current = true; + }, + }; + }; + + useEffect(() => { + if (!hasFocusRef.current) { + return; + } + document.getElementById(getRovingListItemId(uniqId, currentIndex))?.focus(); + }, [activeIndex, currentIndex, uniqId]); + + const setNextIndex = () => + setCurrentIndex((prev) => (prev >= lastIndex ? firstIndex : prev + 1)); + const setPrevIndex = () => + setCurrentIndex((prev) => (prev <= firstIndex ? lastIndex : prev - 1)); + + const onRovingListKeyDown: React.KeyboardEventHandler = (e) => { + const key = e.key.toLowerCase(); + + if (key !== 'tab' && key !== 'enter') { + e.preventDefault(); + } + + switch (key) { + case 'arrowleft': + case 'arrowup': + setPrevIndex(); + return; + case 'arrowright': + case 'arrowdown': + setNextIndex(); + return; + } + }; + + const onRovingListBlur: React.FocusEventHandler = () => { + hasFocusRef.current = false; + }; + + const rovingListProps: React.HTMLAttributes = { + onKeyDown: onRovingListKeyDown, + onBlur: onRovingListBlur, + }; + + return {getRovingItemProps, rovingListProps}; +} diff --git a/src/blocks/SliderNew/Arrow/Arrow.tsx b/src/blocks/SliderNew/Arrow/Arrow.tsx index 3993dadb3..6d73586ee 100644 --- a/src/blocks/SliderNew/Arrow/Arrow.tsx +++ b/src/blocks/SliderNew/Arrow/Arrow.tsx @@ -15,11 +15,17 @@ export interface ArrowProps { type: ArrowType; onClick?: () => void; size?: number; + extraProps?: React.ButtonHTMLAttributes; } -const Arrow = ({type, onClick, className, size = 16}: ArrowProps & ClassNameProps) => ( +const Arrow = ({type, onClick, className, size = 16, extraProps}: ArrowProps & ClassNameProps) => (
      -