Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/components/CommandPalette.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
Bot,
TextAlignStart,
Save,
Keyboard,
} from "lucide-react";
import { DatabaseConnection, SavedQuery, QueryHistoryItem } from "@/lib/types";
import { relationObjects, type DetailedObject } from "@/lib/db/detailed-object";
Expand Down Expand Up @@ -64,6 +65,8 @@ interface CommandPaletteProps {
* shell declines to offer elsewhere too (`MobileNav.onOpenAgent`).
*/
onAskAgent?: () => void;
/** Opens the standalone shell's `ShortcutsDialog` instance (#746). */
onShowShortcuts: () => void;
onLogout: () => void;
}

Expand All @@ -84,6 +87,7 @@ export function CommandPalette({
onFormatQuery,
onSaveQuery,
onAskAgent,
onShowShortcuts,
onLogout,
}: CommandPaletteProps) {
const [open, setOpen] = useState(false);
Expand Down Expand Up @@ -142,6 +146,11 @@ export function CommandPalette({
<Save strokeWidth={1.5} className="w-3.5 h-3.5 text-fg-tertiary" />
<span>Save Current Query</span>
</CommandItem>
<CommandItem onSelect={() => runAction(onShowShortcuts)}>
<Keyboard strokeWidth={1.5} className="w-3.5 h-3.5 text-fg-tertiary" />
<span>Keyboard Shortcuts</span>
<CommandShortcut>?</CommandShortcut>
</CommandItem>
{onAskAgent && (
/*
Named for the ask, not for the surface: `MobileNav` has a control
Expand Down
395 changes: 208 additions & 187 deletions src/components/DataProfiler.tsx

Large diffs are not rendered by default.

152 changes: 152 additions & 0 deletions src/components/ShortcutsDialog.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
"use client";

import React, { forwardRef, useCallback, useEffect, useImperativeHandle, useRef, useSyncExternalStore } from "react";
import { Dialog, DialogContent, DialogHeader, DialogTitle } from "@/components/ui/dialog";
import { SHORTCUT_GROUPS } from "@/lib/shortcuts";

export interface ShortcutsDialogRef {
open: () => void;
}

/**
* Open state lives at module scope rather than in this component's own `useState` (#746
* review): `Studio.tsx` mounts one instance unconditionally and `DataProfiler.tsx` mounts a
* second whenever it's open, so in the standalone shell with the profiler open BOTH are
* mounted at once. Two independent `useState`s would mean two independent dialogs — "?"
* opening both, and one Escape closing only the topmost, leaving the other (and the profiler
* underneath) still up. A shared store fixes the STATE half of that; `primaryInstanceKey`
* below fixes the other half, which instance actually renders the `Dialog`.
*/
let sharedOpen = false;
const openListeners = new Set<() => void>();

function setSharedOpen(next: boolean): void {
if (sharedOpen === next) return;
sharedOpen = next;
openListeners.forEach((listener) => listener());
}

function subscribeOpen(callback: () => void): () => void {
openListeners.add(callback);
return () => openListeners.delete(callback);
}

function getOpenSnapshot(): boolean {
return sharedOpen;
}

function getServerOpenSnapshot(): boolean {
return false;
}

/**
* Whichever instance mounts first renders the `Dialog`; a later one sharing the tree (the
* standalone shell's Studio-level instance is always first in practice, since `DataProfiler`
* mounts only once the profiler opens) shares the same open flag but renders nothing, so
* there is exactly one `Dialog` no matter how many instances are mounted at once. The
* mutation lives inside `subscribePrimary`, which `useSyncExternalStore` calls from its own
* effect — not inside an effect of this component's — so this never calls setState from
* render or from an effect body of its own; `isPrimary` is a pure read of external state,
* the same shape `useFavoriteConnections`/`useConnectionOrder` already use for this reason.
*/
let primaryInstanceKey: object | null = null;
const mountedInstances = new Map<object, () => void>();

function subscribePrimary(key: object, callback: () => void): () => void {
mountedInstances.set(key, callback);
if (primaryInstanceKey === null) primaryInstanceKey = key;
return () => {
mountedInstances.delete(key);
if (primaryInstanceKey === key) {
// Promote whichever instance is still mounted, if any, so it starts rendering the
// Dialog. In practice this is Studio.tsx's own instance outliving DataProfiler's, not
// the reverse, but nothing here assumes that ordering.
const [nextKey] = mountedInstances.keys();
primaryInstanceKey = nextKey ?? null;
}
// Nothing left to show it to - and in the embedded workspace, DataProfiler's is the
// only instance there is, so this is what closes the dialog on the profiler's own
// unmount rather than leaving a stale "open" flag for the next time it mounts.
if (mountedInstances.size === 0) setSharedOpen(false);
mountedInstances.forEach((listener) => listener());
};
}

function isTypingTarget(target: EventTarget | null): boolean {
if (!(target instanceof HTMLElement)) return false;
if (target instanceof HTMLInputElement || target instanceof HTMLTextAreaElement) return true;
if (target.isContentEditable) return true;
// Monaco 0.56 focuses a `div.native-edit-context`, not a textarea or a contentEditable
// element, so neither check above sees it - and `?` is the positional-parameter
// placeholder in SQLite and MySQL, so missing this let the dialog eat the keystroke
// mid-query. `.monaco-editor` is Monaco's own stable root class, not an internal we're
// reaching past: checking "inside the editor at all" survives Monaco changing which
// element it focuses next, where chasing that element by name would not.
return target.closest(".monaco-editor") !== null;
}

/**
* The single place that answers "what shortcuts exist" (#746). Self-contained, following
* `CommandPalette`'s own Cmd/Ctrl+K effect: every instance owns its own "?" listener, so
* mounting it in both `Studio.tsx` and `DataProfiler.tsx` — `DataProfiler` is itself mounted
* by both the standalone shell and the embedded workspace — is what makes the dialog reachable
* everywhere without either host threading open state through props. What's shared across
* instances (module scope, above) is the open flag itself and which one actually renders.
*
* `CommandPalette`'s "Keyboard Shortcuts" entry reaches the standalone shell's instance
* through the imperative handle below, the same seam `QueryEditorRef` already uses for the
* editor. Its `open()` writes the shared flag, so it opens whichever instance is currently
* rendering the dialog regardless of which one the ref happens to be attached to.
*/
export const ShortcutsDialog = forwardRef<ShortcutsDialogRef>(function ShortcutsDialog(_props, ref) {
const instanceKey = useRef<object>({}).current;
const subscribe = useCallback((callback: () => void) => subscribePrimary(instanceKey, callback), [instanceKey]);
const getIsPrimary = useCallback(() => primaryInstanceKey === instanceKey, [instanceKey]);
const rendersDialog = useSyncExternalStore(subscribe, getIsPrimary, () => false);

const open = useSyncExternalStore(subscribeOpen, getOpenSnapshot, getServerOpenSnapshot);

useImperativeHandle(ref, () => ({ open: () => setSharedOpen(true) }), []);

useEffect(() => {
const handleKeyDown = (e: KeyboardEvent) => {
if (e.key !== "?" || isTypingTarget(e.target)) return;
e.preventDefault();
setSharedOpen(true);
};
document.addEventListener("keydown", handleKeyDown);
return () => document.removeEventListener("keydown", handleKeyDown);
}, []);

if (!rendersDialog) return null;

return (
<Dialog open={open} onOpenChange={setSharedOpen}>
<DialogContent className="sm:max-w-md bg-surface border-hairline-strong">
<DialogHeader>
<DialogTitle>Keyboard Shortcuts</DialogTitle>
</DialogHeader>
<div className="space-y-4 max-h-[60vh] overflow-y-auto">
{SHORTCUT_GROUPS.map((group) => (
<div key={group.heading}>
<h3 className="text-xs font-medium text-fg-muted mb-2">{group.heading}</h3>
<div className="space-y-1.5">
{group.shortcuts.map((shortcut) => (
<div
key={`${group.heading}:${shortcut.keys}:${shortcut.description}`}
className="flex items-center justify-between gap-3 text-xs"
>
<span className="text-fg">{shortcut.description}</span>
<kbd className="px-1.5 py-0.5 rounded bg-fill text-fg-secondary font-mono text-[0.7rem] shrink-0">
{shortcut.keys}
</kbd>
</div>
))}
</div>
</div>
))}
</div>
</DialogContent>
</Dialog>
);
});
10 changes: 9 additions & 1 deletion src/components/Studio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { SchemaExplorer } from "@/components/schema-explorer";
import { ConnectionModal } from "@/components/ConnectionModal";
import { CommandPalette } from "@/components/CommandPalette";
import { QueryEditor, QueryEditorRef } from "@/components/QueryEditor";
import { ShortcutsDialog, type ShortcutsDialogRef } from "@/components/ShortcutsDialog";
import { DataImportModal } from "@/components/DataImportModal";
import { QuerySafetyDialog } from "@/components/QuerySafetyDialog";
import { DataProfiler } from "@/components/DataProfiler";
Expand Down Expand Up @@ -100,6 +101,7 @@ const SchemaDiagram = React.lazy(

export default function Studio() {
const queryEditorRef = useRef<QueryEditorRef>(null);
const shortcutsDialogRef = useRef<ShortcutsDialogRef>(null);
const router = useRouter();
const { toast } = useToast();

Expand Down Expand Up @@ -303,7 +305,7 @@ export default function Studio() {
* because an unmeasured "nothing else can reach this" is the mistake D82 was filed over. The
* dialog refuses every exit IT owns: while `applying` it withholds its close button and prevents
* Escape, a press outside and every other interaction outside. What it cannot refuse is a global
* listener, and `grep -rE 'addEventListener\(\s*"keydown' src` answers FOUR, of which TWO can
* listener, and `grep -rE 'addEventListener\(\s*"keydown' src` answers FIVE, of which TWO can
* move the active tab here:
*
* - `src/components/studio/StudioTabBar.tsx`, on `document`: the new-tab shortcut (#745).
Expand All @@ -314,6 +316,9 @@ export default function Studio() {
* own `onClose`. It moves no tab. An earlier form of this paragraph said there were two
* listeners and missed it, which is the unmeasured-absence mistake D82 was filed over, so it is
* named here rather than left out for being harmless.
* - `src/components/ShortcutsDialog.tsx`, on `document` (#746), and MOUNTED BY THIS SHELL. It
* answers `?` alone (guarded against the editor and every text input), opens a dialog that
* reads shortcut labels and closes itself, and moves no tab.
* - `src/components/ui/sidebar.tsx`, on `window`, toggling a sidebar. An unused shadcn primitive
* with no importer anywhere in `src` (P5), so it is mounted nowhere.
*
Expand Down Expand Up @@ -1470,9 +1475,12 @@ export default function Studio() {
onFormatQuery={() => queryEditorRef.current?.format()}
onSaveQuery={() => setIsSaveQueryModalOpen(true)}
onAskAgent={agentEnabled ? askAgentAboutStatement : undefined}
onShowShortcuts={() => shortcutsDialogRef.current?.open()}
onLogout={handleLogout}
/>

<ShortcutsDialog ref={shortcutsDialogRef} />

<MobileNav
activeTab={activeMobileTab}
onTabChange={setActiveMobileTab}
Expand Down
71 changes: 71 additions & 0 deletions src/lib/shortcuts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { SHORTCUTS, shortcutLabel } from "@/lib/keyboard-shortcuts";

export interface ShortcutEntry {
keys: string;
description: string;
}

export interface ShortcutGroup {
heading: string;
shortcuts: ShortcutEntry[];
}

/**
* The one place that answers "what shortcuts exist" (#746).
*
* `SHORTCUTS`/`shortcutLabel` from `@/lib/keyboard-shortcuts` cover every binding that also
* feeds a Monaco keybinding or a `matchesShortcut` check — importing their labels here rather
* than retyping them is what keeps this list from drifting the way `docs/FEATURES.md` no
* longer can (`bun run shortcuts:sync` covers that file, not this one, hence this import).
*
* The rows below that are NOT drawn from `SHORTCUTS` (`?` itself, tab-strip arrow navigation,
* the data profiler's Escape) are display-only: none of them is a Monaco command, so folding
* them into a registry whose whole shape exists to feed `monacoKeybinding` would either force
* a synthetic key code onto something that will never be one, or weaken the registry's typing
* for every real entry to accommodate them. `?` in particular must stay outside it structurally,
* not just by convention — it is a document-level listener that has to EXCLUDE the editor
* (`?` is a live SQL placeholder character), the opposite of what belongs in a table Monaco
* reads bindings from.
*/
export const SHORTCUT_GROUPS: ShortcutGroup[] = [
{
heading: "General",
shortcuts: [
{ keys: shortcutLabel(SHORTCUTS.commandPalette), description: "Open the command palette" },
{ keys: "?", description: "Show this shortcuts dialog" },
],
},
{
heading: "Query editor",
shortcuts: [
{ keys: shortcutLabel(SHORTCUTS.executeQuery), description: "Run the current query" },
{ keys: shortcutLabel(SHORTCUTS.formatQuery), description: "Format the query" },
],
},
{
heading: "Tabs",
shortcuts: [
{ keys: shortcutLabel(SHORTCUTS.newTab), description: "Open a new query tab" },
{ keys: "Left / Right arrow", description: "Move focus between tabs" },
{ keys: "Home / End", description: "Jump to the first / last tab" },
],
},
{
heading: "Data profiler",
shortcuts: [{ keys: "Escape", description: "Close the data profiler" }],
},
{
// Display-only, same reasoning as the tab-strip arrows above: `ObjectTree.tsx`'s
// `onKeyDown` (bound on the `role="tree"` root) implements this itself against the
// W3C tree pattern, none of it is a Monaco command, and it only runs while the tree
// itself has focus - there is nothing here for `SHORTCUTS`/`monacoKeybinding` to hold.
heading: "Object tree",
shortcuts: [
{ keys: "Up / Down arrow", description: "Move focus between rows" },
{ keys: "Left / Right arrow", description: "Collapse / expand the focused row" },
{ keys: "Home / End", description: "Jump to the first / last row" },
{ keys: "Enter / Space", description: "Open the focused row" },
{ keys: "Shift+F10 / Menu key", description: "Open the row's context menu" },
],
},
];
5 changes: 4 additions & 1 deletion src/workspace/StudioWorkspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -667,14 +667,17 @@ export function StudioWorkspace({
* leaves the same two open, so the two shells do not disagree.
*
* THE KEYDOWN PASS. What the dialog cannot refuse is a global listener, and
* `grep -rE 'addEventListener\(\s*"keydown' src` answers FOUR, of which exactly one can move
* `grep -rE 'addEventListener\(\s*"keydown' src` answers FIVE, of which exactly one can move
* the active tab here:
*
* - `src/components/studio/StudioTabBar.tsx:115`, on `document`: the new-tab shortcut, which is
* the one this handler guards. It opens a tab and `addTab` activates it.
* - `src/components/DataProfiler.tsx:210`, on `document`, and MOUNTED BY THIS SHELL below. It is
* bound only while the profiler is open, it answers Escape alone, and all it does is call the
* profiler's `onClose`. It moves no tab, and it cannot unmount this pane.
* - `src/components/ShortcutsDialog.tsx:117`, on `document` (#746), and MOUNTED BY THIS SHELL
* indirectly - `DataProfiler.tsx` always renders one while it is open. It answers `?` alone,
* opens a dialog of shortcut labels, and moves no tab.
* - `src/components/CommandPalette.tsx:103`, on `document`, whose table rows call
* `handleTableClick` and DO move the active tab. That is the standalone shell's second gesture
* in D82, and this shell renders no palette. It is still the reason `onTableClick` above now
Expand Down
14 changes: 14 additions & 0 deletions tests/components/CommandPalette.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ function createDefaultProps(overrides: Partial<Parameters<typeof CommandPalette>
onFormatQuery: mock(() => {}),
onSaveQuery: mock(() => {}),
onAskAgent: mock(() => {}),
onShowShortcuts: mock(() => {}),
onLogout: mock(() => {}),
...overrides,
};
Expand Down Expand Up @@ -446,6 +447,19 @@ describe("CommandPalette", () => {
fireEvent.click(saveItem!);
});

test("Keyboard Shortcuts action callback fires via runAction", () => {
const onShowShortcuts = mock(() => {});
const props = createDefaultProps({ onShowShortcuts });
const { getByText } = render(<CommandPalette {...props} />);

// Open dialog
fireEvent.keyDown(document, { key: "k", metaKey: true });

const shortcutsItem = getByText("Keyboard Shortcuts").closest('[role="option"]');
expect(shortcutsItem).not.toBeNull();
fireEvent.click(shortcutsItem!);
});

/**
* The item names the agent because the in-editor assistant it used to open no
* longer exists (#331 T3), and names the QUERY because the ask is about the
Expand Down
Loading
Loading