Replace the inlined New-Folder modal in FolderNav with the shipped NewFolderDialog shadcn component (which has duplicate-name detection the inline form lacks) #134

New issue

Closed

opened 2026-10-03 03:58:38 +00:00 by Mark · 1 comment

Mark commented

2026-10-03 03:58:38 +00:00

Owner

Finding

src/components/gallery/NewFolderDialog.tsx is a complete, working shadcn/ui modal for creating a new folder in the Gallery. It has duplicate-name detection, input validation, and keyboard handling. It has no importers. src/components/gallery/FolderNav.tsx inlined its own folder-creation modal instead of using this component.

The Gallery has TWO implementations of the new-folder modal: the shadcn one (correct) and the inlined one (in FolderNav.tsx). Consolidating to use the real component is the right fix.

Where

Shipped component: src/components/gallery/NewFolderDialog.tsx (95 lines, full shadcn/ui Dialog with Header/Content/Footer, Input, Button, useRef for autofocus, useEffect for reset, isDuplicate detection, isInvalid guard)
Inlined duplicate: src/components/gallery/FolderNav.tsx (lines 122-160, the inline <form> for new folder creation)
Tests: src/components/gallery/issue-112-aria.test.tsx (180-186) already tests the getByPlaceholderText(/folder name/i) and queryByPlaceholderText behavior on the inlined form

Suggested fix shape

Replace the inlined form in FolderNav.tsx with <NewFolderDialog>. The component already accepts open, onOpenChange, existingFolders, onConfirm — all the props the inlined form needs. The wiring is mechanical:

FolderNav.tsx: replace the local useState for "showing the new folder form" with a dialogOpen boolean
Replace the inlined form with <NewFolderDialog open={dialogOpen} onOpenChange={setDialogOpen} existingFolders={folders.map(f => f.name)} onConfirm={handleAdd} />
Delete the local form state and the form JSX
Update issue-112-aria.test.tsx to query the new dialog structure (the getByPlaceholderText should still work since NewFolderDialog uses <Input placeholder="Folder name...">)

The benefits:

Duplicate-name detection that's already in NewFolderDialog (currently missing from FolderNav's inline form — silent name collisions possible)
Proper shadcn/ui focus management, escape-to-close, backdrop click
A single source of truth for new-folder behavior

Severity

Medium. The inlined form works but lacks the duplicate-name detection the real component has. Two users creating "Holiday Designs" would silently produce two folders.

Discovered by

Gap analysis (2026-10-02) of unreached components.

src/components/gallery/issue-112-aria.test.tsx — tests the inlined form, will need updating to the dialog structure
Round 1 fix commit 1788bdc — "fix(gallery): replace window.prompt 'New Folder' with an inline modal" — that fix was correct (window.prompt was worse), but the inlined version is now superseded by the real component.

## Finding `src/components/gallery/NewFolderDialog.tsx` is a complete, working shadcn/ui modal for creating a new folder in the Gallery. It has duplicate-name detection, input validation, and keyboard handling. **It has no importers.** `src/components/gallery/FolderNav.tsx` inlined its own folder-creation modal instead of using this component. The Gallery has TWO implementations of the new-folder modal: the shadcn one (correct) and the inlined one (in `FolderNav.tsx`). Consolidating to use the real component is the right fix. ## Where - Shipped component: `src/components/gallery/NewFolderDialog.tsx` (95 lines, full shadcn/ui Dialog with Header/Content/Footer, Input, Button, useRef for autofocus, useEffect for reset, isDuplicate detection, isInvalid guard) - Inlined duplicate: `src/components/gallery/FolderNav.tsx` (lines 122-160, the inline `<form>` for new folder creation) - Tests: `src/components/gallery/issue-112-aria.test.tsx` (180-186) already tests the `getByPlaceholderText(/folder name/i)` and `queryByPlaceholderText` behavior on the inlined form ## Suggested fix shape Replace the inlined form in `FolderNav.tsx` with `<NewFolderDialog>`. The component already accepts `open`, `onOpenChange`, `existingFolders`, `onConfirm` — all the props the inlined form needs. The wiring is mechanical: 1. `FolderNav.tsx`: replace the local `useState` for "showing the new folder form" with a `dialogOpen` boolean 2. Replace the inlined form with `<NewFolderDialog open={dialogOpen} onOpenChange={setDialogOpen} existingFolders={folders.map(f => f.name)} onConfirm={handleAdd} />` 3. Delete the local form state and the form JSX 4. Update `issue-112-aria.test.tsx` to query the new dialog structure (the `getByPlaceholderText` should still work since `NewFolderDialog` uses `<Input placeholder="Folder name...">`) The benefits: - Duplicate-name detection that's already in `NewFolderDialog` (currently missing from `FolderNav`'s inline form — silent name collisions possible) - Proper shadcn/ui focus management, escape-to-close, backdrop click - A single source of truth for new-folder behavior ## Severity Medium. The inlined form works but lacks the duplicate-name detection the real component has. Two users creating "Holiday Designs" would silently produce two folders. ## Discovered by Gap analysis (2026-10-02) of unreached components. ## Related - `src/components/gallery/issue-112-aria.test.tsx` — tests the inlined form, will need updating to the dialog structure - Round 1 fix commit `1788bdc` — "fix(gallery): replace window.prompt 'New Folder' with an inline modal" — that fix was correct (window.prompt was worse), but the inlined version is now superseded by the real component.

Mark added the

feature-request

triage-approved

labels

2026-10-03 03:58:46 +00:00

Mark referenced this issue from a commit

2026-10-03 04:40:44 +00:00

fix(gallery): use shipped NewFolderDialog shadcn component instead of inlined form (#134)

Mark closed this issue

2026-10-03 04:40:44 +00:00

Mark commented

2026-10-03 04:40:54 +00:00

Author

Owner

Closed by commit a6741ad on origin/main (fix(gallery): use shipped NewFolderDialog shadcn component instead of inlined form).

Replaced FolderNav's inlined new-folder form with the shipped NewFolderDialog component (the one that already existed but had no importers). The new dialog adds the duplicate-name detection that the inline form was missing.

Tests:

src/components/gallery/issue-112-aria.test.tsx: 7/7 pass (updated placeholder query)
src/pages/Gallery.test.tsx: 12/12 pass (two pre-existing test-rot failures that exercised the dead inline form now exercise the real dialog)

Full vitest suite: 539/559 pass — 20 remaining failures are pre-existing (ExportDialog getThreadCatalog mock gap, SettingsModal thread-library count assertions, issue-128 right-panel testids) unrelated to #134.

Closed by commit a6741ad on origin/main (fix(gallery): use shipped NewFolderDialog shadcn component instead of inlined form). Replaced FolderNav's inlined new-folder form with the shipped NewFolderDialog component (the one that already existed but had no importers). The new dialog adds the duplicate-name detection that the inline form was missing. Tests: - src/components/gallery/issue-112-aria.test.tsx: 7/7 pass (updated placeholder query) - src/pages/Gallery.test.tsx: 12/12 pass (two pre-existing test-rot failures that exercised the dead inline form now exercise the real dialog) Full vitest suite: 539/559 pass — 20 remaining failures are pre-existing (ExportDialog getThreadCatalog mock gap, SettingsModal thread-library count assertions, issue-128 right-panel testids) unrelated to #134.

Mark referenced this issue

2026-10-03 05:45:15 +00:00

Gallery has search and view toggle but no sort, no date range, no favorites quick-filter, and Design has no updatedAt field — large galleries are unmanageable #151

Mark referenced this issue

2026-10-03 17:30:29 +00:00

Settings modal lacks focus trap — has role="dialog" and aria-modal="true" but no focus management, so Tab can move focus to the underlying page #168