From 500dc456236c175fb5376dbabfaa21c8d56187d8 Mon Sep 17 00:00:00 2001 From: gcornut Date: Fri, 15 Dec 2023 10:16:29 +0100 Subject: [PATCH 1/3] docs(lightbox): add accessibility concerns section --- CHANGELOG.md | 1 + .../content/product/components/lightbox/index.mdx | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72d5c548a..35bf20746 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Tooltip: fixed tooltip closing when mouse is hovering the tooltip text. - Tooltip: fixed close on Escape key pressed (not only when anchor is focused). +- Lightbox: document accessibility concerns. ## [3.6.0][] - 2023-12-05 diff --git a/packages/site-demo/content/product/components/lightbox/index.mdx b/packages/site-demo/content/product/components/lightbox/index.mdx index 09dc4c30b..7527a93ba 100644 --- a/packages/site-demo/content/product/components/lightbox/index.mdx +++ b/packages/site-demo/content/product/components/lightbox/index.mdx @@ -4,6 +4,18 @@ +### Accessibility concerns + +This component uses the `dialog` accessible pattern, a few specific behaviors are applied: + +- When opening, focus will be set either on the provided `focusElement` prop or on the close button (if defined) or on the dialog itself. +Once the focus is set within, it will be trapped and stay within the lightbox until it is closed. + +- A label must be defined using either `aria-label` or `aria-labelledby` props. +Setting `aria-labelledby` is recommended as it target a visible label rather than adding an invisible `aria-label` on the dialog. + +- When closing, the activating element should take focus. Make sure to fill the `parentElement` prop to make this work properly. + ### Properties From 9bc282e4cba98854dc895f2f326a29c9152ab7f1 Mon Sep 17 00:00:00 2001 From: gcornut Date: Fri, 15 Dec 2023 11:50:24 +0100 Subject: [PATCH 2/3] fix(lightbox): fix dialog accessibility --- CHANGELOG.md | 1 + .../components/lightbox/Lightbox.stories.tsx | 66 +++++++++++++++++-- .../src/components/lightbox/Lightbox.tsx | 25 +++++-- .../lumx-react/src/stories/controls/image.ts | 6 ++ 4 files changed, 89 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35bf20746..b715c02d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Tooltip: fixed tooltip closing when mouse is hovering the tooltip text. - Tooltip: fixed close on Escape key pressed (not only when anchor is focused). +- Lightbox: fixed aria dialog accessibility (reworked role, labelling and default focus element). - Lightbox: document accessibility concerns. ## [3.6.0][] - 2023-12-05 diff --git a/packages/lumx-react/src/components/lightbox/Lightbox.stories.tsx b/packages/lumx-react/src/components/lightbox/Lightbox.stories.tsx index 4bc5b9235..4c6e3be29 100644 --- a/packages/lumx-react/src/components/lightbox/Lightbox.stories.tsx +++ b/packages/lumx-react/src/components/lightbox/Lightbox.stories.tsx @@ -1,8 +1,8 @@ /* eslint-disable react-hooks/rules-of-hooks,react/display-name */ import React from 'react'; -import { ImageBlock, Alignment, Lightbox, Button } from '@lumx/react'; +import { ImageBlock, Alignment, Lightbox, Button, Slideshow, SlideshowItem } from '@lumx/react'; import { useBooleanState } from '@lumx/react/hooks/useBooleanState'; -import { LANDSCAPE_IMAGES } from '@lumx/react/stories/controls/image'; +import { LANDSCAPE_IMAGES, LANDSCAPE_IMAGES_ALT } from '@lumx/react/stories/controls/image'; export default { title: 'LumX components/lightbox/Lightbox', @@ -28,9 +28,17 @@ export default { /** * Base LightBox with image block */ -export const ImageBlock_ = { +export const Image = { args: { - children: , + 'aria-label': 'Fullscreen image', + children: ( + + ), }, }; @@ -39,7 +47,55 @@ export const ImageBlock_ = { */ export const WithCloseButton = { args: { - ...ImageBlock_.args, + ...Image.args, closeButtonProps: { label: 'Close' }, }, }; + +/** + * Demo a LightBox containing an image slideshow + */ +export const ImageSlideshow = { + args: { + 'aria-label': 'Fullscreen image slideshow', + closeButtonProps: { label: 'Close' }, + children: ( + `${currentGroup} of ${totalGroup}`} + > + + + + + + + + + + + + + ), + }, +}; diff --git a/packages/lumx-react/src/components/lightbox/Lightbox.tsx b/packages/lumx-react/src/components/lightbox/Lightbox.tsx index 5c32dbd18..b16de5ff0 100644 --- a/packages/lumx-react/src/components/lightbox/Lightbox.tsx +++ b/packages/lumx-react/src/components/lightbox/Lightbox.tsx @@ -1,4 +1,4 @@ -import React, { forwardRef, RefObject, useRef, useEffect } from 'react'; +import React, { forwardRef, RefObject, useRef, useEffect, AriaAttributes } from 'react'; import classNames from 'classnames'; import { createPortal } from 'react-dom'; @@ -19,7 +19,7 @@ import { useTransitionVisibility } from '@lumx/react/hooks/useTransitionVisibili /** * Defines the props of the component. */ -export interface LightboxProps extends GenericProps, HasTheme { +export interface LightboxProps extends GenericProps, HasTheme, Pick { /** Props to pass to the close button (minus those already set by the Lightbox props). */ closeButtonProps?: Pick & Omit; @@ -27,6 +27,8 @@ export interface LightboxProps extends GenericProps, HasTheme { isOpen?: boolean; /** Reference to the element that triggered modal opening to set focus on. */ parentElement: RefObject; + /** Reference to the element that should get the focus when the lightbox opens. By default, the close button or the lightbox itself will take focus. */ + focusElement?: RefObject; /** Whether to keep the dialog open on clickaway or escape press. */ preventAutoClose?: boolean; /** Z-axis position. */ @@ -54,13 +56,17 @@ const CLASSNAME = getRootClassName(COMPONENT_NAME); */ export const Lightbox: Comp = forwardRef((props, ref) => { const { - ariaLabel, + 'aria-labelledby': propAriaLabelledBy, + ariaLabelledBy = propAriaLabelledBy, + 'aria-label': propAriaLabel, + ariaLabel = propAriaLabel, children, className, closeButtonProps, isOpen, onClose, parentElement, + focusElement, preventAutoClose, theme, zIndex, @@ -75,6 +81,8 @@ export const Lightbox: Comp = forwardRef((props, const childrenRef = useRef(null); // eslint-disable-next-line react-hooks/rules-of-hooks const wrapperRef = useRef(null); + // eslint-disable-next-line react-hooks/rules-of-hooks + const closeButtonRef = useRef(null); // eslint-disable-next-line react-hooks/rules-of-hooks useDisableBodyScroll(isOpen && wrapperRef.current); @@ -84,7 +92,12 @@ export const Lightbox: Comp = forwardRef((props, // Handle focus trap. // eslint-disable-next-line react-hooks/rules-of-hooks - useFocusTrap(isOpen && wrapperRef.current, childrenRef.current?.firstChild); + useFocusTrap( + // Focus trap zone + isOpen && wrapperRef.current, + // Focus element (fallback on close button and then on the dialog) + focusElement?.current || closeButtonRef.current || wrapperRef.current, + ); // eslint-disable-next-line react-hooks/rules-of-hooks const previousOpen = useRef(isOpen); @@ -116,7 +129,10 @@ export const Lightbox: Comp = forwardRef((props, ref={mergeRefs(ref, wrapperRef)} {...forwardedProps} aria-label={ariaLabel} + aria-labelledby={ariaLabelledBy} aria-modal="true" + role="dialog" + tabIndex={-1} className={classNames( className, handleBasicClasses({ @@ -131,6 +147,7 @@ export const Lightbox: Comp = forwardRef((props, {closeButtonProps && ( Date: Tue, 19 Dec 2023 13:29:34 +0100 Subject: [PATCH 3/3] chore(dependencies): update react types --- packages/lumx-react/package.json | 6 +-- yarn.lock | 66 ++++++++++---------------------- 2 files changed, 23 insertions(+), 49 deletions(-) diff --git a/packages/lumx-react/package.json b/packages/lumx-react/package.json index 56b5b09cb..80e01c39d 100644 --- a/packages/lumx-react/package.json +++ b/packages/lumx-react/package.json @@ -44,9 +44,9 @@ "@types/classnames": "^2.2.9", "@types/jest": "^29.2.1", "@types/lodash": "^4.14.149", - "@types/react": "^16.9.11", - "@types/react-dom": "^16.9.4", - "@types/react-is": "^16.7.1", + "@types/react": "^17.0.2", + "@types/react-dom": "^17.0.2", + "@types/react-is": "^17.0.2", "autoprefixer": "^9.7.4", "babel-jest": "29.1.2", "babel-loader": "^8.0.6", diff --git a/yarn.lock b/yarn.lock index 27205d078..6b58c2f1a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6229,9 +6229,9 @@ __metadata: "@types/classnames": ^2.2.9 "@types/jest": ^29.2.1 "@types/lodash": ^4.14.149 - "@types/react": ^16.9.11 - "@types/react-dom": ^16.9.4 - "@types/react-is": ^16.7.1 + "@types/react": ^17.0.2 + "@types/react-dom": ^17.0.2 + "@types/react-is": ^17.0.2 autoprefixer: ^9.7.4 babel-jest: 29.1.2 babel-loader: ^8.0.6 @@ -8501,21 +8501,12 @@ __metadata: languageName: node linkType: hard -"@types/react-dom@npm:<18.0.0": - version: 17.0.18 - resolution: "@types/react-dom@npm:17.0.18" +"@types/react-dom@npm:<18.0.0, @types/react-dom@npm:^17.0.2": + version: 17.0.25 + resolution: "@types/react-dom@npm:17.0.25" dependencies: "@types/react": ^17 - checksum: b74525b1a13a0e27fe20859ff7a7e8f7e4581fb9d45ed1b6447ad1534d86f813818353c39d0df2e28f9d2b9be2e3af1908c244b2214a979393d19f217665e614 - languageName: node - linkType: hard - -"@types/react-dom@npm:^16.9.4": - version: 16.9.4 - resolution: "@types/react-dom@npm:16.9.4" - dependencies: - "@types/react": "*" - checksum: 5261c5aeaa86a39a9bcf5997a391c9fb38a00dc4cb64fb025dfc2ec7664ce17621f8f5e79f612734ad7948972b86b096113edff64ecd5b4090d68c773f399239 + checksum: d1e582682478e0848c8d54ea3e89d02047bac6d916266b85ce63731b06987575919653ea7159d98fda47ade3362b8c4d5796831549564b83088e7aa9ce8b60ed languageName: node linkType: hard @@ -8528,44 +8519,34 @@ __metadata: languageName: node linkType: hard -"@types/react-is@npm:^16.7.1": - version: 16.7.1 - resolution: "@types/react-is@npm:16.7.1" +"@types/react-is@npm:^17.0.2": + version: 17.0.7 + resolution: "@types/react-is@npm:17.0.7" dependencies: - "@types/react": "*" - checksum: 064b0bc6aaf98a87bd8c56f17407e429e675c06ca5fa523102b3f5fc4c53eac08fd27a493605f52e26891fae2dd65e03a475ef729d4ad5b9ae8104b187b3c4a9 - languageName: node - linkType: hard - -"@types/react@npm:*, @types/react@npm:^16.9.11": - version: 16.9.11 - resolution: "@types/react@npm:16.9.11" - dependencies: - "@types/prop-types": "*" - csstype: ^2.2.0 - checksum: 6e4b66a31d9b728f0da6ea64a733dd7403c695c42cd7ff8ca6474dbacc0c314d28c3ef05d8b091f9ffa9090516bca0ddcea5751ae9dbef268d225abe2956b28f + "@types/react": ^17 + checksum: a8f11067795dbcf54a54d5fdc1977816be155fd04051e850f7c85dbbad83897f846dd3e474d56bd12a7055e0ae1825185f41c6f56342fd5cd31a08df3b3fbfff languageName: node linkType: hard -"@types/react@npm:>=16": - version: 18.2.9 - resolution: "@types/react@npm:18.2.9" +"@types/react@npm:*, @types/react@npm:>=16": + version: 18.2.45 + resolution: "@types/react@npm:18.2.45" dependencies: "@types/prop-types": "*" "@types/scheduler": "*" csstype: ^3.0.2 - checksum: f155256171a2d701eb962a1d3aa2a1c9ee36d9dd4a4aecb911d29e50717aab1a76914aef25242665147c455b9e8d081d1a60275d13ca81075c148ebd6607414a + checksum: 40b256bdce67b026348022b4f8616a693afdad88cf493b77f7b4e6c5f4b0e4ba13a6068e690b9b94572920840ff30d501ea3d8518e1f21cc8fb8204d4b140c8a languageName: node linkType: hard -"@types/react@npm:^17": - version: 17.0.52 - resolution: "@types/react@npm:17.0.52" +"@types/react@npm:^17, @types/react@npm:^17.0.2": + version: 17.0.73 + resolution: "@types/react@npm:17.0.73" dependencies: "@types/prop-types": "*" "@types/scheduler": "*" csstype: ^3.0.2 - checksum: a51b98dd87838d161278fdf9dd78e6a4ff8c018f406d6647f77963e144fb52a8beee40c89fd0e7e840eaeaa8bd9fe2f34519410540b1a52d43a6f8b4d2fbce33 + checksum: 08107645acdd734c8ddb4d26f1b43dfa0d75f7a8d268eaacb897337e103eaa620fe8c3c6972dab9860aaa47bbee1da587cf06b11bb4e655588e38485daf48a6c languageName: node linkType: hard @@ -14166,13 +14147,6 @@ __metadata: languageName: node linkType: hard -"csstype@npm:^2.2.0": - version: 2.6.7 - resolution: "csstype@npm:2.6.7" - checksum: 4495eba98af6cd9d9c61b4bf8aa1ac457d98455effec77f58f1a88842457cb0ef740152bbaa5d47e05b0b7a41dae64ef978e0e1a17065241b4e5914707e801eb - languageName: node - linkType: hard - "csstype@npm:^3.0.2": version: 3.1.1 resolution: "csstype@npm:3.1.1"