fix(a11y): use useId() for Modal aria-labelledby and add test (#1301)

Modal dialogs now carry an accessible name: the dialog is linked to its title via aria-labelledby, with a per-instance id so several open dialogs cannot collide. Adds a Modal test suite covering the accessible-name contract, id uniqueness, and close behaviour.
This commit is contained in:
Ali Mahmmoud
2026-07-14 18:50:52 +03:00
committed by GitHub
parent 1e8db450c4
commit ab70cbd528
4 changed files with 129 additions and 2 deletions
+6
View File
@@ -363,6 +363,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Horizontal album rails clipped an outer card shadow at the edges, which only themes that use a real drop shadow ran into. Working around it meant overriding the rail's `overflow`, and that disabled the rail's `<` / `>` scroll arrows. Rails now reserve room for the shadow inside the rail itself, so the arrows keep working; a theme that needs more room can raise `--rail-shadow-room` instead of touching `overflow`.
### Accessibility — modal dialogs announce their title
**By [@AliMahmoudDev](https://github.com/AliMahmoudDev), PR [#1301](https://github.com/Psychotoxical/psysonic/pull/1301)**
* Modal dialogs carried no accessible name, so a screen reader announced them without saying which dialog had opened. The dialog is now linked to its title, and each instance gets its own id so several open dialogs cannot be confused for one another.
## [1.49.0] - 2026-06-29
+7
View File
@@ -480,6 +480,13 @@ const CONTRIBUTOR_ENTRIES = [
'Sync: form POST for large play queues to avoid HTTP 414 behind reverse proxies (PR #1262)',
],
},
{
github: 'AliMahmoudDev',
since: '1.50.0',
contributions: [
'Accessibility: modal dialogs announce their title to screen readers (aria-labelledby) (PR #1301)',
],
},
] as const;
// PR number of a contributor's first listed contribution, used as the
+111
View File
@@ -0,0 +1,111 @@
import { describe, it, expect, vi } from 'vitest';
import { screen, fireEvent } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { renderWithProviders } from '@/test/helpers/renderWithProviders';
import Modal from '@/ui/Modal';
describe('Modal', () => {
const defaultProps = {
open: true,
onClose: vi.fn(),
title: 'Test Modal',
children: <p>Modal body content</p>,
};
it('renders nothing when open is false', () => {
// Query the document, not the render container: Modal portals into
// document.body, so the container is empty whether it is open or not.
renderWithProviders(<Modal {...defaultProps} open={false} />);
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
});
it('renders the dialog when open is true', () => {
renderWithProviders(<Modal {...defaultProps} />);
expect(screen.getByRole('dialog')).toBeInTheDocument();
});
it('renders the title text', () => {
renderWithProviders(<Modal {...defaultProps} />);
expect(screen.getByText('Test Modal')).toBeInTheDocument();
});
it('renders children content', () => {
renderWithProviders(<Modal {...defaultProps} />);
expect(screen.getByText('Modal body content')).toBeInTheDocument();
});
it('has aria-modal="true" on the dialog', () => {
renderWithProviders(<Modal {...defaultProps} />);
expect(screen.getByRole('dialog')).toHaveAttribute('aria-modal', 'true');
});
it('links the dialog to its title via aria-labelledby', () => {
renderWithProviders(<Modal {...defaultProps} />);
const dialog = screen.getByRole('dialog');
const labelledBy = dialog.getAttribute('aria-labelledby');
expect(labelledBy).toBeTruthy();
const titleEl = document.getElementById(labelledBy!);
expect(titleEl).toBeTruthy();
expect(titleEl!.textContent).toContain('Test Modal');
});
it('uses a unique id per instance so multiple modals do not clash', () => {
renderWithProviders(
<>
<Modal {...defaultProps} title="First" />
<Modal {...defaultProps} title="Second" />
</>,
);
const dialogs = screen.getAllByRole('dialog');
const id1 = dialogs[0].getAttribute('aria-labelledby');
const id2 = dialogs[1].getAttribute('aria-labelledby');
expect(id1).not.toBe(id2);
});
it('calls onClose when Escape is pressed', () => {
const onClose = vi.fn();
renderWithProviders(<Modal {...defaultProps} onClose={onClose} />);
fireEvent.keyDown(window, { key: 'Escape' });
expect(onClose).toHaveBeenCalledOnce();
});
it('calls onClose when the backdrop is clicked', async () => {
const onClose = vi.fn();
renderWithProviders(<Modal {...defaultProps} onClose={onClose} />);
await userEvent.click(screen.getByRole('dialog').parentElement!);
expect(onClose).toHaveBeenCalledOnce();
});
it('does not call onClose when the dialog content is clicked', async () => {
const onClose = vi.fn();
renderWithProviders(<Modal {...defaultProps} onClose={onClose} />);
await userEvent.click(screen.getByRole('dialog'));
expect(onClose).not.toHaveBeenCalled();
});
it('calls onClose when the close button is clicked', async () => {
const onClose = vi.fn();
renderWithProviders(<Modal {...defaultProps} onClose={onClose} />);
await userEvent.click(screen.getByRole('button', { name: /close/i }));
expect(onClose).toHaveBeenCalledOnce();
});
it('hides the close button when hideClose is true', () => {
renderWithProviders(<Modal {...defaultProps} hideClose />);
expect(screen.queryByRole('button', { name: /close/i })).not.toBeInTheDocument();
});
it('renders the subtitle when provided', () => {
renderWithProviders(<Modal {...defaultProps} subtitle="v1.2.3" />);
expect(screen.getByText('v1.2.3')).toBeInTheDocument();
});
it('renders the footer when provided', () => {
renderWithProviders(
<Modal {...defaultProps} footer={<button type="button">Save</button>} />,
);
expect(screen.getByRole('button', { name: 'Save' })).toBeInTheDocument();
});
});
+5 -2
View File
@@ -1,4 +1,4 @@
import { type ReactNode, useEffect } from 'react';
import { type ReactNode, useEffect, useId } from 'react';
import { createPortal } from 'react-dom';
import { X } from 'lucide-react';
@@ -31,6 +31,8 @@ interface ModalProps {
export default function Modal({
open, onClose, title, subtitle, icon, footer, size = 'md', hideClose, closeLabel, children,
}: ModalProps) {
const titleId = useId();
useEffect(() => {
if (!open) return;
const onKey = (e: KeyboardEvent) => {
@@ -48,12 +50,13 @@ export default function Modal({
className={`ui-modal ui-modal--${size}`}
role="dialog"
aria-modal="true"
aria-labelledby={titleId}
onClick={e => e.stopPropagation()}
>
<div className="ui-modal-header">
{icon && <span className="ui-modal-icon">{icon}</span>}
<div className="ui-modal-titles">
<span className="ui-modal-title">{title}</span>
<span className="ui-modal-title" id={titleId}>{title}</span>
{subtitle != null && <span className="ui-modal-subtitle">{subtitle}</span>}
</div>
{!hideClose && (