Merge pull request #1035 from lumapps/fix/DSW-48-lightbox-a11y

fix(lightbox): fix dialog accessibility
lumapps · Dec 19, 2023 · cdd0e17 · cdd0e17
2 parents 8150af1 + 817a718
commit cdd0e17
Show file tree

Hide file tree

Showing 7 changed files with 125 additions and 58 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,8 @@ 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/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/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: <ImageBlock align={Alignment.center} alt="" fillHeight image={LANDSCAPE_IMAGES.landscape1} />,
+        'aria-label': 'Fullscreen image',
+        children: (
+            <ImageBlock
+                align={Alignment.center}
+                fillHeight
+                image={LANDSCAPE_IMAGES.landscape1}
+                alt={LANDSCAPE_IMAGES_ALT.landscape1}
+            />
+        ),
     },
 };
 
@@ -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: (
+            <Slideshow
+                aria-label="Image slideshow"
+                theme="dark"
+                slideshowControlsProps={{
+                    nextButtonProps: { label: 'Next image' },
+                    previousButtonProps: { label: 'Previous image' },
+                }}
+                slideGroupLabel={(currentGroup, totalGroup) => `${currentGroup} of ${totalGroup}`}
+            >
+                <SlideshowItem>
+                    <ImageBlock
+                        align="center"
+                        fillHeight
+                        image={LANDSCAPE_IMAGES.landscape1}
+                        alt={LANDSCAPE_IMAGES_ALT.landscape1}
+                    />
+                </SlideshowItem>
+
+                <SlideshowItem>
+                    <ImageBlock
+                        align="center"
+                        fillHeight
+                        image={LANDSCAPE_IMAGES.landscape2}
+                        alt={LANDSCAPE_IMAGES_ALT.landscape2}
+                    />
+                </SlideshowItem>
+
+                <SlideshowItem>
+                    <ImageBlock
+                        align="center"
+                        fillHeight
+                        image={LANDSCAPE_IMAGES.landscape3}
+                        alt={LANDSCAPE_IMAGES_ALT.landscape3}
+                    />
+                </SlideshowItem>
+            </Slideshow>
+        ),
+    },
+};
diff --git 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,14 +19,16 @@ 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<AriaAttributes, 'aria-label' | 'aria-labelledby'> {
     /** Props to pass to the close button (minus those already set by the Lightbox props). */
     closeButtonProps?: Pick<IconButtonProps, 'label'> &
         Omit<IconButtonProps, 'label' | 'onClick' | 'icon' | 'emphasis' | 'color'>;
     /** Whether the component is open or not. */
     isOpen?: boolean;
     /** Reference to the element that triggered modal opening to set focus on. */
     parentElement: RefObject<any>;
+    /** 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<HTMLElement>;
     /** 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<LightboxProps, HTMLDivElement> = 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<LightboxProps, HTMLDivElement> = forwardRef((props,
     const childrenRef = useRef<any>(null);
     // eslint-disable-next-line react-hooks/rules-of-hooks
     const wrapperRef = useRef<HTMLDivElement>(null);
+    // eslint-disable-next-line react-hooks/rules-of-hooks
+    const closeButtonRef = useRef<HTMLButtonElement>(null);
 
     // eslint-disable-next-line react-hooks/rules-of-hooks
     useDisableBodyScroll(isOpen && wrapperRef.current);
@@ -84,7 +92,12 @@ export const Lightbox: Comp<LightboxProps, HTMLDivElement> = 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<LightboxProps, HTMLDivElement> = 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<LightboxProps, HTMLDivElement> = forwardRef((props,
             {closeButtonProps && (
                 <IconButton
                     {...closeButtonProps}
+                    ref={closeButtonRef}
                     className={`${CLASSNAME}__close`}
                     color={ColorPalette.light}
                     emphasis={Emphasis.low}

diff --git a/packages/lumx-react/src/stories/controls/image.ts b/packages/lumx-react/src/stories/controls/image.ts
@@ -22,6 +22,12 @@ export const SQUARE_IMAGES = { square1, square2 };
 export const SVG_IMAGES = { defaultSvg };
 export const EMPTY_IMAGES = { emptyImage };
 export const LANDSCAPE_IMAGES = { landscape1, landscape1s200, landscape2, landscape3 };
+export const LANDSCAPE_IMAGES_ALT: { [key in keyof typeof LANDSCAPE_IMAGES]: string } = {
+    landscape1: 'A majestic snowy mountain range with a peak covered in glistening snow',
+    landscape1s200: 'A majestic snowy mountain range with a peak covered in glistening snow',
+    landscape2: 'A colorful rack of shirts displaying various hues and styles',
+    landscape3: 'An open book resting on a table, ready to be explored and read',
+};
 export const PORTRAIT_IMAGES = { portrait1, portrait1s200, portrait2, portrait3 };
 export const IMAGES = {
     ...LANDSCAPE_IMAGES,

diff --git a/packages/site-demo/content/product/components/lightbox/index.mdx b/packages/site-demo/content/product/components/lightbox/index.mdx
@@ -4,6 +4,18 @@
 
 <DemoBlock demo="default" />
 
+### 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
 
 <PropTable component="Lightbox" />
diff --git 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"