mirror of
https://github.com/amitwh/markdown-converter.git
synced 2026-08-02 10:00:17 +05:30
fix(modals): drop shadcn-style aria-describedby to silence Radix warning
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.
This commit is contained in:
@@ -17,10 +17,10 @@ export function AboutDialog() {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open={isOpen} onOpenChange={(o) => !o && closeModal()}>
|
<Dialog open={isOpen} onOpenChange={(o) => !o && closeModal()}>
|
||||||
<DialogContent aria-describedby="about-desc">
|
<DialogContent>
|
||||||
<DialogHeader>
|
<DialogHeader>
|
||||||
<DialogTitle>About MarkdownConverter</DialogTitle>
|
<DialogTitle>About MarkdownConverter</DialogTitle>
|
||||||
<DialogDescription id="about-desc">
|
<DialogDescription>
|
||||||
Professional Markdown editor and universal file converter.
|
Professional Markdown editor and universal file converter.
|
||||||
</DialogDescription>
|
</DialogDescription>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
|
|||||||
@@ -33,10 +33,10 @@ export function AsciiGeneratorDialog() {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
||||||
<DialogContent aria-describedby="ascii-desc" className="max-w-2xl">
|
<DialogContent className="max-w-2xl">
|
||||||
<DialogHeader>
|
<DialogHeader>
|
||||||
<DialogTitle>ASCII generator</DialogTitle>
|
<DialogTitle>ASCII generator</DialogTitle>
|
||||||
<DialogDescription id="ascii-desc">Type text, pick a font, see ASCII art</DialogDescription>
|
<DialogDescription>Type text, pick a font, see ASCII art</DialogDescription>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
<div className="space-y-3">
|
<div className="space-y-3">
|
||||||
<div>
|
<div>
|
||||||
|
|||||||
@@ -17,10 +17,10 @@ export function ConfirmDialog(props: ConfirmProps) {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open onOpenChange={(o) => !o && handleCancel()}>
|
<Dialog open onOpenChange={(o) => !o && handleCancel()}>
|
||||||
<DialogContent aria-describedby="confirm-body">
|
<DialogContent>
|
||||||
<DialogHeader>
|
<DialogHeader>
|
||||||
<DialogTitle>{title}</DialogTitle>
|
<DialogTitle>{title}</DialogTitle>
|
||||||
<DialogDescription id="confirm-body">{body}</DialogDescription>
|
<DialogDescription>{body}</DialogDescription>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
<DialogFooter>
|
<DialogFooter>
|
||||||
<Button variant="ghost" onClick={handleCancel}>
|
<Button variant="ghost" onClick={handleCancel}>
|
||||||
|
|||||||
@@ -37,10 +37,10 @@ export function ExportBatchDialog({ sourcePaths }: { sourcePaths: string[] }) {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
||||||
<DialogContent aria-describedby="batch-desc">
|
<DialogContent>
|
||||||
<DialogHeader>
|
<DialogHeader>
|
||||||
<DialogTitle>Batch export</DialogTitle>
|
<DialogTitle>Batch export</DialogTitle>
|
||||||
<DialogDescription id="batch-desc">{sourcePaths.length} files</DialogDescription>
|
<DialogDescription>{sourcePaths.length} files</DialogDescription>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
<div className="space-y-3 text-sm">
|
<div className="space-y-3 text-sm">
|
||||||
<div>
|
<div>
|
||||||
|
|||||||
@@ -41,10 +41,10 @@ export function ExportDocxDialog({ sourcePath }: { sourcePath: string }) {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
||||||
<DialogContent aria-describedby="docx-desc">
|
<DialogContent>
|
||||||
<DialogHeader>
|
<DialogHeader>
|
||||||
<DialogTitle>Export to DOCX</DialogTitle>
|
<DialogTitle>Export to DOCX</DialogTitle>
|
||||||
<DialogDescription id="docx-desc">{sourcePath}</DialogDescription>
|
<DialogDescription>{sourcePath}</DialogDescription>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
<div className="space-y-3 text-sm">
|
<div className="space-y-3 text-sm">
|
||||||
<div>
|
<div>
|
||||||
|
|||||||
@@ -43,10 +43,10 @@ export function ExportHtmlDialog({ sourcePath }: { sourcePath: string }) {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
||||||
<DialogContent aria-describedby="html-desc">
|
<DialogContent>
|
||||||
<DialogHeader>
|
<DialogHeader>
|
||||||
<DialogTitle>Export to HTML</DialogTitle>
|
<DialogTitle>Export to HTML</DialogTitle>
|
||||||
<DialogDescription id="html-desc">{sourcePath}</DialogDescription>
|
<DialogDescription>{sourcePath}</DialogDescription>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
<div className="space-y-3 text-sm">
|
<div className="space-y-3 text-sm">
|
||||||
<label className="flex items-center gap-2">
|
<label className="flex items-center gap-2">
|
||||||
|
|||||||
@@ -56,10 +56,10 @@ export function ExportPdfDialog({ sourcePath }: { sourcePath: string }) {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
||||||
<DialogContent aria-describedby="pdf-desc">
|
<DialogContent>
|
||||||
<DialogHeader>
|
<DialogHeader>
|
||||||
<DialogTitle>Export to PDF</DialogTitle>
|
<DialogTitle>Export to PDF</DialogTitle>
|
||||||
<DialogDescription id="pdf-desc">{sourcePath}</DialogDescription>
|
<DialogDescription>{sourcePath}</DialogDescription>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
<div className="space-y-3 text-sm">
|
<div className="space-y-3 text-sm">
|
||||||
<div>
|
<div>
|
||||||
|
|||||||
@@ -47,10 +47,10 @@ export function FindInFilesDialog() {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
||||||
<DialogContent aria-describedby="find-desc" className="max-w-2xl">
|
<DialogContent className="max-w-2xl">
|
||||||
<DialogHeader>
|
<DialogHeader>
|
||||||
<DialogTitle>Find in files</DialogTitle>
|
<DialogTitle>Find in files</DialogTitle>
|
||||||
<DialogDescription id="find-desc">
|
<DialogDescription>
|
||||||
{rootPath ? `Search in ${rootPath}` : 'No folder open'}
|
{rootPath ? `Search in ${rootPath}` : 'No folder open'}
|
||||||
</DialogDescription>
|
</DialogDescription>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
|
|||||||
@@ -15,10 +15,10 @@ export function SettingsSheet() {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Sheet open onOpenChange={(o) => !o && closeModal()}>
|
<Sheet open onOpenChange={(o) => !o && closeModal()}>
|
||||||
<SheetContent aria-describedby="settings-desc" side="right" className="w-full sm:max-w-[480px]">
|
<SheetContent side="right" className="w-full sm:max-w-[480px]">
|
||||||
<SheetHeader>
|
<SheetHeader>
|
||||||
<SheetTitle>Settings</SheetTitle>
|
<SheetTitle>Settings</SheetTitle>
|
||||||
<SheetDescription id="settings-desc">Editor, theme, and export preferences</SheetDescription>
|
<SheetDescription>Editor, theme, and export preferences</SheetDescription>
|
||||||
</SheetHeader>
|
</SheetHeader>
|
||||||
<Tabs defaultValue="editor" className="mt-4">
|
<Tabs defaultValue="editor" className="mt-4">
|
||||||
<TabsList className="grid w-full grid-cols-5">
|
<TabsList className="grid w-full grid-cols-5">
|
||||||
|
|||||||
@@ -60,10 +60,10 @@ export function TableGeneratorDialog() {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
||||||
<DialogContent aria-describedby="table-desc" className="max-w-2xl">
|
<DialogContent className="max-w-2xl">
|
||||||
<DialogHeader>
|
<DialogHeader>
|
||||||
<DialogTitle>Table generator</DialogTitle>
|
<DialogTitle>Table generator</DialogTitle>
|
||||||
<DialogDescription id="table-desc">Specify rows × columns to generate a markdown table</DialogDescription>
|
<DialogDescription>Specify rows × columns to generate a markdown table</DialogDescription>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
<div className="space-y-3">
|
<div className="space-y-3">
|
||||||
<div className="flex gap-3">
|
<div className="flex gap-3">
|
||||||
|
|||||||
@@ -17,10 +17,10 @@ export function WelcomeDialog() {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open onOpenChange={(o) => !o && handleClose()}>
|
<Dialog open onOpenChange={(o) => !o && handleClose()}>
|
||||||
<DialogContent aria-describedby="welcome-desc" className="max-w-xl">
|
<DialogContent className="max-w-xl">
|
||||||
<DialogHeader>
|
<DialogHeader>
|
||||||
<DialogTitle>Welcome to MarkdownConverter</DialogTitle>
|
<DialogTitle>Welcome to MarkdownConverter</DialogTitle>
|
||||||
<DialogDescription id="welcome-desc">
|
<DialogDescription>
|
||||||
A polished editor for Markdown, with PDF, DOCX, and HTML export.
|
A polished editor for Markdown, with PDF, DOCX, and HTML export.
|
||||||
</DialogDescription>
|
</DialogDescription>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
|
|||||||
@@ -65,10 +65,10 @@ export function WordExportDialog({ sourcePath }: { sourcePath: string }) {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
<Dialog open onOpenChange={(o) => !o && closeModal()}>
|
||||||
<DialogContent aria-describedby="word-desc">
|
<DialogContent>
|
||||||
<DialogHeader>
|
<DialogHeader>
|
||||||
<DialogTitle>Export to Word (.docx)</DialogTitle>
|
<DialogTitle>Export to Word (.docx)</DialogTitle>
|
||||||
<DialogDescription id="word-desc">{sourcePath}</DialogDescription>
|
<DialogDescription>{sourcePath}</DialogDescription>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
<div className="space-y-3 text-sm">
|
<div className="space-y-3 text-sm">
|
||||||
<div>
|
<div>
|
||||||
|
|||||||
@@ -31,4 +31,27 @@ describe('WelcomeDialog', () => {
|
|||||||
await userEvent.click(screen.getByRole('button', { name: /get started/i }));
|
await userEvent.click(screen.getByRole('button', { name: /get started/i }));
|
||||||
expect(useSettingsStore.getState().welcomeDismissed).toBe(true);
|
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();
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user