[2/7] Extract a shared Modal component and migrate all five dialogs

[alex-clickhouse]

Stacked on #272 — base is alex-clickhouse/task-board-api, so the diff here is only the Modal work. GitHub retargets this to main when #272 merges.

Groundwork for the task board, but it stands on its own: the board needs a dialog primitive, and the app didn't have one.

Why

Five dialogs each hand-rolled the same backdrop:

	Escape	Focus trap	Scroll lock	role="dialog"
ShortcutsModal	✅	❌	❌	❌
TaskCreateDialog	❌	❌	❌	❌
TaskStatusManager	❌	❌	❌	❌
SkillsPage	❌	❌	❌	❌
SkillDetailPage	❌	❌	❌	❌

Two had also drifted onto a different border colour and corner radius than the other three.

Three details that aren't boilerplate

Escape is handled in the capture phase. App.tsx and ChatPage.tsx install document-level shortcut handlers that also claim Escape — to clear a search box, to stop generation. A bubble-phase listener fires after those, so dismissing a dialog could also stop a generation running behind it. ShortcutsModal already knew this and carried a capture-phase handler with a comment explaining why; that reasoning now covers all five.

Only the topmost dialog reacts. Every open Modal installs a listener, so without a stack one Escape would close a confirmation and the dialog that raised it. The same refcount stops an inner dialog from restoring body scroll while an outer one is still open.

The backdrop closes on mousedown, not click. A click fires on the backdrop when a drag that started inside the panel — selecting text in a textarea — is released outside it. On click that throws the dialog away mid-interaction.

Also fixed

• Focus moves into the panel on open and returns to where it came from on close. Previously, dismissing a dialog restarted keyboard navigation at the top of the document.
• The two dialogs holding real typing (new task, new skill) no longer discard it on a stray backdrop click.
• First prefers-reduced-motion handling in the app — the new modal animation plus the transform-based ones already in index.css.

Notes for review

• The footer slot takes a layout override (footerClassName) because TaskStatusManager's footer is a form, not a button row. Everything else uses the default right-aligned row.
• TaskCreateDialog's submit button sits in the footer but stays wired to the form via form={id}, so Enter-to-submit still works from any field.
• Migrating SkillsPage/SkillDetailPage unifies them onto the house border-border-subtle + rounded-xl — a small deliberate visual change, since a shared component with two looks defeats the point.
• First createPortal in the app; dialogs now render at document.body instead of inside whatever happened to contain them.

Testing

• npx tsc -b — clean
• npm run build — clean
• npx eslint — unchanged; the 1 error + 5 warnings in these files are all pre-existing and untouched
• 19 behavioural specs for this component — they live in [3/7] Add frontend test infrastructure (Vitest + Testing Library) #274, which adds the Vitest harness this repo didn't have

Interaction behaviour is the whole point of this component and none of it is reachable by a type checker, so it's asserted rather than eyeballed: portal target, ARIA semantics, Escape (including that it does not leak to the app-level handlers), backdrop dismissal, the drag-release-outside case, focus entry/wrap/restore, stacked-dialog Escape isolation, and scroll-lock refcounting.

Writing those specs caught a bug in the first version of this PR, fixed here in be57e5f:

The scroll lock read modalStack.length to decide if it was the last dialog out, but React runs effect cleanups in declaration order — so the closing dialog was still in the stack, the count never reached 0, and overflow: hidden was never lifted. Closing any dialog left the page unscrollable. tsc and vite build were both clean, and the page looks entirely normal until you try to scroll.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Introduces a shared Modal UI primitive for the web app and migrates the existing dialogs to use it, consolidating dialog behavior (Escape handling, focus management, scroll lock, and portal rendering) into a single implementation.

Changes:

• Added web/src/components/ui/Modal.tsx implementing a portal-based dialog with capture-phase Escape handling, focus trap/restore, and scroll locking with refcounting.
• Migrated ShortcutsModal, TaskCreateDialog, TaskStatusManager, SkillsPage create dialog, and SkillDetailPage delete confirmation to the shared Modal.
• Added modal entrance animations and prefers-reduced-motion handling in web/src/index.css.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
web/src/components/ui/Modal.tsx	New shared modal primitive (portal, Escape capture, focus trap/restore, scroll lock, sizing, header/footer slots).
web/src/components/ShortcutsModal.tsx	Replaced custom overlay/Escape handling with Modal.
web/src/components/Tasks/TaskCreateDialog.tsx	Migrated task creation dialog to Modal and moved submit button into modal footer via form association.
web/src/components/Tasks/TaskStatusManager.tsx	Migrated status manager to Modal, using footer slot for the pinned add-status UI.
web/src/pages/SkillsPage.tsx	Migrated “New Skill” dialog to Modal and disabled backdrop-close to prevent data loss.
web/src/pages/SkillDetailPage.tsx	Migrated delete confirmation to Modal.
web/src/index.css	Added modal animations plus reduced-motion overrides.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.