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

feat(slideshow): rework the keyboard and focus accessibility #1202

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- `SlideShow`: use `inert` on hidden slides (on top of `aria-hidden` and `tabindex="-1"`) for greater semantic.
- `SlideShow`: reworked keyboard & focus management
- Pagination items do not use roving tabindex anymore
- Roles `tab` & `tabpanel` are not used anymore
- Switching slides moves the focus
- On the slide if it contains multiple focusable element
- On the focusable element if the slide contains exactly one focusable element
- On the pagination item otherwise

## [3.9.3][] - 2024-10-09

### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const ImageSlideshow: React.FC<ImageSlideshowProps> = ({
activeIndex,
slideshowId,
setSlideshow,
slideshow,
slideshowSlidesId,
slidesCount,
onNextClick,
Expand Down Expand Up @@ -61,6 +62,7 @@ export const ImageSlideshow: React.FC<ImageSlideshowProps> = ({
onNextClick={onNextClick}
onPreviousClick={onPreviousClick}
onPaginationClick={onPaginationClick}
parentRef={slideshow}
{...slideshowControlsProps}
paginationItemProps={(index: number) => {
const props = slideshowControlsProps?.paginationItemProps?.(index) || {};
Expand Down
22 changes: 15 additions & 7 deletions packages/lumx-react/src/components/slideshow/Slides.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ import chunk from 'lodash/chunk';

import classNames from 'classnames';

import { FULL_WIDTH_PERCENT } from '@lumx/react/components/slideshow/constants';
import { FULL_WIDTH_PERCENT, NEXT_SLIDE_EVENT, PREV_SLIDE_EVENT } from '@lumx/react/components/slideshow/constants';
import { Comp, GenericProps, HasTheme } from '@lumx/react/utils/type';
import { getRootClassName, handleBasicClasses } from '@lumx/react/utils/className';
import { useMergeRefs } from '@lumx/react/utils/mergeRefs';
import { useKeyNavigate } from '@lumx/react/components/slideshow/useKeyNavigate';
import { buildSlideShowGroupId, SlideshowItemGroup } from './SlideshowItemGroup';

export interface SlidesProps extends GenericProps, HasTheme {
Expand All @@ -32,7 +34,7 @@ export interface SlidesProps extends GenericProps, HasTheme {
/**
* Accessible label to set on a slide group.
* Receives the group position starting from 1 and the total number of groups.
* */
*/
slideGroupLabel?: (groupPosition: number, groupTotal: number) => string;
}

Expand Down Expand Up @@ -70,7 +72,6 @@ export const Slides: Comp<SlidesProps, HTMLDivElement> = forwardRef((props, ref)
slideGroupLabel,
...forwardedProps
} = props;
const wrapperRef = React.useRef<HTMLDivElement>(null);
const startIndexVisible = activeIndex;
const endIndexVisible = startIndexVisible + 1;

Expand All @@ -82,10 +83,17 @@ export const Slides: Comp<SlidesProps, HTMLDivElement> = forwardRef((props, ref)
return groupBy && groupBy > 1 ? chunk(childrenArray, groupBy) : childrenArray;
}, [children, groupBy]);

const slidesRef = React.useRef<HTMLDivElement>(null);

const slide = slidesRef.current;
const onNextSlide = React.useCallback(() => slide?.dispatchEvent(new CustomEvent(NEXT_SLIDE_EVENT)), [slide]);
const onPrevSlide = React.useCallback(() => slide?.dispatchEvent(new CustomEvent(PREV_SLIDE_EVENT)), [slide]);
useKeyNavigate(slide, onNextSlide, onPrevSlide);

return (
<section
id={id}
ref={ref}
ref={useMergeRefs(slidesRef, ref)}
{...forwardedProps}
className={classNames(className, handleBasicClasses({ prefix: CLASSNAME, theme }), {
[`${CLASSNAME}--fill-height`]: fillHeight,
Expand All @@ -100,14 +108,14 @@ export const Slides: Comp<SlidesProps, HTMLDivElement> = forwardRef((props, ref)
onMouseLeave={toggleAutoPlay}
aria-live={isAutoPlaying ? 'off' : 'polite'}
>
<div ref={wrapperRef} className={`${CLASSNAME}__wrapper`} style={wrapperStyle}>
<div className={`${CLASSNAME}__wrapper`} style={wrapperStyle}>
{groups.map((group, index) => (
<SlideshowItemGroup
key={index}
id={slidesId && buildSlideShowGroupId(slidesId, index)}
role={hasControls ? 'tabpanel' : 'group'}
label={slideGroupLabel ? slideGroupLabel(index + 1, groups.length) : undefined}
label={slideGroupLabel?.(index + 1, groups.length)}
isDisplayed={index >= startIndexVisible && index < endIndexVisible}
slidesRef={slidesRef}
>
{group}
</SlideshowItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable jsx-a11y/anchor-is-valid */
import React from 'react';
import range from 'lodash/range';
import { AspectRatio, Button, FlexBox, ImageBlock, Slideshow, SlideshowItem, Orientation } from '@lumx/react';
import { AspectRatio, Button, FlexBox, ImageBlock, Slideshow, SlideshowItem, Orientation, Link } from '@lumx/react';
import { IMAGES, LANDSCAPE_IMAGES } from '@lumx/react/stories/controls/image';

export default {
Expand Down Expand Up @@ -65,6 +66,16 @@ export const ResponsiveSlideShowSwipe = () => {
}}
slideGroupLabel={(currentGroup, totalGroup) => `${currentGroup} of ${totalGroup}`}
>
<SlideshowItem>
<FlexBox
style={{ border: '1px solid grey', maxWidth: 300, height: 300 }}
hAlign="center"
vAlign="center"
>
<Link href="#">A link</Link>
<Button>A button</Button>
</FlexBox>
</SlideshowItem>
{slides.map((slide) => (
<SlideshowItem key={`${slide}`}>
<FlexBox
Expand Down
18 changes: 2 additions & 16 deletions packages/lumx-react/src/components/slideshow/Slideshow.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import React, { forwardRef } from 'react';

import { SlideshowControls, SlideshowControlsProps, Theme, Slides, SlidesProps } from '@lumx/react';
import { DEFAULT_OPTIONS } from '@lumx/react/hooks/useSlideshowControls';
import { Comp, GenericProps } from '@lumx/react/utils/type';
import { useFocusWithin } from '@lumx/react/hooks/useFocusWithin';
import { mergeRefs } from '@lumx/react/utils/mergeRefs';
import { buildSlideShowGroupId } from './SlideshowItemGroup';
import { DEFAULT_OPTIONS } from './useSlideshowControls';

/**
* Defines the props of the component.
Expand All @@ -14,7 +13,7 @@ export interface SlideshowProps
extends GenericProps,
Pick<SlidesProps, 'autoPlay' | 'slidesId' | 'id' | 'theme' | 'fillHeight' | 'groupBy' | 'slideGroupLabel'> {
/** current slide active */
activeIndex?: SlidesProps['activeIndex'];
activeIndex?: number;
/** Interval between each slide when automatic rotation is enabled. */
interval?: number;
/** Props to pass to the slideshow controls (minus those already set by the Slideshow props). */
Expand Down Expand Up @@ -134,27 +133,14 @@ export const Slideshow: Comp<SlideshowProps, HTMLDivElement> = forwardRef((props
parentRef={slideshow}
theme={theme}
isAutoPlaying={isAutoPlaying}
nextButtonProps={{
'aria-controls': slideshowSlidesId,
...slideshowControlsProps.nextButtonProps,
}}
previousButtonProps={{
'aria-controls': slideshowSlidesId,
...slideshowControlsProps.previousButtonProps,
}}
playButtonProps={
autoPlay
? {
'aria-controls': slideshowSlidesId,
onClick: toggleForcePause,
...slideshowControlsProps.playButtonProps,
}
: undefined
}
paginationItemProps={(index) => ({
'aria-controls': buildSlideShowGroupId(slideshowSlidesId, index),
...slideshowControlsProps.paginationItemProps?.(index),
})}
/>
</div>
) : undefined
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,10 @@ export const ControllingSlideshow = ({ images = Object.values(LANDSCAPE_IMAGES),
parentRef={slideshow}
theme={theme}
isAutoPlaying={isAutoPlaying}
nextButtonProps={{ label: 'Next', 'aria-controls': slideshowSlidesId }}
previousButtonProps={{ label: 'Previous', 'aria-controls': slideshowSlidesId }}
nextButtonProps={{ label: 'Next' }}
previousButtonProps={{ label: 'Previous' }}
playButtonProps={{
label: 'Play/Pause',
'aria-controls': slideshowSlidesId,
onClick: toggleForcePause,
}}
paginationItemLabel={(index) => `Slide ${index}`}
Expand Down
Loading
Loading