mirror of
https://github.com/amitwh/markdown-converter.git
synced 2026-08-02 18:10:18 +05:30
amitwh
58ca3014d8
Every DialogContent had aria-describedby="X-desc" and every DialogDescription had a matching id="X-desc". That pattern is the default shadcn/ui recipe, but it breaks Radix 1.1.x. Root cause: Radix auto-generates a descriptionId via useId() (e.g. :r0:) and stores it in the Dialog context. The DescriptionWarning effect does `document.getElementById(descriptionId)` and warns if the element is missing. When the description element's id is overridden to "welcome-desc", the DOM has id="welcome-desc" but the context still has :r0:, so the lookup misses and the warning fires on every dialog open. Fix: drop both overrides. Let Radix manage the id and aria-describedby itself. Screen-reader semantics are unchanged (Radix still wires Content -> Description via the auto-generated id). Touched: 12 modal files (11 DialogContent/DialogDescription pairs + SettingsSheet which uses Sheet). +1 regression test in WelcomeDialog.test.tsx that spies on console.warn and asserts the "Missing Description" message is never emitted. Tests: 306/306 pass (was 305). Long-term memory: radix-aria-describedby-anti-pattern.md captures the do-not-reintroduce rule for future agents.
58 lines
2.4 KiB
TypeScript
58 lines
2.4 KiB
TypeScript
import { describe, it, expect, beforeEach } from 'vitest';
|
|
import { render, screen } from '@testing-library/react';
|
|
import userEvent from '@testing-library/user-event';
|
|
import { WelcomeDialog } from '@/components/modals/WelcomeDialog';
|
|
import { useSettingsStore } from '@/stores/settings-store';
|
|
import { useAppStore } from '@/stores/app-store';
|
|
|
|
describe('WelcomeDialog', () => {
|
|
beforeEach(() => {
|
|
localStorage.clear();
|
|
useSettingsStore.setState(useSettingsStore.getInitialState());
|
|
useAppStore.setState({ modal: { kind: null } } as any);
|
|
});
|
|
|
|
it('renders a heading and quick-start content', () => {
|
|
render(<WelcomeDialog />);
|
|
expect(screen.getByRole('heading', { name: /welcome/i })).toBeInTheDocument();
|
|
expect(screen.getByText(/open a folder/i)).toBeInTheDocument();
|
|
});
|
|
|
|
it('closing without the checkbox does not dismiss future welcome dialogs', async () => {
|
|
render(<WelcomeDialog />);
|
|
await userEvent.click(screen.getByRole('button', { name: /get started/i }));
|
|
expect(useSettingsStore.getState().welcomeDismissed).toBe(false);
|
|
expect(useAppStore.getState().modal).toEqual({ kind: null });
|
|
});
|
|
|
|
it('checking "don\'t show again" persists the flag', async () => {
|
|
render(<WelcomeDialog />);
|
|
await userEvent.click(screen.getByRole('checkbox', { name: /don't show again/i }));
|
|
await userEvent.click(screen.getByRole('button', { name: /get started/i }));
|
|
expect(useSettingsStore.getState().welcomeDismissed).toBe(true);
|
|
});
|
|
|
|
// Regression guard: Radix Dialog emits a console.warn when the Content's
|
|
// aria-describedby points at an id that isn't in the DOM. The default
|
|
// shadcn/ui pattern (custom id="X-desc" on DialogDescription) breaks that
|
|
// link because Radix's internal descriptionId is auto-generated via useId.
|
|
// The fix is to drop the override and let Radix manage the id itself.
|
|
it('does not emit the Radix "Missing Description" warning', async () => {
|
|
const warnings: string[] = [];
|
|
const spy = vi.spyOn(console, 'warn').mockImplementation((...args) => {
|
|
warnings.push(args.map(String).join(' '));
|
|
});
|
|
try {
|
|
render(<WelcomeDialog />);
|
|
// Effects that fire the warning run in a microtask; one tick is enough.
|
|
await Promise.resolve();
|
|
const offending = warnings.filter((w) =>
|
|
/Missing\s+`?Description`?|aria-describedby=\{undefined\}/.test(w),
|
|
);
|
|
expect(offending).toEqual([]);
|
|
} finally {
|
|
spy.mockRestore();
|
|
}
|
|
});
|
|
});
|