Repository navigation
feat(ui): quick-create palette with icon badges + Alt+N binding - #1085
Conversation
📝 WalkthroughWalkthroughAdds a global quick-create palette and zustand UI state; replaces multiple local CreatePageDialog usages with store-driven flows; adds a hotkey and dashboard integration; introduces a standalone prototype for the quick-create UI. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UIStore as UIStore\n(useUIStore)
participant QuickCreate as QuickCreatePalette\n(Component)
participant API as API\n(/api/upload, /api/pages)
participant SWR as SWR\n(cache/mutate)
participant Router as Router\n(next/navigation)
User->>UIStore: openQuickCreate(parentId?)
UIStore-->>QuickCreate: quickCreateOpen=true\nparentOverride=parentId
QuickCreate->>User: show type selection (phase: type-select)
User->>QuickCreate: select type
QuickCreate->>QuickCreate: set name / phase -> name-entry
alt type == FILE
User->>QuickCreate: choose file
QuickCreate->>API: POST /api/upload (FormData)
API-->>QuickCreate: return new page
else other type
QuickCreate->>API: POST /api/pages (title,type,content)
API-->>QuickCreate: return new page
end
QuickCreate->>SWR: mutate('/api/drives/.../pages') / refresh cache
QuickCreate->>UIStore: closeQuickCreate()
QuickCreate->>Router: navigate to new page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e947b4a0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const rawPageId = params?.pageId; | ||
| const driveId = Array.isArray(rawDriveId) ? rawDriveId[0] : (rawDriveId ?? undefined); | ||
| const pageId = Array.isArray(rawPageId) ? rawPageId[0] : (rawPageId ?? undefined); |
There was a problem hiding this comment.
Derive current page from files route params
This palette only reads params.pageId, but the Files explorer route (/dashboard/[driveId]/files/[[...path]]) exposes the active folder/page as params.path[0]. In that route, quick-create opened via Cmd+N or the sidebar + cannot infer context, so effectiveParentId falls back to null and new pages are created at drive root instead of the folder being viewed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b05fffc. Now reads params.path[0] as the page context when params.pageId is absent, covering the Files explorer route (/dashboard/[driveId]/files/[[...path]]). The palette will now correctly create inside the viewed folder rather than at drive root.
| closeQuickCreate(); | ||
| await navigateToPage(newPage.id, driveId); |
There was a problem hiding this comment.
Revalidate page tree before navigating to new page
After creation succeeds, the code immediately navigates without updating the /api/drives/${driveId}/pages SWR cache. Since page rendering (e.g., CenterPanel) resolves the route page from that tree cache, a delayed/missed socket event can leave the new page absent and show "Page not found in the current tree" right after create. Triggering a tree mutate (or optimistic insert) before navigation avoids this stale-cache failure mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b05fffc. Added useSWRConfig().mutate() calls for /api/drives/${driveId}/pages immediately after both page creation and file upload, before navigating. This forces the tree to refresh synchronously so the new page is present in the cache when CenterPanel renders.
Replace CreatePageDialog with a Raycast-style two-phase command palette. Cmd+N triggers from anywhere in a drive; phase 1 selects page type, phase 2 names it and shows where the page will land. Context-aware: creates a child when viewing a folder, sibling otherwise, root when no page is open. Sidebar + and context menu "Add child page" both route through the same palette. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Master added a New Page button and header layout to FilesFinderContent after this branch was created, wired to CreatePageDialog. Since CreatePageDialog is deleted in this branch, the CI merge-commit failed to resolve the import. Replace the CreatePageDialog import/state/render with openQuickCreate(currentPageId) so the Files view header New Page button opens the quick-create palette. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
8b49bfa to
f3cfb42
Compare
P1: Also read params.path[0] for the files explorer route (/dashboard/[driveId]/files/[[...path]]) so cmd+N opened from within a folder correctly infers effectiveParentId instead of falling back to drive root. P2: Mutate the SWR page-tree cache immediately after page or file creation, before navigating, so the new page appears in the tree and CenterPanel never shows "not found". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/web/src/stores/useUIStore.ts (1)
11-17: Tri-stateparentOverrideis subtle — document the contract.
quickCreateParentOverridedistinguishes three cases:undefined(no override → use context-aware placement),null(explicit drive root), andstring(explicit parent page id). This is a valid but easily-misused API — a caller writingopenQuickCreate(currentPageId)wherecurrentPageIdisstring | nullwill collapse "no page open" into "explicit root", which may or may not be intended (seeFilesFinderContent.tsxline 129).Consider either (a) adding a short JSDoc on
openQuickCreate/the field clarifying the three-state semantics, or (b) splitting into two explicit signals (e.g.,parentOverrideMode: 'context' | 'root' | 'page'plus an id) to make misuse impossible.Optional: JSDoc-only fix
+ /** + * Open the quick-create palette. + * `@param` parentOverride + * - `undefined` (or omitted): use context-aware placement based on the current route/page. + * - `null`: force creation at the drive root. + * - `string`: create as a child of the given page id. + */ openQuickCreate: (parentOverride?: string | null) => void;Also applies to: 44-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/stores/useUIStore.ts` around lines 11 - 17, Document the tri-state contract for quickCreateParentOverride and openQuickCreate: add JSDoc to the quickCreateParentOverride field and the openQuickCreate(parentOverride?: string | null) method explaining that undefined = use context-aware placement, null = explicit drive root, and string = explicit parent page id (and warn callers that passing a string | null variable may unintentionally map null → root). Optionally mention the safer alternative (split into explicit mode + id) so reviewers can later refactor; reference quickCreateParentOverride and openQuickCreate in the comments so maintainers see where the contract lives.apps/web/src/components/files/__tests__/FilesEmptyState.test.tsx (1)
14-17: Consider asserting selector-based access pattern.The mock invokes the selector synchronously, which works, but doesn't verify the component is using
useUIStoreas a selector (vs.getState()or destructured access). Minor — the existing assertion onopenQuickCreatecall at Line 88 covers the behavior. No change required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/files/__tests__/FilesEmptyState.test.tsx` around lines 14 - 17, Update the mock of useUIStore so the mocked function accepts and invokes the selector (i.e., implement useUIStore: (selector) => { record that selector was called; return selector({ openQuickCreate: hoisted.openQuickCreate }) }), and add an assertion in FilesEmptyState.test.tsx that the selector was invoked (spy/assert the selector call) to verify the component uses the selector-based access pattern; reference the existing symbols useUIStore, selector, openQuickCreate, and hoisted.openQuickCreate when making the changes.apps/web/src/components/create/QuickCreatePalette.tsx (2)
77-92: MemoizeeffectiveParentId/contextLabelto avoid per-render tree walks.Both derived values use IIFEs, so
findNodeAndParent(tree, pageId)and the breadcrumb join run on every render (including while the palette is closed, since the early return is at line 206 after these compute). For large page trees this is wasted work and also causeseffectiveParentIdto re-identity on every render, which can propagate throughuseBreadcrumbsandhandleCreate's deps. Wrapping inuseMemois a cheap, mechanical improvement.♻️ Proposed refactor
- // Derive which parent to create in - const effectiveParentId: string | null = (() => { - if (quickCreateParentOverride !== undefined) return quickCreateParentOverride; - if (!pageId || !tree) return null; - const found = findNodeAndParent(tree, pageId); - if (!found) return null; - return found.node.type === PageType.FOLDER ? found.node.id : found.node.parentId; - })(); + // Derive which parent to create in + const effectiveParentId = useMemo<string | null>(() => { + if (quickCreateParentOverride !== undefined) return quickCreateParentOverride; + if (!pageId || !tree) return null; + const found = findNodeAndParent(tree, pageId); + if (!found) return null; + return found.node.type === PageType.FOLDER ? found.node.id : found.node.parentId; + }, [quickCreateParentOverride, pageId, tree]); const { breadcrumbs } = useBreadcrumbs(effectiveParentId); - const contextLabel = (() => { - if (!effectiveParentId) return 'Drive root'; - if (!breadcrumbs?.length) return '…'; - return breadcrumbs.map((b) => b.title).join(' › '); - })(); + const contextLabel = useMemo(() => { + if (!effectiveParentId) return 'Drive root'; + if (!breadcrumbs?.length) return '…'; + return breadcrumbs.map((b) => b.title).join(' › '); + }, [effectiveParentId, breadcrumbs]);(Add
useMemoto the existingreactimport.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/create/QuickCreatePalette.tsx` around lines 77 - 92, Wrap the IIFE calculations for effectiveParentId and contextLabel in React.useMemo to avoid recomputing findNodeAndParent(tree, pageId) and the breadcrumbs join each render; add useMemo to the React import, memoize effectiveParentId using [quickCreateParentOverride, pageId, tree] as deps, memoize contextLabel using [effectiveParentId, breadcrumbs] as deps, and ensure these stable values are used by useBreadcrumbs and handleCreate so they no longer re-create on every render in QuickCreatePalette.
126-134: Minor: 100mssetTimeoutbefore clicking the file input is load-bearing but unexplained.The hidden
<input type="file">is rendered unconditionally inside the fragment (lines 210-223) wheneverquickCreateOpenis true, sofileInputRef.currentshould already be attached whenhandleSelectType(FILE)runs during phase 1 — the delay isn't needed for ref availability. If the intent is to let the Radix Dialog settle/keep focus trapping happy before opening the native file picker, a brief comment explaining why would prevent a future refactor from removing it and breaking the upload flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/create/QuickCreatePalette.tsx` around lines 126 - 134, handleSelectType uses a 100ms setTimeout to click fileInputRef when PageType.FILE, but the file input is already mounted when quickCreateOpen is true so the delay is unexplained and brittle; either remove the setTimeout and call fileInputRef.current?.click() directly, or keep the delayed invoke but replace the magic 100ms with requestAnimationFrame (or a microtask) and add a clear comment above the code explaining this is to wait for Radix Dialog focus/animation to settle before opening the native file picker; update the handleSelectType implementation accordingly and reference fileInputRef and PageType.FILE when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/create/QuickCreatePalette.tsx`:
- Around line 95-105: The Cmd+N binding in the useEffect handler cannot be
intercepted in browsers; update the quick-create hotkey to a web-safe
combination or gate it to Electron only: locate the useEffect block that defines
handler (uses getEffectiveBinding('pages.quick-create'), matchesKeyEvent,
isEditingActive, openQuickCreate) and either change the default binding returned
for 'pages.quick-create' to a browser-safe combo (e.g., Meta+K, Ctrl+K, or
Meta+Shift+N) or add a runtime check (electron/environment flag) around the
handler so Cmd+N is only registered when running in Electron, and ensure any
docs/config mention the platform limitation.
In `@apps/web/src/components/files/FilesFinderContent.tsx`:
- Line 129: The Files view is passing currentPageId into openQuickCreate which
becomes parentOverride in QuickCreatePalette and bypasses the folder-type checks
in effectiveParentId; to fix, either validate that currentPageId is a FOLDER
before calling openQuickCreate from the Button in FilesFinderContent.tsx (only
pass the id when its type is FOLDER, otherwise pass undefined) or remove the
parentOverride parameter and call openQuickCreate without arguments so
QuickCreatePalette's effectiveParentId logic (the checks around
parentOverride/effectiveParentId) determines the correct parent id; update
FilesFinderContent.tsx (the Button onClick) or QuickCreatePalette.tsx (the
parentOverride usage) accordingly.
---
Nitpick comments:
In `@apps/web/src/components/create/QuickCreatePalette.tsx`:
- Around line 77-92: Wrap the IIFE calculations for effectiveParentId and
contextLabel in React.useMemo to avoid recomputing findNodeAndParent(tree,
pageId) and the breadcrumbs join each render; add useMemo to the React import,
memoize effectiveParentId using [quickCreateParentOverride, pageId, tree] as
deps, memoize contextLabel using [effectiveParentId, breadcrumbs] as deps, and
ensure these stable values are used by useBreadcrumbs and handleCreate so they
no longer re-create on every render in QuickCreatePalette.
- Around line 126-134: handleSelectType uses a 100ms setTimeout to click
fileInputRef when PageType.FILE, but the file input is already mounted when
quickCreateOpen is true so the delay is unexplained and brittle; either remove
the setTimeout and call fileInputRef.current?.click() directly, or keep the
delayed invoke but replace the magic 100ms with requestAnimationFrame (or a
microtask) and add a clear comment above the code explaining this is to wait for
Radix Dialog focus/animation to settle before opening the native file picker;
update the handleSelectType implementation accordingly and reference
fileInputRef and PageType.FILE when making the change.
In `@apps/web/src/components/files/__tests__/FilesEmptyState.test.tsx`:
- Around line 14-17: Update the mock of useUIStore so the mocked function
accepts and invokes the selector (i.e., implement useUIStore: (selector) => {
record that selector was called; return selector({ openQuickCreate:
hoisted.openQuickCreate }) }), and add an assertion in FilesEmptyState.test.tsx
that the selector was invoked (spy/assert the selector call) to verify the
component uses the selector-based access pattern; reference the existing symbols
useUIStore, selector, openQuickCreate, and hoisted.openQuickCreate when making
the changes.
In `@apps/web/src/stores/useUIStore.ts`:
- Around line 11-17: Document the tri-state contract for
quickCreateParentOverride and openQuickCreate: add JSDoc to the
quickCreateParentOverride field and the openQuickCreate(parentOverride?: string
| null) method explaining that undefined = use context-aware placement, null =
explicit drive root, and string = explicit parent page id (and warn callers that
passing a string | null variable may unintentionally map null → root).
Optionally mention the safer alternative (split into explicit mode + id) so
reviewers can later refactor; reference quickCreateParentOverride and
openQuickCreate in the comments so maintainers see where the contract lives.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66fb4b79-8169-4c88-afad-ab3417ba0cb6
📒 Files selected for processing (10)
apps/web/src/app/dashboard/DashboardLayoutClient.tsxapps/web/src/components/create/QuickCreatePalette.tsxapps/web/src/components/files/FilesEmptyState.tsxapps/web/src/components/files/FilesFinderContent.tsxapps/web/src/components/files/__tests__/FilesEmptyState.test.tsxapps/web/src/components/layout/left-sidebar/CreatePageDialog.tsxapps/web/src/components/layout/left-sidebar/index.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsxapps/web/src/lib/hotkeys/registry.tsapps/web/src/stores/useUIStore.ts
💤 Files with no reviewable changes (1)
- apps/web/src/components/layout/left-sidebar/CreatePageDialog.tsx
Meta+N is reserved by web browsers for "new window" and cannot be overridden via preventDefault. Gate the keydown listener to isElectron() so web users are not surprised by a new browser window. Web users can still create pages via the UI buttons or by remapping the hotkey in Settings. FilesFinderContent was passing currentPageId as parentOverride to openQuickCreate(), bypassing the folder-type check in effectiveParentId. Remove the argument so the palette reads params.path[0] from the URL and applies the standard logic: non-folder pages use their parentId, folders accept children. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Wrap effectiveParentId and contextLabel in useMemo to avoid per-render tree walks when palette is closed - Add JSDoc on quickCreateParentOverride explaining the three-state semantics (undefined/null/string) to prevent caller misuse - Add comment on the 100ms setTimeout explaining Radix Dialog animation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Addressed nitpick suggestions from the latest review:
|
Meta+N is reserved by browsers and can't be overridden. Alt+N fires on both web and Electron, so the isElectron() gate is no longer needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add colored lucide icon badges per page type (matching sidebar icons) - Add ⌥N kbd hint to search input row - Replace emoji in phase-2 header with icon badge - Add prototype in prototypes/pagespace-quick-create/ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
apps/web/src/components/create/QuickCreatePalette.tsx (1)
230-234: Error toast fallbacks are inconsistent and can surface empty messages.Two small rough edges on the catch paths:
- Line 231:
toast.error((error as Error).message)— ifmessageis empty (e.g., athrow new Error()or a non-Error thrown value coerced throughas Error), the toast shows nothing. No fallback.- Line 256:
toast.error((error as Error).message ?? 'Failed to create page')—??only triggers onnull/undefined, not empty strings, so this fallback is weaker than it looks.Consider normalizing both paths:
🛠 Proposed fix
- } catch (error) { - toast.error((error as Error).message); + } catch (error) { + const msg = error instanceof Error && error.message ? error.message : 'Failed to upload file'; + toast.error(msg); }- } catch (error) { - toast.error((error as Error).message ?? 'Failed to create page'); + } catch (error) { + const msg = error instanceof Error && error.message ? error.message : 'Failed to create page'; + toast.error(msg); }Also applies to: 255-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/create/QuickCreatePalette.tsx` around lines 230 - 234, The catch blocks use (error as Error).message (and a ?? fallback) which can produce empty toasts for empty-string messages; update both catch paths in QuickCreatePalette.tsx to normalize the error text before calling toast.error by extracting a safe message (e.g., if error is an Error use error.message.trim(), else String(error)), then if that result is empty use a hard fallback like 'Failed to create page'; replace the direct toast.error((error as Error).message) and toast.error((error as Error).message ?? 'Failed to create page') calls with toast.error(safeMessage) and keep setIsCreating(false) in the finally block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/create/QuickCreatePalette.tsx`:
- Around line 162-172: The keyboard handler opens the palette even when it's
already open, which can reset the parent context; update the handler in
QuickCreatePalette to first check that quickCreateOpen is false before calling
openQuickCreate (i.e., guard the condition with !quickCreateOpen) so the hotkey
becomes a no-op when the palette is open, and add quickCreateOpen to the
useEffect dependency array; reference getEffectiveBinding, matchesKeyEvent,
isEditingActive, driveId, openQuickCreate and quickCreateOpen to locate the
logic to change.
In `@prototypes/pagespace-quick-create/src/components/QuickCreate.tsx`:
- Around line 324-370: Palette's local lastCreated state is lost when onClose
unmounts the component, preventing SuccessToast from ever showing; move the
lastCreated state and its setter into the parent QuickCreate and render
SuccessToast from QuickCreate so the toast outlives the dialog, then change
Palette to accept an onCreated (or setLastCreated) prop and call that from
handleCreate instead of calling its local setLastCreated, and remove the local
lastCreated/SucessToast from Palette; ensure handleCreate still calls onClose
after invoking the parent's onCreated.
- Around line 379-386: The keyboard handler in the useEffect (handler) checks
e.key for Alt+N which fails on macOS due to dead keys; change the check to use
e.code === "KeyN" (still checking e.altKey) and keep the Escape logic as-is,
then re-register the same handler via
window.addEventListener/window.removeEventListener so the Alt+N hotkey works
reliably across layouts.
In `@prototypes/pagespace-quick-create/src/styles/global.css`:
- Line 37: The font-family declaration uses quoted "Inter" which violates the
stylelint rule; update the font-family property (the font-family line in
global.css) to remove the quotes around Inter so it reads Inter, ui-sans-serif,
system-ui, -apple-system, sans-serif to satisfy the font-family-name-quotes
rule.
---
Nitpick comments:
In `@apps/web/src/components/create/QuickCreatePalette.tsx`:
- Around line 230-234: The catch blocks use (error as Error).message (and a ??
fallback) which can produce empty toasts for empty-string messages; update both
catch paths in QuickCreatePalette.tsx to normalize the error text before calling
toast.error by extracting a safe message (e.g., if error is an Error use
error.message.trim(), else String(error)), then if that result is empty use a
hard fallback like 'Failed to create page'; replace the direct
toast.error((error as Error).message) and toast.error((error as Error).message
?? 'Failed to create page') calls with toast.error(safeMessage) and keep
setIsCreating(false) in the finally block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20906da0-4521-43c4-80fb-c38839839b3e
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprototypes/pagespace-quick-create/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
apps/web/src/components/create/QuickCreatePalette.tsxprototypes/pagespace-quick-create/index.htmlprototypes/pagespace-quick-create/package.jsonprototypes/pagespace-quick-create/src/App.tsxprototypes/pagespace-quick-create/src/components/QuickCreate.tsxprototypes/pagespace-quick-create/src/main.tsxprototypes/pagespace-quick-create/src/styles/global.cssprototypes/pagespace-quick-create/tsconfig.jsonprototypes/pagespace-quick-create/vite.config.ts
✅ Files skipped from review due to trivial changes (4)
- prototypes/pagespace-quick-create/src/App.tsx
- prototypes/pagespace-quick-create/index.html
- prototypes/pagespace-quick-create/package.json
- prototypes/pagespace-quick-create/tsconfig.json
| function Palette({ crumbs, onClose }: { crumbs: BreadcrumbItem[]; onClose: () => void }) { | ||
| const [phase, setPhase] = useState<"type-select" | "name-entry">("type-select"); | ||
| const [selectedType, setSelectedType] = useState<PageType | null>(null); | ||
| const [lastCreated, setLastCreated] = useState<{ type: PageType; name: string } | null>(null); | ||
|
|
||
| const handleSelect = (type: PageType) => { setSelectedType(type); setPhase("name-entry"); }; | ||
| const handleCreate = (name: string) => { | ||
| if (!selectedType) return; | ||
| setLastCreated({ type: selectedType, name }); | ||
| onClose(); | ||
| }; | ||
|
|
||
| return ( | ||
| <> | ||
| {/* Backdrop */} | ||
| <div onClick={onClose} style={{ position: "fixed", inset: 0, background: "oklch(0 0 0 / 0.5)", backdropFilter: "blur(2px)", zIndex: 10 }} /> | ||
|
|
||
| {/* Dialog shell — DialogContent "overflow-hidden p-0" + max-w-[480px] */} | ||
| <div | ||
| role="dialog" | ||
| aria-modal="true" | ||
| style={{ | ||
| position: "fixed", top: "18%", left: "50%", transform: "translateX(-50%)", | ||
| width: "calc(100vw - 32px)", maxWidth: 480, | ||
| background: "var(--popover)", color: "var(--popover-foreground)", | ||
| border: "1px solid var(--border)", | ||
| borderRadius: "var(--radius)", | ||
| boxShadow: "var(--shadow-elevated)", | ||
| overflow: "hidden", | ||
| zIndex: 20, | ||
| animation: "popIn 140ms ease", | ||
| }} | ||
| > | ||
| {phase === "type-select" && ( | ||
| <TypeSelect crumbs={crumbs} onSelect={handleSelect} onClose={onClose} /> | ||
| )} | ||
| {phase === "name-entry" && selectedType && ( | ||
| <NameEntry type={selectedType} crumbs={crumbs} onBack={() => setPhase("type-select")} onCreate={handleCreate} onClose={onClose} /> | ||
| )} | ||
| </div> | ||
|
|
||
| {lastCreated && ( | ||
| <SuccessToast type={lastCreated.type} name={lastCreated.name} onDone={() => setLastCreated(null)} /> | ||
| )} | ||
| </> | ||
| ); | ||
| } |
There was a problem hiding this comment.
SuccessToast never renders — lastCreated state is unmounted with Palette.
handleCreate on Line 330-334 calls setLastCreated(...) and then onClose(). Since onClose flips open to false in the parent QuickCreate, React batches both updates and unmounts Palette on the next render — discarding its local lastCreated state before SuccessToast ever renders. The AI summary describes a 2s success toast on confirm, but with this wiring users will never see it.
Lift lastCreated to QuickCreate (the playground shell) so the toast outlives the dialog:
🐛 Proposed fix (lift success state to playground)
-function Palette({ crumbs, onClose }: { crumbs: BreadcrumbItem[]; onClose: () => void }) {
+function Palette({
+ crumbs,
+ onClose,
+ onCreated,
+}: {
+ crumbs: BreadcrumbItem[];
+ onClose: () => void;
+ onCreated: (type: PageType, name: string) => void;
+}) {
const [phase, setPhase] = useState<"type-select" | "name-entry">("type-select");
const [selectedType, setSelectedType] = useState<PageType | null>(null);
- const [lastCreated, setLastCreated] = useState<{ type: PageType; name: string } | null>(null);
const handleSelect = (type: PageType) => { setSelectedType(type); setPhase("name-entry"); };
const handleCreate = (name: string) => {
if (!selectedType) return;
- setLastCreated({ type: selectedType, name });
+ onCreated(selectedType, name);
onClose();
};
...
- {lastCreated && (
- <SuccessToast type={lastCreated.type} name={lastCreated.name} onDone={() => setLastCreated(null)} />
- )}Then in QuickCreate:
const [open, setOpen] = useState(false);
+ const [lastCreated, setLastCreated] = useState<{ type: PageType; name: string } | null>(null);
...
- {open && <Palette crumbs={context.crumbs} onClose={() => setOpen(false)} />}
+ {open && (
+ <Palette
+ crumbs={context.crumbs}
+ onClose={() => setOpen(false)}
+ onCreated={(type, name) => setLastCreated({ type, name })}
+ />
+ )}
+ {lastCreated && (
+ <SuccessToast
+ type={lastCreated.type}
+ name={lastCreated.name}
+ onDone={() => setLastCreated(null)}
+ />
+ )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@prototypes/pagespace-quick-create/src/components/QuickCreate.tsx` around
lines 324 - 370, Palette's local lastCreated state is lost when onClose unmounts
the component, preventing SuccessToast from ever showing; move the lastCreated
state and its setter into the parent QuickCreate and render SuccessToast from
QuickCreate so the toast outlives the dialog, then change Palette to accept an
onCreated (or setLastCreated) prop and call that from handleCreate instead of
calling its local setLastCreated, and remove the local lastCreated/SucessToast
from Palette; ensure handleCreate still calls onClose after invoking the
parent's onCreated.
There was a problem hiding this comment.
Good catch — this is a prototype-only issue. The real QuickCreatePalette uses Sonner's toast() which lives outside the component tree and is unaffected by unmounting. The prototype is for design iteration only and won't be shipped; I'll leave it as-is rather than add state-lifting complexity to a throwaway file.
There was a problem hiding this comment.
@2witstudios, that makes complete sense — Sonner's toast() is portal-based and survives component unmounts, so the production QuickCreatePalette is unaffected. Happy to leave the prototype as-is.
🧠 Learnings used
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 822
File: packages/lib/src/content/page-types.config.ts:189-206
Timestamp: 2026-04-03T17:40:35.327Z
Learning: In `2witstudios/PageSpace`, new page types that are not yet fully implemented are registered in `PAGE_TYPE_CONFIGS` (packages/lib/src/content/page-types.config.ts) with `experimental: true`. `getCreatablePageTypes()` filters out experimental types and is used as the single source of truth for all creation surfaces: the API Zod schema in `apps/web/src/app/api/pages/route.ts`, the UI dropdown in `apps/web/src/components/layout/left-sidebar/CreatePageDialog.tsx`, and the AI tool enum in `apps/web/src/lib/ai/tools/page-write-tools.ts`. Do not flag hardcoded type lists in these files — they all use `getCreatablePageTypes()` dynamically. The experimental gate is the correct pattern for forward-declaring a page type before its UI component is ready.
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 258
File: apps/realtime/src/validation.ts:0-0
Timestamp: 2026-01-27T03:45:52.322Z
Learning: Enforce using paralleldrive/cuid2 for ID generation across the TypeScript codebase (not UUIDs). IDs should follow the CUID2 format: lowercase alphanumeric starting with a letter, matching ^[a-z][a-z0-9]{1,31}$ with a maximum length of 32 characters. Audit code paths that generate IDs (e.g., new UUID usages) and replace with cuid2 equivalents; ensure generated IDs are consistently lowercase and validated against the regex, and document any exceptions where IDs may differ in semantic meaning.
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 699
File: apps/marketing/src/app/docs/self-hosting/environment/page.tsx:34-37
Timestamp: 2026-02-18T05:15:03.695Z
Learning: Ensure that cross-subdomain cookie handling uses two environment variables: COOKIE_DOMAIN (server-side, for Set-Cookie headers in server code like apps/web/src/lib/auth/cookie-config.ts) and NEXT_PUBLIC_COOKIE_DOMAIN (client-side, for document.cookie interactions in theme-cookie.ts in both apps/web and apps/marketing). This pattern should be verified across all related files that set or rely on cookie domains to maintain consistent domain scoping and enable cross-subdomain functionality.
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 1106
File: packages/lib/src/services/__tests__/app-shell-service.integration.test.ts:11-16
Timestamp: 2026-04-24T14:51:21.236Z
Learning: Do NOT treat imports from `pagespace/db/db`, `pagespace/db/operators`, or `pagespace/db/schema/*` as invalid/non-existent or recommend reverting to the `pagespace/db` barrel import. The `packages/db/package.json` `exports` map intentionally declares these schema subpath entries (via pnpm/native resolution), and this subpath import style is the correct, intended import pattern for all packages consuming `pagespace/db`.
| background: var(--background); | ||
| color: var(--foreground); | ||
| /* Geist Sans isn't available as a web font; Inter is visually identical */ | ||
| font-family: "Inter", ui-sans-serif, system-ui, -apple-system, sans-serif; |
There was a problem hiding this comment.
Stylelint: drop the quotes around Inter.
The repo's font-family-name-quotes rule (value always-where-required) disallows quotes around single-word family names. Only multi-word families or reserved keywords need quoting.
🎨 Proposed fix
- font-family: "Inter", ui-sans-serif, system-ui, -apple-system, sans-serif;
+ font-family: Inter, ui-sans-serif, system-ui, -apple-system, sans-serif;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| font-family: "Inter", ui-sans-serif, system-ui, -apple-system, sans-serif; | |
| font-family: Inter, ui-sans-serif, system-ui, -apple-system, sans-serif; |
🧰 Tools
🪛 Stylelint (17.9.0)
[error] 37-37: Expected no quotes around "Inter" (font-family-name-quotes)
(font-family-name-quotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@prototypes/pagespace-quick-create/src/styles/global.css` at line 37, The
font-family declaration uses quoted "Inter" which violates the stylelint rule;
update the font-family property (the font-family line in global.css) to remove
the quotes around Inter so it reads Inter, ui-sans-serif, system-ui,
-apple-system, sans-serif to satisfy the font-family-name-quotes rule.
There was a problem hiding this comment.
Prototype file, not subject to the repo's stylelint rules (the prototype has its own isolated Vite config outside apps/). No change needed here.
There was a problem hiding this comment.
@2witstudios, understood — the prototypes/pagespace-quick-create/ directory is a self-contained sandbox with its own package.json and vite.config.ts, entirely outside the apps/ scope where the repo's stylelint rules apply. I'll keep that in mind for future reviews.
✏️ Learnings added
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 1085
File: prototypes/pagespace-quick-create/src/styles/global.css:37-37
Timestamp: 2026-04-24T20:53:45.026Z
Learning: The `prototypes/` directory in 2witstudios/PageSpace contains isolated standalone prototypes (e.g., `prototypes/pagespace-quick-create/`) with their own `package.json` and `vite.config.ts`. These prototypes are outside the repo's `apps/` scope and are NOT subject to the repo's stylelint, ESLint, or Prettier rules. Do not flag style/lint violations in any file under `prototypes/`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 812
File: scripts/vitest.config.ts:1-6
Timestamp: 2026-03-18T05:06:39.746Z
Learning: In `scripts/vitest.config.ts` and all other `vitest.config.ts` / `vite.config.ts` files in the 2witstudios/PageSpace monorepo, using `__dirname` directly is valid and intentional. Vite pre-processes config files (bundling them via Rolldown into a temporary file) and injects CJS globals (`__dirname`, `__filename`, `require`) into the evaluation scope even when the project uses ESM. Do not flag `__dirname` usage in Vite/Vitest config files as an ESM incompatibility.
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-24T16:36:42.397Z
Learning: Applies to apps/web/{app,components,lib,src}/**/*.{ts,tsx,js,jsx} : Code must be formatted with Prettier and linted with Next/ESLint as configured in `apps/web/eslint.config.mjs`
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 794
File: packages/lib/src/__tests__/file-processor.test.ts:74-76
Timestamp: 2026-03-15T20:30:27.893Z
Learning: `packages/lib/src/__tests__/file-processor.test.ts` is an integration test that requires a running PostgreSQL database. It is excluded from vitest execution in `packages/lib/vitest.config.ts`. The `as any` casts used for `global.fetch` mock injection are intentional at the integration boundary and should not be flagged for replacement with `vi.mocked()`. Any quality improvements to this file should be addressed in a dedicated integration-test-quality PR.
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-25T00:38:08.816Z
Learning: Applies to **/*.{ts,tsx} : Keep commits and diffs minimal and focused on specific changes
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 820
File: packages/lib/src/integrations/repositories/provider-repository.test.ts:529-548
Timestamp: 2026-04-01T04:28:40.797Z
Learning: In `packages/lib/src/integrations/repositories/provider-repository.test.ts`, ALL test suites (including `refreshBuiltinProviders`) intentionally define local inline mock implementations of each repository function instead of importing the real production functions. This is a deliberate, file-wide pattern: tests operate on `MockDb` (a plain `vi.fn()` object) to avoid having to mock the entire `pagespace/db` module and Drizzle query-builder chain. Do NOT flag the absence of production imports or the presence of duplicated logic in this file as an issue.
CreatePageDialog was deleted in this branch (replaced by QuickCreatePalette). No changes on master conflicted with our implementation files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Re-pressing Alt+N while the palette is open was calling openQuickCreate() again, silently resetting quickCreateParentOverride to undefined. Guard with !quickCreateOpen so the hotkey is a no-op when the palette is already visible. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On macOS, Alt/Option remaps e.key for many letters (e.g. Alt+N produces "~"). matchesKeyEvent was comparing e.key and would silently fail for any Alt+letter hotkey in web browsers on Mac. Now falls back to e.code for alt+single-letter bindings, which always reflects the physical key regardless of modifier-remapped characters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(ui): cmd+n quick-create page palette Replace CreatePageDialog with a Raycast-style two-phase command palette. Cmd+N triggers from anywhere in a drive; phase 1 selects page type, phase 2 names it and shows where the page will land. Context-aware: creates a child when viewing a folder, sibling otherwise, root when no page is open. Sidebar + and context menu "Add child page" both route through the same palette. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ui): wire FilesFinderContent New Page button to palette Master added a New Page button and header layout to FilesFinderContent after this branch was created, wired to CreatePageDialog. Since CreatePageDialog is deleted in this branch, the CI merge-commit failed to resolve the import. Replace the CreatePageDialog import/state/render with openQuickCreate(currentPageId) so the Files view header New Page button opens the quick-create palette. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ui): read files route path param + mutate tree on create P1: Also read params.path[0] for the files explorer route (/dashboard/[driveId]/files/[[...path]]) so cmd+N opened from within a folder correctly infers effectiveParentId instead of falling back to drive root. P2: Mutate the SWR page-tree cache immediately after page or file creation, before navigating, so the new page appears in the tree and CenterPanel never shows "not found". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ui): gate Meta+N to Electron; fix Files parentOverride bypass Meta+N is reserved by web browsers for "new window" and cannot be overridden via preventDefault. Gate the keydown listener to isElectron() so web users are not surprised by a new browser window. Web users can still create pages via the UI buttons or by remapping the hotkey in Settings. FilesFinderContent was passing currentPageId as parentOverride to openQuickCreate(), bypassing the folder-type check in effectiveParentId. Remove the argument so the palette reads params.path[0] from the URL and applies the standard logic: non-folder pages use their parentId, folders accept children. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(ui): memoize palette parent derivation; document tri-state API - Wrap effectiveParentId and contextLabel in useMemo to avoid per-render tree walks when palette is closed - Add JSDoc on quickCreateParentOverride explaining the three-state semantics (undefined/null/string) to prevent caller misuse - Add comment on the 100ms setTimeout explaining Radix Dialog animation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(hotkey): switch quick-create binding to Alt+N Meta+N is reserved by browsers and can't be overridden. Alt+N fires on both web and Electron, so the isElectron() gate is no longer needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(ui): replace emoji with icon badges in palette - Add colored lucide icon badges per page type (matching sidebar icons) - Add ⌥N kbd hint to search input row - Replace emoji in phase-2 header with icon badge - Add prototype in prototypes/pagespace-quick-create/ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ui): skip hotkey open when palette already open Re-pressing Alt+N while the palette is open was calling openQuickCreate() again, silently resetting quickCreateParentOverride to undefined. Guard with !quickCreateOpen so the hotkey is a no-op when the palette is already visible. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(hotkey): use e.code for alt+letter on macOS On macOS, Alt/Option remaps e.key for many letters (e.g. Alt+N produces "~"). matchesKeyEvent was comparing e.key and would silently fail for any Alt+letter hotkey in web browsers on Mac. Now falls back to e.code for alt+single-letter bindings, which always reflects the physical key regardless of modifier-remapped characters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Alt+Nas the universal hotkey for the quick-create palette (replacesMeta+Nwhich browsers intercept)isElectron()gate —Alt+Nworks identically on web and desktop⌥Nkeyboard hint to the right of the search input rowprototypes/pagespace-quick-create/for rapid design iterationTest plan
Alt+Nopens the palette from any dashboard page (web browser)Alt+Nopens the palette in Electron desktop⌥Nkbd hint visible on the right of the search rowpnpm typecheckpasses (pre-existing module errors unrelated to this PR)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Prototypes