Feature/confirm dialog

[sharppaul]

23cc6b5 Replace the native dialogs with a Confirm component
dc1d0b8 Confirm before a reset of all settings

[Copilot]

🟡 Changes recommended

Unresolved dialog accessibility and table-selection issues remain, and the system-info flow can proceed with a null node.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR replaces native dialogs with a shared asynchronous confirmation component.

Changes:

• Adds global confirmation state and dialog rendering.
• Confirms settings resets, query actions, node changes, and deletions.
• Replaces native dialogs across system-info and query flows.

File summaries

File	Summary
src/routes/system-info/+page.svelte	Replaces the system-info alert.
src/routes/settings/+page.svelte	Confirms settings reset.
src/routes/+layout.svelte	Mounts the shared dialog.
src/lib/stores/confirm.ts	Provides confirmation state and APIs.
src/lib/components/query-builder/QueryWorkspace.svelte.ts	Confirms node and query changes.
src/lib/components/query-builder/QueryWorkbenchPanes.svelte	Handles asynchronous node changes.
src/lib/components/query-builder/QueryBuilderTableSelector.svelte	Confirms table changes.
src/lib/components/query-builder/QueryBuilderSelectorBlock.svelte	Handles asynchronous block closure.
src/lib/components/modals/Confirm.svelte	Renders the confirmation dialog.
src/lib/components/modals/AddBeaconModal.svelte	Confirms close and delete actions.

Review details

Suppressed comments (1)

src/lib/components/modals/Confirm.svelte:59

• This dialog never moves or traps focus. Keyboard users can activate controls behind the question despite aria-modal. Move focus into the dialog and restore it on close.

{#if $confirmRequest}
	<div class="confirm-layer">
		<Modal title={$confirmRequest.title} onClose={() => answerConfirm(false)} width="440px">
			<p>{$confirmRequest.message}</p>

			{#if $confirmRequest.note}
				<p class="note">{$confirmRequest.note}</p>
			{/if}

			<div slot="footer" class="confirm-actions">
				{#if $confirmRequest.cancelLabel !== null}
					<Button variant="outline" onclick={() => answerConfirm(false)}>
						{$confirmRequest.cancelLabel ?? 'Cancel'}
					</Button>
				{/if}

				<Button
					variant={$confirmRequest.destructive ? 'destructive' : 'default'}
					onclick={() => answerConfirm(true)}
				>
					{$confirmRequest.confirmLabel ?? 'Continue'}
				</Button>
			</div>
		</Modal>

• Files reviewed: 10/10 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Resolve duplicate Escape handling and add focus management and keyboard trapping to the confirmation dialog.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/lib/components/modals/Confirm.svelte:38

• This dialog never moves focus into the dialog or traps Tab. A keyboard user can tab to controls behind the backdrop. Move focus to an action on open, trap Tab, and restore focus after close.

src/lib/components/modals/AddBeaconModal.svelte:71

• This component still registers a document Escape handler, while Modal also calls onClose for Escape. With this async implementation, one Escape on a dirty form calls closeModal twice and the second askConfirm replaces the first. Keep one Escape path and track dirty input separately.

			const goAhead = await askConfirm({
				title: 'Close without saving',
				message: 'This form holds changes that the app did not save yet.',
				note: 'The changes go away.',
				confirmLabel: 'Close',

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Lite