From 0d3dd3ed6866d8432ac4472bb759525224119c49 Mon Sep 17 00:00:00 2001 From: Parth Gartan Date: Sun, 18 Oct 2026 15:34:01 +0000 Subject: [PATCH 1/3] fix(Modal): dynamically wire unique aria-labelledby and enforce accessible name contract (#1864) Signed-off-by: Parth Gartan --- src/custom/Modal/index.tsx | 131 +++++++++++++++++++++++++++++++------ 1 file changed, 111 insertions(+), 20 deletions(-) diff --git a/src/custom/Modal/index.tsx b/src/custom/Modal/index.tsx index 9959a6d82..5bdbb8e57 100644 --- a/src/custom/Modal/index.tsx +++ b/src/custom/Modal/index.tsx @@ -1,4 +1,7 @@ -import { ButtonProps, DialogProps, styled } from '@mui/material'; +import { ButtonProps, DialogOwnerState, DialogProps, styled } from '@mui/material'; +// Stable per-instance id across React 17/18/19 (same hook MUI Dialog uses +// internally; on React 17 the generated id appears after the first effect). +import { unstable_useId as useId } from '@mui/material/utils'; import React, { useRef, useState } from 'react'; import { Box, Dialog, IconButton, Paper, Typography } from '../../base'; import { ContainedButton, OutlinedButton, TextButton } from '../../base/Button/Button'; @@ -8,14 +11,40 @@ import FullScreenExitIcon from '../../icons/Fullscreen/FullScreenExitIcon'; import { CustomTooltip } from '../CustomTooltip'; import { HelperTextPopover } from '../HelperTextPopover'; -interface ModalProps extends DialogProps { +interface BaseModalProps extends Omit { closeModal: () => void; - title: string; headerIcon?: React.ReactNode; reactNode?: React.ReactNode; isFullScreenModeAllowed?: boolean; } +export type ModalProps = BaseModalProps & + ( + | { + /** + * Visible title for the modal. Automatically wired to aria-labelledby on the dialog paper. + * Note on precedence: when both `title` and `aria-label` are provided, `aria-labelledby` + * (pointing to the title) takes precedence per the W3C Accessible Name (accname) specification; + * `aria-label` is intentionally not rendered on the dialog paper. + */ + title: string; + 'aria-label'?: string; + 'aria-labelledby'?: string; + } + | { + /** Titleless modal with explicit aria-label for assistive technologies. */ + title?: string; + 'aria-label': string; + 'aria-labelledby'?: string; + } + | { + /** Titleless modal with explicit aria-labelledby reference. */ + title?: string; + 'aria-label'?: string; + 'aria-labelledby': string; + } + ); + interface ModalFooterProps { children: React.ReactNode; variant?: 'filled' | 'transparent'; @@ -29,9 +58,11 @@ type openModalCallback = (props: { onClose?: () => void; }) => void; -interface UseModalReturnI extends ModalProps { +export interface UseModalReturnI extends BaseModalProps { + title: string; openModal: openModalCallback; isOpen: boolean; + open: boolean; } export const CloseBtn = styled(IconButton)` @@ -146,17 +177,21 @@ const StyledFooter = styled('div', { } })); -export const Modal: React.FC = ({ - open, - closeModal, - title, - headerIcon, - reactNode, - children, - maxWidth = 'xs', - isFullScreenModeAllowed, - ...props -}) => { +const nonEmpty = (v?: string) => (v && v.trim() ? v : undefined); + +export const Modal: React.FC = (allProps) => { + const { + open, + closeModal, + title, + headerIcon, + reactNode, + children, + maxWidth = 'xs', + isFullScreenModeAllowed, + ...props + } = allProps; + /* * Use the incoming fullScreen prop only as the initial state. * After initialization, fullscreen is managed internally so, external @@ -165,7 +200,16 @@ export const Modal: React.FC = ({ * prop is applied after {...restProps}, so any caller-provided fullWidth is * intentionally overridden without needing to strip it here. */ - const { fullScreen: initialFullScreenState = false, ...restProps } = props; + const { + fullScreen: initialFullScreenState = false, + 'aria-label': resolvedAriaLabel, + 'aria-labelledby': callerAriaLabelledby, + 'aria-describedby': callerAriaDescribedby, + slotProps: callerSlotProps, + ...restProps + } = props; + + const titleId = useId(restProps.id ? `${restProps.id}-title` : undefined); const [fullScreen, setFullScreen] = useState(initialFullScreenState); @@ -173,21 +217,68 @@ export const Modal: React.FC = ({ setFullScreen((prev) => !prev); }; + const hasTitle = Boolean(title && title.trim().length > 0); + + const callerPaperProp = callerSlotProps?.paper; + + // Merges Paper accessibility attributes while strictly preserving caller's + // object-valued or callback-valued slotProps.paper. + const mergePaperAttributes = (baseProps: Record = {}) => { + const callerPaperLabelledby = baseProps['aria-labelledby'] as string | undefined; + const callerPaperLabel = baseProps['aria-label'] as string | undefined; + const callerPaperDescribedby = baseProps['aria-describedby'] as string | undefined; + + const finalLabelledby = + nonEmpty(callerAriaLabelledby) || + nonEmpty(callerPaperLabelledby) || + (hasTitle ? titleId : undefined); + const finalLabel = finalLabelledby + ? undefined + : nonEmpty(resolvedAriaLabel) || nonEmpty(callerPaperLabel); + const finalDescribedby = nonEmpty(callerAriaDescribedby) || nonEmpty(callerPaperDescribedby); + + const result: Record = { + ...baseProps, + // Suppress dangling auto-generated aria-labelledby on the dialog paper when + // neither title nor an explicit aria-labelledby is provided. + 'aria-labelledby': finalLabelledby, + 'aria-describedby': finalDescribedby + }; + + if (finalLabel) { + result['aria-label'] = finalLabel; + } else { + delete result['aria-label']; + } + + return result; + }; + + const resolvedPaperSlotProps = + typeof callerPaperProp === 'function' + ? (ownerState: DialogOwnerState) => + mergePaperAttributes(callerPaperProp(ownerState) as Record) + : mergePaperAttributes(callerPaperProp as Record | undefined); + + const mergedSlotProps = { + ...callerSlotProps, + paper: resolvedPaperSlotProps + }; + return ( - {title && ( + {hasTitle && ( {headerIcon && {headerIcon}} - + {title}
Date: Sun, 18 Oct 2026 15:35:51 +0000 Subject: [PATCH 2/3] test(Modal): add accessible name type checks and runtime regression tests (#1864) Signed-off-by: Parth Gartan --- src/__testing__/ModalAccessibility.test.tsx | 610 ++++++++++++++++++ .../fixtures/modalAccessibleName.tsx | 27 + .../tsconfig.modalAccessibleName.json | 7 + .../modalAccessibleNameTypes.test.ts | 37 ++ 4 files changed, 681 insertions(+) create mode 100644 src/__testing__/ModalAccessibility.test.tsx create mode 100644 src/__testing__/fixtures/modalAccessibleName.tsx create mode 100644 src/__testing__/fixtures/tsconfig.modalAccessibleName.json create mode 100644 src/__testing__/modalAccessibleNameTypes.test.ts diff --git a/src/__testing__/ModalAccessibility.test.tsx b/src/__testing__/ModalAccessibility.test.tsx new file mode 100644 index 000000000..1e4ed7c58 --- /dev/null +++ b/src/__testing__/ModalAccessibility.test.tsx @@ -0,0 +1,610 @@ +import type { DialogOwnerState, PaperProps } from '@mui/material'; +import { render, screen } from '@testing-library/react'; +import React from 'react'; +import { Modal } from '../custom/Modal'; +import { SistentThemeProvider } from '../theme'; + +// The Modal primitives transitively import CustomTooltip -> Markdown -> react-markdown (ESM-only). +// Jest's transformIgnorePatterns allowlist excludes the markdown ESM tree, so those leaf modules +// are stubbed here (mirrors DangerConfirmationModal.test.tsx and CatalogFilterSection.test.tsx). +jest.mock('react-markdown', () => ({ + __esModule: true, + default: ({ children }: { children: React.ReactNode }) =>
{children}
+})); + +jest.mock('remark-gfm', () => ({ + __esModule: true, + default: () => {} +})); + +jest.mock('rehype-raw', () => ({ + __esModule: true, + default: () => {} +})); + +describe('Modal accessibility contract (#1864)', () => { + it('connects dialog aria-labelledby to the rendered title element id by default', () => { + render( + + +
Modal Content
+ + + ); + + const dialog = screen.getByRole('dialog'); + const title = screen.getByTestId('modal-title'); + + const labelledBy = dialog.getAttribute('aria-labelledby'); + expect(labelledBy).toBeTruthy(); + expect(title.getAttribute('id')).toBe(labelledBy); + expect(title.textContent).toBe('Confirmation Dialog'); + + // Does not attach nonexistent aria-describedby + expect(dialog.getAttribute('aria-describedby')).toBeNull(); + }); + + it('generates distinct IDs for concurrently mounted modal instances to prevent DOM collision', () => { + render( + + +
Content One
+ + +
Content Two
+ + + ); + + // MUI Dialog applies aria-hidden to inert backdrop siblings; query with hidden: true + const dialogs = screen.getAllByRole('dialog', { hidden: true }); + const titles = screen.getAllByTestId('modal-title'); + + expect(dialogs).toHaveLength(2); + expect(titles).toHaveLength(2); + + // Assert order-independently that both dialogs pair 1:1 with distinct title IDs + const labelIds = dialogs.map((d) => d.getAttribute('aria-labelledby')); + const titleIds = titles.map((t) => t.getAttribute('id')); + + expect(new Set(labelIds).size).toBe(2); + expect(new Set(titleIds).size).toBe(2); + expect([...labelIds].sort()).toEqual([...titleIds].sort()); + }); + + it('derives title id from caller-provided id prop', () => { + render( + + +
Content
+ + + ); + + const dialog = screen.getByRole('dialog'); + const title = screen.getByTestId('modal-title'); + + expect(document.getElementById('delete-confirmation')).toBeTruthy(); + expect(dialog.getAttribute('aria-labelledby')).toBe('delete-confirmation-title'); + expect(title.getAttribute('id')).toBe('delete-confirmation-title'); + expect(title.textContent).toBe('Delete Resource'); + }); + + it('forwards custom aria-labelledby and aria-describedby without reusing aria-labelledby as title id', () => { + render( + +

External Section Heading

+ +
Description text
+ + + ); + + const dialog = screen.getByRole('dialog'); + const title = screen.getByTestId('modal-title'); + + // Dialog targets caller-supplied accessible name & description + expect(dialog.getAttribute('aria-labelledby')).toBe('external-heading'); + expect(dialog.getAttribute('aria-describedby')).toBe('custom-description'); + + // Title element gets its own distinct ID and does NOT collide with external-heading + const titleId = title.getAttribute('id'); + expect(titleId).toBeTruthy(); + expect(titleId).not.toBe('external-heading'); + + // Ensure external-heading remains uniquely owned by the external element in the DOM + const elementsWithExternalHeading = document.querySelectorAll('#external-heading'); + expect(elementsWithExternalHeading).toHaveLength(1); + expect(elementsWithExternalHeading[0].tagName.toLowerCase()).toBe('h2'); + }); + + it('supports multi-id aria-labelledby without producing invalid HTML ids on the title element', () => { + render( + + Dialog: + System Status + +
Status content
+ + + ); + + const dialog = screen.getByRole('dialog'); + const title = screen.getByTestId('modal-title'); + + expect(dialog.getAttribute('aria-labelledby')).toBe('label-prefix label-main'); + // Title element ID must be a single valid token, never the space-separated list + const titleId = title.getAttribute('id'); + expect(titleId).toBeTruthy(); + expect(titleId?.includes(' ')).toBe(false); + }); + + it('correctly provides accessible name to role="dialog" for titleless modal via aria-label', () => { + render( + + +
Settings content
+ + + ); + + const dialog = screen.getByRole('dialog'); + const root = dialog.closest('.MuiDialog-root'); + + // Dialog root role="presentation" div must not receive aria-label + expect(root).not.toBeNull(); + expect(root?.getAttribute('aria-label')).toBeNull(); + + // Accessible name is attached directly to role="dialog" + expect(dialog.getAttribute('aria-label')).toBe('Settings configuration'); + // Suppresses dangling generated aria-labelledby so aria-label is not obscured + expect(dialog.getAttribute('aria-labelledby')).toBeNull(); + expect(dialog.getAttribute('aria-describedby')).toBeNull(); + // No title header rendered + expect(screen.queryByTestId('modal-title')).toBeNull(); + }); + + it('does not leak aria-label onto the dialog root when both title and aria-label are provided and prioritizes aria-labelledby', () => { + render( + + +
Modal Content
+ + + ); + + const dialog = screen.getByRole('dialog'); + const title = screen.getByTestId('modal-title'); + const root = dialog.closest('.MuiDialog-root'); + + expect(root).not.toBeNull(); + expect(root?.getAttribute('aria-label')).toBeNull(); + // accname specification: aria-labelledby pointing to title wins; aria-label is omitted + expect(dialog.getAttribute('aria-label')).toBeNull(); + expect(dialog.getAttribute('aria-labelledby')).toBe(title.getAttribute('id')); + }); + + it('applies caller aria-describedby to role="dialog" and keeps it off the dialog root', () => { + render( + + +
Description content
+ + + ); + + const dialog = screen.getByRole('dialog'); + const root = dialog.closest('.MuiDialog-root'); + + expect(dialog.getAttribute('aria-describedby')).toBe('custom-description-id'); + expect(root?.getAttribute('aria-describedby')).toBeNull(); + }); + + it('renders titleless modal with only aria-labelledby without a title header', () => { + render( + + Section Heading + +
Content
+ + + ); + + const dialog = screen.getByRole('dialog'); + expect(dialog.getAttribute('aria-labelledby')).toBe('external-section-label'); + expect(dialog.getAttribute('aria-label')).toBeNull(); + expect(screen.queryByTestId('modal-title')).toBeNull(); + }); + + it('preserves object-valued slotProps.paper custom properties alongside accessibility attributes', () => { + const paperProps: PaperProps & { 'data-testid'?: string } = { + className: 'custom-paper-classname', + 'data-testid': 'customized-paper-slot', + elevation: 12 + }; + + render( + + +
Custom paper content
+ + + ); + + const dialog = screen.getByRole('dialog'); + const title = screen.getByTestId('modal-title'); + + // Retains caller's slotProps.paper properties + expect(dialog.classList.contains('custom-paper-classname')).toBe(true); + expect(dialog.getAttribute('data-testid')).toBe('customized-paper-slot'); + + // Merges correct accessibility attributes + expect(dialog.getAttribute('aria-labelledby')).toBe(title.getAttribute('id')); + }); + + it('preserves callback-valued slotProps.paper with ownerState and merged accessibility attributes', () => { + const paperCallback = jest.fn((ownerState: DialogOwnerState) => ({ + className: `dynamic-paper-${ownerState.fullWidth ? 'full' : 'fixed'}`, + 'data-testid': 'callback-paper-slot', + 'data-fullscreen': String(ownerState.fullScreen) + })); + + render( + + +
Callback paper content
+ + + ); + + const dialog = screen.getByRole('dialog'); + const title = screen.getByTestId('modal-title'); + + // Callback was invoked with ownerState + expect(paperCallback).toHaveBeenCalled(); + const passedOwnerState = paperCallback.mock.calls[0][0]; + expect(passedOwnerState).toBeDefined(); + + // Returned attributes are preserved + expect(dialog.getAttribute('data-testid')).toBe('callback-paper-slot'); + expect(dialog.classList.contains('dynamic-paper-full')).toBe(true); + + // Accessibility attributes are merged + expect(dialog.getAttribute('aria-labelledby')).toBe(title.getAttribute('id')); + }); + + it('honors aria precedence in slotProps.paper (Modal prop > slotProps.paper > generated title id)', () => { + // 1. Modal-level prop wins over slotProps.paper + const { unmount } = render( + + +
Content
+ + + ); + + let dialog = screen.getByRole('dialog'); + expect(dialog.getAttribute('aria-labelledby')).toBe('modal-level-labelledby'); + expect(dialog.getAttribute('aria-describedby')).toBe('modal-level-describedby'); + unmount(); + + // 2. slotProps.paper wins over generated title id + render( + + +
Content
+ + + ); + + dialog = screen.getByRole('dialog'); + const title = screen.getByTestId('modal-title'); + expect(dialog.getAttribute('aria-labelledby')).toBe('paper-level-labelledby-only'); + expect(dialog.getAttribute('aria-labelledby')).not.toBe(title.getAttribute('id')); + expect(dialog.getAttribute('aria-describedby')).toBe('paper-level-describedby-only'); + }); + + it('unmounting and remounting a modal yields a title id matching aria-labelledby on each mount', () => { + const { unmount } = render( + + +
First Run
+ + + ); + + let dialog = screen.getByRole('dialog'); + let title = screen.getByTestId('modal-title'); + const firstId = title.getAttribute('id'); + expect(firstId).toBeTruthy(); + expect(dialog.getAttribute('aria-labelledby')).toBe(firstId); + + unmount(); + + render( + + +
Second Run
+ + + ); + + dialog = screen.getByRole('dialog'); + title = screen.getByTestId('modal-title'); + const secondId = title.getAttribute('id'); + expect(secondId).toBeTruthy(); + expect(dialog.getAttribute('aria-labelledby')).toBe(secondId); + }); + + it('resolves accessible name from title, aria-label, or external aria-labelledby at runtime', () => { + const noop = jest.fn(); + + // (a) title only: accessible name matches title text + const { unmount: unmountTitle } = render( + + +
Content
+ + + ); + expect(screen.getByRole('dialog', { name: 'Visible Title' })).toBeTruthy(); + unmountTitle(); + + // (b) aria-label only: accessible name matches aria-label + const { unmount: unmountLabel } = render( + + +
Content
+ + + ); + expect(screen.getByRole('dialog', { name: 'Explicit Aria Label' })).toBeTruthy(); + unmountLabel(); + + // (c) aria-labelledby only: accessible name resolves external element text + const { unmount: unmountLabelledby } = render( + +

External Heading Title

+ +
Content
+ + + ); + expect(screen.getByRole('dialog', { name: 'External Heading Title' })).toBeTruthy(); + unmountLabelledby(); + }); + + it('treats whitespace-only title as absent, omitting generated aria-labelledby and preserving aria-label fallback', () => { + render( + + +
Content
+ + + ); + + const dialog = screen.getByRole('dialog'); + expect(dialog.getAttribute('aria-label')).toBe('Whitespace Fallback'); + expect(dialog.getAttribute('aria-labelledby')).toBeNull(); + expect(screen.queryByTestId('modal-title')).toBeNull(); + }); + + it('prioritizes top-level aria-label over paper-level aria-label on titleless modals', () => { + render( + + +
Content
+ + + ); + + const dialog = screen.getByRole('dialog'); + expect(dialog.getAttribute('aria-label')).toBe('Top Level Name'); + expect(dialog.getAttribute('aria-labelledby')).toBeNull(); + }); + + it('suppresses paper-level aria-label when aria-labelledby is present from title or prop', () => { + render( + + +
Content
+ + + ); + + const dialog = screen.getByRole('dialog'); + const title = screen.getByTestId('modal-title'); + expect(dialog.getAttribute('aria-labelledby')).toBe(title.getAttribute('id')); + expect(dialog.getAttribute('aria-label')).toBeNull(); + }); + + it('derives distinct title IDs for concurrently mounted modals with distinct caller-provided IDs', () => { + render( + + +
First
+ + +
Second
+ + + ); + + const dialogs = screen.getAllByRole('dialog', { hidden: true }); + const titles = screen.getAllByTestId('modal-title'); + + const dialogLabelledby = dialogs.map((d) => d.getAttribute('aria-labelledby')).sort(); + const titleIds = titles.map((t) => t.getAttribute('id')).sort(); + const expected = ['modal-first-title', 'modal-second-title']; + + expect(dialogLabelledby).toEqual(titleIds); + expect(dialogLabelledby).toEqual(expected); + }); + + it('falls back to generated title id when caller passes empty string aria-labelledby', () => { + render( + + +
Content
+ + + ); + + const dialog = screen.getByRole('dialog'); + const title = screen.getByTestId('modal-title'); + const titleId = title.getAttribute('id'); + + expect(titleId).toBeTruthy(); + expect(dialog.getAttribute('aria-labelledby')).toBe(titleId); + }); + + it('falls through to slotProps.paper aria-describedby when top-level aria-describedby is empty string', () => { + render( + + +
Content
+ + + ); + + const dialog = screen.getByRole('dialog'); + expect(dialog.getAttribute('aria-describedby')).toBe('paper-describedby-id'); + }); + + it('prioritizes slotProps.paper aria-labelledby over top-level aria-label on titleless modal', () => { + render( + + +
Content
+ + + ); + + const dialog = screen.getByRole('dialog'); + expect(dialog.getAttribute('aria-labelledby')).toBe('paper-level-labelledby-id'); + expect(dialog.getAttribute('aria-label')).toBeNull(); + }); + + it('falls through whitespace-only aria-labelledby, aria-label, and aria-describedby to title id and paper-desc', () => { + render( + + +
Content
+ + + ); + + const dialog = screen.getByRole('dialog'); + const title = screen.getByTestId('modal-title'); + + expect(dialog.getAttribute('aria-labelledby')).toBe(title.getAttribute('id')); + expect(dialog.getAttribute('aria-label')).toBeNull(); + expect(dialog.getAttribute('aria-describedby')).toBe('paper-desc'); + }); +}); diff --git a/src/__testing__/fixtures/modalAccessibleName.tsx b/src/__testing__/fixtures/modalAccessibleName.tsx new file mode 100644 index 000000000..9ae526029 --- /dev/null +++ b/src/__testing__/fixtures/modalAccessibleName.tsx @@ -0,0 +1,27 @@ +// Type-level fixture for Modal accessible name contract (#1864). +// It is compiled by `modalAccessibleNameTypes.test.ts` via `tsc --noEmit`, +// never executed, and is deliberately outside jest's `*.test.*` glob so jest +// does not collect it. +import { Modal } from '../../custom/Modal'; + +const closeModal = () => {}; + +// Valid combination 1: title only +export const withTitle = ; + +// Valid combination 2: aria-label only +export const withAriaLabel = ; + +// Valid combination 3: aria-labelledby only +export const withAriaLabelledby = ( + +); + +// Valid combination 4: useModal return object spread onto Modal +declare const modalState: import('../../custom/Modal').UseModalReturnI; +export const withUseModal = ; + +// Self-check / negative case: modal without title, aria-label, or aria-labelledby +// is rejected at compile time by the ModalProps union. +// @ts-expect-error Modal without title must provide aria-label or aria-labelledby +export const rejectsNameless = ; diff --git a/src/__testing__/fixtures/tsconfig.modalAccessibleName.json b/src/__testing__/fixtures/tsconfig.modalAccessibleName.json new file mode 100644 index 000000000..9b5395c66 --- /dev/null +++ b/src/__testing__/fixtures/tsconfig.modalAccessibleName.json @@ -0,0 +1,7 @@ +{ + "extends": "../../../tsconfig.json", + "compilerOptions": { + "noEmit": true + }, + "include": ["./modalAccessibleName.tsx"] +} diff --git a/src/__testing__/modalAccessibleNameTypes.test.ts b/src/__testing__/modalAccessibleNameTypes.test.ts new file mode 100644 index 000000000..f0746fca7 --- /dev/null +++ b/src/__testing__/modalAccessibleNameTypes.test.ts @@ -0,0 +1,37 @@ +import { type Diagnostics, typeCheckFixture } from './helpers/tscFixture'; + +// Type-level regression guard for Modal accessible name contract (#1864). +// +// ModalProps is typed as a union requiring either `title: string`, +// `'aria-label': string`, or `'aria-labelledby': string`. +// +// jest transforms with @swc/jest, which strips types without checking them, so +// a plain type-only assertion file would pass no matter what ModalProps is. +// This test therefore shells out to `tsc` over a fixture that exercises the +// valid combinations and asserts the compile-time rejection of nameless modals, +// through the shared `typeCheckFixture` harness. + +const FIXTURE = 'src/__testing__/fixtures/modalAccessibleName.tsx'; +const PROJECT = 'src/__testing__/fixtures/tsconfig.modalAccessibleName.json'; + +describe('Modal accessible name type contract', () => { + let diagnostics: Diagnostics; + + beforeAll(() => { + diagnostics = typeCheckFixture(FIXTURE, PROJECT); + }, 180_000); + + // Ordered deliberately: the two assertions below are only meaningful once the + // fixture is known to have been compiled, so prove that first. + it('compiles the fixture', () => { + expect(diagnostics.compiledFixture).toBe(true); + }); + + it('reports no config-level failure that would skip the fixture', () => { + expect(diagnostics.config).toEqual([]); + }); + + it('accepts valid accessible name combinations and rejects nameless modals', () => { + expect(diagnostics.fixture).toEqual([]); + }); +}); From 6ec8e494d33bb12df11bbb144206d45ea5c87e5b Mon Sep 17 00:00:00 2001 From: Parth Gartan Date: Sun, 18 Oct 2026 15:37:12 +0000 Subject: [PATCH 3/3] docs(Modal): document dynamic title IDs and accessible name precedence in DESIGN.md (#1864) Signed-off-by: Parth Gartan --- DESIGN.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESIGN.md b/DESIGN.md index fffec3202..2f911a8a3 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -531,7 +531,7 @@ Component-specific behaviors are never promoted to Canonical Design Rules withou - **Enforcement Status**: Contrast is guided by theme tokens and helper utilities (`readableTextColor`), but is not enforced by a global automated linting/test suite across every component. - **Label Capitalization**: Action labels use `textTransform: 'capitalize'` built into `textB2SemiBold`. - **Semantic ARIA**: Icon-only buttons must supply descriptive `aria-label` and `Tooltip`. -- **Modal Dialog Accessibility (`aria-labelledby`)**: Sistent's `Modal` component (`src/custom/Modal/index.tsx`) hardcodes `aria-labelledby="alert-dialog-slide-title"` and `aria-describedby="alert-dialog-slide-description"` on the underlying dialog, while its internal title element does not currently forward an `id`. This library-level accessibility gap is tracked in [#1864](https://github.com/layer5io/sistent/issues/1864). As a temporary consumer workaround until resolved in the library, consumers should conditionally mount dialog instances (`{isOpen && }`) rather than keeping multiple hidden instances mounted simultaneously to avoid duplicate-ID collisions in the DOM accessibility tree. +- **Modal Dialog Accessibility (`aria-labelledby` / `aria-describedby`)**: Sistent's `Modal` component (`src/custom/Modal/index.tsx`) generates a dynamic, stable per-instance title ID via MUI's `unstable_useId` (`${id}-title` when caller provides `id`, otherwise an auto-generated token) wired to `aria-labelledby` on the dialog paper, eliminating DOM ID collisions across concurrent modals (#1864). `ModalProps` enforces an accessible name at compile time via a union requiring `title`, `aria-label`, or `aria-labelledby`. When a visible title is non-empty, `aria-labelledby` defaults to the generated title element ID; empty or whitespace-only titles are treated as absent for accessibility decisions, omitting the title header (including its action controls) and dangling `aria-labelledby`. Caller-provided accessibility attributes are forwarded directly to the dialog paper (kept off the dialog presentation root), and caller `slotProps.paper` attributes are preserved in both object and callback forms. `aria-labelledby` resolves as Modal-level `aria-labelledby` > `slotProps.paper['aria-labelledby']` > the generated title ID (for a non-blank `title`). `aria-label` applies only when no `aria-labelledby` resolves, with Modal-level `aria-label` > `slotProps.paper['aria-label']`. `aria-describedby` follows Modal-level > `slotProps.paper`. Callers requiring uniqueness across concurrent instances should either supply distinct `id` props or rely on the automatic collision-free `useId` tokens. ---