feat(electron): add canvas-first workspace shell#5
Merged
Conversation
- analyze the current Electron shell, view chrome, and canvas primitives - recommend a hybrid canvas-first shell with bottom dock and top-right system menu - capture migration phases, tradeoffs, validation steps, and implementation example code
- rename the exploration from 0103 to 0104 - update the document title to match the new numbering
- replace the sidebar-first renderer shell - keep a persistent home canvas with overlay focus surfaces - add a bottom action dock and top-right system menu - create linked page and database cards on the canvas - extend canvas viewport APIs for zoom-based transitions - update the exploration checklist for implemented items
- add compact header controls for focused page views - reduce database toolbar density inside the overlay shell - wire compact mode from the new app shell - update the exploration checklist for the page chrome item
- add pnpm onlyBuiltDependencies entry for better-sqlite3 - unblock sqlite adapter tests in fresh worktrees where native builds were skipped - keep the existing deps:node workflow able to rebuild the native binding before test runs
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralizes a canvas-first shell with state-driven navigation and overlays; adds ActionDock and SystemMenu UI primitives; upgrades CanvasView to a forwardRef with linked-document and viewport snapshot APIs; exposes fit/snapshot methods in canvas hooks/renderer; and adds minimalChrome/compact toggles across page/database/document headers. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant App as App (Shell)
participant Canvas as CanvasView
participant View as Page/Database View
participant Viewport as Viewport Service
User->>App: open linked document (double-click / menu)
App->>Canvas: focusLinkedDocument(docId)
Canvas-->>App: return ViewportSnapshot
App->>Viewport: store snapshot
App->>App: set shell state -> page-focus / database-focus
App->>View: render focused View (minimalChrome = true)
View-->>User: show focused document
User->>App: return to canvas
App->>Viewport: retrieve snapshot
App->>Canvas: restoreViewport(snapshot)
Canvas-->>User: canvas restored with nodes and zoom
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
Preview removed for PR #5. |
- limit the canvas-first card renderer to linked documents and note cards - keep existing non-document canvas nodes on their default rendering path - reduce the risk of breaking older canvases that contain shapes or other rich node types
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/electron/src/renderer/components/DatabaseView.tsx (1)
1434-1526:⚠️ Potential issue | 🟠 Major
minimalChromeis not applied in the empty-database branch.You implemented compact behavior for the main toolbar, but the early return at Line 1399 still renders full chrome (presence + share) even when
minimalChromeis true. This breaks focused-mode consistency on new/empty databases.💡 Suggested fix
- if (columns.length === 0) { + if (columns.length === 0) { return ( <div ref={containerRef} className="flex-1 flex flex-col overflow-hidden"> {/* Toolbar */} - <div className="flex items-center gap-2 p-3 border-b border-border bg-secondary"> + <div + className={[ + 'flex items-center gap-2 border-b border-border', + minimalChrome ? 'bg-background/80 px-5 py-3 backdrop-blur-xl' : 'bg-secondary p-3' + ].join(' ')} + > <input type="text" - className="text-lg font-semibold border-none bg-transparent text-foreground outline-none placeholder:text-muted-foreground" + className={[ + 'border-none bg-transparent text-foreground outline-none placeholder:text-muted-foreground', + minimalChrome ? 'text-base font-semibold' : 'text-lg font-semibold' + ].join(' ')} value={database?.title || ''} onChange={(e) => update({ title: e.target.value })} placeholder="Untitled" /> - <PresenceAvatars presence={presence} localDid={did} /> + {!minimalChrome && <PresenceAvatars presence={presence} localDid={did} />} <div className="flex-1" /> - <ShareButton docId={docId} docType="database" /> + {!minimalChrome && <ShareButton docId={docId} docType="database" />} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/DatabaseView.tsx` around lines 1434 - 1526, The empty-database early-return branch is still rendering full chrome regardless of minimalChrome; update that branch so it respects minimalChrome by conditionally rendering the same compact elements as the main toolbar. Concretely, in the empty-database JSX ensure PresenceAvatars (presence, did), the schema badge (schemaMetadata, setSchemaInfoModalOpen), the Clone button (columns.length, setCloneSchemaModalOpen), the unresolved comments button (commentUnresolvedCount, setSidebarOpen), the view switcher (setViewMode, viewMode) and ShareButton (docId, docType) are wrapped with the same !minimalChrome checks (or render their minimal variants) as in the non-empty toolbar, so the UI is consistent in focused/minimal mode.
🧹 Nitpick comments (2)
apps/electron/src/renderer/components/PageView.tsx (1)
726-740: Add Playwright coverage forminimalChromebranches.These branches hide multiple interaction affordances and change layout density; please add an Electron Playwright test that validates both modes and key actions still work in focused mode.
Based on learnings: "Test UI changes in Electron with Playwright after implementing".
Also applies to: 746-749
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/PageView.tsx` around lines 726 - 740, Add an Electron Playwright test that exercises the PageView minimalChrome branches: render PageView with minimalChrome false and true, assert visibility/state of SyncIndicator (SyncIndicator), share button (showShareButton), unresolved comment button (unresolvedCount and its title/text), and PresenceAvatars (PresenceAvatars/localDid); simulate clicking the unresolved comment button to ensure setSidebarOpen toggles the sidebar; verify compact layout differences when compact/minimalChrome is set. Name the test to cover both modes and include assertions that UI affordances are hidden when minimalChrome is true and interactive behaviors (clicking unresolvedCount, presence display) still work when focused.apps/electron/src/renderer/components/SystemMenu.tsx (1)
10-21: Usetypealiases for these object shapes.
RecentDocumentandSystemMenuPropsare currently interfaces; per coding guidelines,typeis preferred for defining object shapes.Proposed fix
-interface RecentDocument { +type RecentDocument = { id: string title: string type: 'page' | 'database' | 'canvas' -} +} -interface SystemMenuProps { +type SystemMenuProps = { recentDocuments: RecentDocument[] onOpenDocument: (docId: string) => void onOpenSettings: () => void onAddShared: () => void -} +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/SystemMenu.tsx` around lines 10 - 21, Replace the interface declarations with type aliases: change the interface RecentDocument to a type alias named RecentDocument that describes the same object shape (id, title, type: 'page' | 'database' | 'canvas') and convert SystemMenuProps to a type alias named SystemMenuProps that keeps the same properties (recentDocuments: RecentDocument[], onOpenDocument, onOpenSettings, onAddShared). Update any references if necessary (e.g., recentDocuments, onOpenDocument) so the rest of the file continues to use the new type aliases instead of interfaces.
🤖 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/electron/src/renderer/App.tsx`:
- Line 287: Before navigating to settings, clear any pending transition timer
stored in transitionTimerRef to prevent the delayed overlay from firing after
navigation; update the handlers that call setShellState({ kind: 'settings' })
(the execute callbacks that currently do just setShellState) to call
clearTimeout(transitionTimerRef.current) and set transitionTimerRef.current =
null (or undefined) immediately before calling setShellState, and apply the same
change to all entry points that open Settings (the other execute handler and any
settings navigation code paths).
- Around line 193-209: The async handler handleCreateLinkedDocument should catch
rejections from create(schema, { title }) to prevent unhandled promise
rejections; wrap the existing body in a try-catch, await create inside the try,
and in catch log or surface the error via the app logger/UI per guidelines
(include the error object), then return early on failure; preserve the current
behavior of calling canvasViewRef.current?.addLinkedDocumentNode({ id:
newDocument.id, title, type }), setShellState({ kind: 'canvas-home' }) and
setActiveNodeId(homeCanvasId) only when newDocument is truthy, and ensure any
type narrowing on newDocument follows existing patterns used elsewhere in this
file.
- Around line 121-130: The fire-and-forget call to create(CanvasSchema, ...) can
reject and currently has no rejection handler, leaving creatingHomeCanvasRef and
the UI loading state stuck; add a .catch handler to the promise returned by
create() to handle errors explicitly: in the .catch capture the error, call your
UI error reporting (e.g., show a toast or set an error state) and/or clear any
loading state, then keep the existing .finally that sets
creatingHomeCanvasRef.current = false; ensure you still only call
setHomeCanvasId and setActiveNodeId inside the successful .then branch so
failures never leave the app in the "home canvas created" state.
In `@apps/electron/src/renderer/components/CanvasView.tsx`:
- Line 5: The import line currently uses "import type" which erases runtime
symbols; change the import so that Canvas and createNode are imported as runtime
values (not type-only) so createNode (used around lines where it's invoked) and
the JSX <Canvas /> component resolve at runtime; update the import to include
Canvas and createNode as regular imports while keeping type imports like
CanvasNode, Rect, CanvasHandle as type-only if desired.
In
`@docs/explorations/0104_`[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md:
- Line 13: Replace the user-facing string "titlebar" with "title bar" throughout
the document; specifically update the occurrences of "titlebar" (e.g., the line
currently reading "- a top titlebar plus per-view top toolbars" and the other
instances noted at lines 54 and 420) so all references use the two-word form
"title bar" for consistency and readability.
In `@packages/canvas/src/hooks/useCanvas.ts`:
- Around line 302-307: The setViewportSnapshot function currently accepts
unvalidated external input; validate that snapshot.x, snapshot.y, and
snapshot.zoom are finite numbers (reject or coerce NaN/undefined), then clamp
zoom to safe bounds (e.g., introduce MIN_ZOOM and MAX_ZOOM constants and set
zoom = Math.min(MAX_ZOOM, Math.max(MIN_ZOOM, snapshot.zoom))); optionally clamp
x/y to acceptable bounds if you have canvas limits (or at least ensure they are
finite), then assign the sanitized values to viewportRef.current.x/y/zoom and
call setViewportState(viewportRef.current.clone()) as before; update
setViewportSnapshot to perform these checks before assignment.
---
Outside diff comments:
In `@apps/electron/src/renderer/components/DatabaseView.tsx`:
- Around line 1434-1526: The empty-database early-return branch is still
rendering full chrome regardless of minimalChrome; update that branch so it
respects minimalChrome by conditionally rendering the same compact elements as
the main toolbar. Concretely, in the empty-database JSX ensure PresenceAvatars
(presence, did), the schema badge (schemaMetadata, setSchemaInfoModalOpen), the
Clone button (columns.length, setCloneSchemaModalOpen), the unresolved comments
button (commentUnresolvedCount, setSidebarOpen), the view switcher (setViewMode,
viewMode) and ShareButton (docId, docType) are wrapped with the same
!minimalChrome checks (or render their minimal variants) as in the non-empty
toolbar, so the UI is consistent in focused/minimal mode.
---
Nitpick comments:
In `@apps/electron/src/renderer/components/PageView.tsx`:
- Around line 726-740: Add an Electron Playwright test that exercises the
PageView minimalChrome branches: render PageView with minimalChrome false and
true, assert visibility/state of SyncIndicator (SyncIndicator), share button
(showShareButton), unresolved comment button (unresolvedCount and its
title/text), and PresenceAvatars (PresenceAvatars/localDid); simulate clicking
the unresolved comment button to ensure setSidebarOpen toggles the sidebar;
verify compact layout differences when compact/minimalChrome is set. Name the
test to cover both modes and include assertions that UI affordances are hidden
when minimalChrome is true and interactive behaviors (clicking unresolvedCount,
presence display) still work when focused.
In `@apps/electron/src/renderer/components/SystemMenu.tsx`:
- Around line 10-21: Replace the interface declarations with type aliases:
change the interface RecentDocument to a type alias named RecentDocument that
describes the same object shape (id, title, type: 'page' | 'database' |
'canvas') and convert SystemMenuProps to a type alias named SystemMenuProps that
keeps the same properties (recentDocuments: RecentDocument[], onOpenDocument,
onOpenSettings, onAddShared). Update any references if necessary (e.g.,
recentDocuments, onOpenDocument) so the rest of the file continues to use the
new type aliases instead of interfaces.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4babc0a-c8df-4a21-9c98-fc5ccd38aef2
📒 Files selected for processing (11)
apps/electron/src/renderer/App.tsxapps/electron/src/renderer/components/ActionDock.tsxapps/electron/src/renderer/components/CanvasView.tsxapps/electron/src/renderer/components/DatabaseView.tsxapps/electron/src/renderer/components/DocumentHeader.tsxapps/electron/src/renderer/components/PageView.tsxapps/electron/src/renderer/components/SystemMenu.tsxdocs/explorations/0104_[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.mdpackage.jsonpackages/canvas/src/hooks/useCanvas.tspackages/canvas/src/renderer/Canvas.tsx
...APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md
Outdated
Show resolved
Hide resolved
Contributor
Greptile SummaryThis PR replaces the sidebar-driven shell with a canvas-first workspace: the home view is a persistent infinite canvas, page/database documents open as floating overlays via a compact focused-mode chrome, and a bottom action dock + top-right system menu replace the old sidebar controls. The shell state machine ( Key points from this review (items already discussed in prior rounds are excluded):
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
LOAD[App mounts / isLoading]
LOAD -->|canvases.length === 0| BOOTSTRAP[bootstrapHomeCanvas]
LOAD -->|canvases exist| SET_HOME[setHomeCanvasId to most-recent canvas]
BOOTSTRAP -->|success| SET_HOME
BOOTSTRAP -->|failure| ERROR_SCREEN[Error screen with Retry button]
ERROR_SCREEN -->|user clicks Retry| BOOTSTRAP
SET_HOME --> CANVAS_HOME
subgraph Shell States
CANVAS_HOME["canvas-home\n(CanvasView fullscreen)"]
PAGE_FOCUS["page-focus\n(PageView overlay)"]
DB_FOCUS["database-focus\n(DatabaseView overlay)"]
SETTINGS["settings\n(SettingsView overlay)"]
end
CANVAS_HOME -->|double-click linked page/db node\nfocusDocument + 180 ms timer| PAGE_FOCUS
CANVAS_HOME -->|double-click linked db node\nfocusDocument + 180 ms timer| DB_FOCUS
CANVAS_HOME -->|ActionDock / palette: Open Settings| SETTINGS
CANVAS_HOME -->|ActionDock: Create Page/Database\nhandleCreateLinkedDocument| CANVAS_HOME
PAGE_FOCUS -->|ActionDock: Canvas button\nhandleReturnHome| CANVAS_HOME
DB_FOCUS -->|ActionDock: Canvas button\nhandleReturnHome| CANVAS_HOME
SETTINGS -->|SettingsView onClose\nhandleReturnHome| CANVAS_HOME
PAGE_FOCUS -->|clearTransitionTimer called by any transition| CANCEL_TIMER[cancel pending\noverlay timer]
DB_FOCUS --> CANCEL_TIMER
SETTINGS --> CANCEL_TIMER
CANCEL_TIMER --> CANVAS_HOME
Last reviewed commit: 91d57de |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- extract the canvas shell card-selection logic into a renderer helper - mark shell-created notes explicitly instead of styling every card node - add a regression test so generic canvas cards keep their default rendering
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/electron/src/renderer/components/CanvasView.tsx (1)
5-5:⚠️ Potential issue | 🔴 CriticalImport
CanvasandcreateNodeas runtime values (notimport type).Line 5 currently erases both symbols at compile time, but they are used at runtime (Line 137 / Line 157 for
createNode, Line 260 for<Canvas />).Fix
-import type { CanvasNode, Rect, Canvas, createNode, type CanvasHandle } from '@xnetjs/canvas' +import type { CanvasHandle, CanvasNode, Rect } from '@xnetjs/canvas' +import { Canvas, createNode } from '@xnetjs/canvas'Run this to verify the mismatch quickly:
#!/bin/bash set -euo pipefail FILE="apps/electron/src/renderer/components/CanvasView.tsx" echo "=== Import line ===" sed -n '5p' "$FILE" echo "=== Runtime usage of createNode ===" rg -nP '\bcreateNode\s*\(' "$FILE" echo "=== Runtime usage of Canvas component ===" rg -n '<Canvas\b' "$FILE"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/CanvasView.tsx` at line 5, The imports for Canvas and createNode are currently marked as type-only and get erased at compile time, causing runtime errors where createNode() (used around the createNode calls) and the <Canvas /> component (used where Canvas is rendered) are referenced; update the import from '@xnetjs/canvas' so that Canvas and createNode are imported as runtime values (not type-only) while keeping purely type imports (CanvasNode, Rect, CanvasHandle) as types, i.e. split or adjust the import list so Canvas and createNode are value imports and the others remain type imports.
🧹 Nitpick comments (3)
apps/electron/src/renderer/components/CanvasView.tsx (2)
27-44: Prefertypealiases for these object-shape declarations.Lines 27-44 define object shapes with
interface; this repo standard preferstype.Suggested refactor
-interface ViewportSnapshot { +type ViewportSnapshot = { x: number y: number zoom: number -} +} -interface CanvasViewProps { +type CanvasViewProps = { docId: string documents?: LinkedDocumentItem[] onOpenDocument?: (docId: string, docType: Exclude<LinkedDocType, 'canvas'>) => void -} +} -export interface CanvasViewHandle { +export type CanvasViewHandle = { addLinkedDocumentNode: (document: LinkedDocumentItem) => void addCanvasNote: () => void focusLinkedDocument: (docId: string) => ViewportSnapshot | null restoreViewport: (snapshot: ViewportSnapshot) => void -} +}As per coding guidelines: Prefer
typeoverinterfacefor defining object shapes in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/CanvasView.tsx` around lines 27 - 44, The declarations ViewportSnapshot, CanvasViewProps, and CanvasViewHandle are currently defined with interface but the repo prefers type aliases; refactor each interface to a type alias with the same name and identical shape (e.g., convert "interface ViewportSnapshot { ... }" to "type ViewportSnapshot = { ... }"), keeping all property names and signatures (including addLinkedDocumentNode, addCanvasNote, focusLinkedDocument, restoreViewport) unchanged so all usages remain valid.
224-287: Add one Playwright flow for the new canvas-shell interaction path.This UI now depends on interactions (double-click linked card open + viewport actions) that are hard to protect with unit tests alone; one Electron Playwright regression would reduce breakage risk.
Based on learnings: Test UI changes in Electron with Playwright after implementing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/CanvasView.tsx` around lines 224 - 287, Add a Playwright Electron test that covers the new canvas-shell interaction: locate a canvas node rendered via Canvas (use shouldRenderCanvasShellCard/renderNodeCard to identify the shell card), simulate a double-click on a linked card and assert onOpenDocument behavior opens the expected document (matching linkedNodeId and linkedType), then exercise viewport actions by clicking the Fit and Reset IconButton controls (which call canvasRef.current?.fitToContent and resetView) and assert the canvas transform/zoom changed and then returned; place the flow as an end-to-end Electron Playwright test to catch regressions in onNodeDoubleClick and the toolbar actions.apps/electron/src/renderer/lib/canvas-shell.ts (1)
5-9: Use atypealias forLinkedDocumentItem.Line 5 uses
interface, while this codebase standardizes object-shape declarations withtype.Suggested refactor
-export interface LinkedDocumentItem { +export type LinkedDocumentItem = { id: string title: string type: LinkedDocType -} +}As per coding guidelines: Prefer
typeoverinterfacefor defining object shapes in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/lib/canvas-shell.ts` around lines 5 - 9, Replace the interface declaration for LinkedDocumentItem with a TypeScript type alias to match the codebase standard: change the declaration of LinkedDocumentItem (which currently uses "interface LinkedDocumentItem { id: string; title: string; type: LinkedDocType }") to a type alias using "type LinkedDocumentItem = { ... }", preserving the property names and types (id, title, type: LinkedDocType); update any imports/exports if needed to keep usages intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/electron/src/renderer/components/CanvasView.tsx`:
- Line 5: The imports for Canvas and createNode are currently marked as
type-only and get erased at compile time, causing runtime errors where
createNode() (used around the createNode calls) and the <Canvas /> component
(used where Canvas is rendered) are referenced; update the import from
'@xnetjs/canvas' so that Canvas and createNode are imported as runtime values
(not type-only) while keeping purely type imports (CanvasNode, Rect,
CanvasHandle) as types, i.e. split or adjust the import list so Canvas and
createNode are value imports and the others remain type imports.
---
Nitpick comments:
In `@apps/electron/src/renderer/components/CanvasView.tsx`:
- Around line 27-44: The declarations ViewportSnapshot, CanvasViewProps, and
CanvasViewHandle are currently defined with interface but the repo prefers type
aliases; refactor each interface to a type alias with the same name and
identical shape (e.g., convert "interface ViewportSnapshot { ... }" to "type
ViewportSnapshot = { ... }"), keeping all property names and signatures
(including addLinkedDocumentNode, addCanvasNote, focusLinkedDocument,
restoreViewport) unchanged so all usages remain valid.
- Around line 224-287: Add a Playwright Electron test that covers the new
canvas-shell interaction: locate a canvas node rendered via Canvas (use
shouldRenderCanvasShellCard/renderNodeCard to identify the shell card), simulate
a double-click on a linked card and assert onOpenDocument behavior opens the
expected document (matching linkedNodeId and linkedType), then exercise viewport
actions by clicking the Fit and Reset IconButton controls (which call
canvasRef.current?.fitToContent and resetView) and assert the canvas
transform/zoom changed and then returned; place the flow as an end-to-end
Electron Playwright test to catch regressions in onNodeDoubleClick and the
toolbar actions.
In `@apps/electron/src/renderer/lib/canvas-shell.ts`:
- Around line 5-9: Replace the interface declaration for LinkedDocumentItem with
a TypeScript type alias to match the codebase standard: change the declaration
of LinkedDocumentItem (which currently uses "interface LinkedDocumentItem { id:
string; title: string; type: LinkedDocType }") to a type alias using "type
LinkedDocumentItem = { ... }", preserving the property names and types (id,
title, type: LinkedDocType); update any imports/exports if needed to keep usages
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62df4a0e-3d4d-4acc-80a7-7b99cd03ce95
📒 Files selected for processing (3)
apps/electron/src/renderer/components/CanvasView.tsxapps/electron/src/renderer/lib/canvas-shell.test.tsapps/electron/src/renderer/lib/canvas-shell.ts
- catch home-canvas creation failures - catch linked document creation failures - restore runtime canvas imports - hide onboarding after nodes exist - add a debug-panel action to the system menu - skip zoom-style shell transitions for reduced motion - sanitize restored viewport snapshots
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/electron/src/renderer/App.tsx (1)
296-301:⚠️ Potential issue | 🟡 MinorClear pending transition timer before opening Settings.
A delayed overlay transition scheduled via
transitionTimerRef(Line 175) can still fire after navigating to settings. Both the palette command (Line 300) and the SystemMenu callback (Line 378) should clear the timer first.Suggested fix
+ const handleOpenSettings = useCallback(() => { + clearTransitionTimer() + setShellState({ kind: 'settings' }) + }, [clearTransitionTimer]) + const paletteCommands = useMemo<PaletteCommand[]>( () => [ // ... other commands ... { id: 'open-settings', name: 'Open Settings', description: 'Open the system settings overlay', icon: 'settings', - execute: () => setShellState({ kind: 'settings' }) + execute: handleOpenSettings },And update the SystemMenu callback at Line 378:
- onOpenSettings={() => setShellState({ kind: 'settings' })} + onOpenSettings={handleOpenSettings}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/App.tsx` around lines 296 - 301, The delayed overlay transition timer referenced by transitionTimerRef can fire after navigation to settings; before calling setShellState({ kind: 'settings' }) in the 'open-settings' command (id: 'open-settings') and in the SystemMenu callback, clear any pending timer by checking transitionTimerRef.current and calling clearTimeout(transitionTimerRef.current) (and then setting transitionTimerRef.current = null) so the pending transition cannot run after the settings overlay opens.
🧹 Nitpick comments (5)
docs/explorations/0104_[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md (2)
548-568: Refine the return type to match implementation.The function always returns
type: 'embed', but the return type declarestype: 'card' | 'embed'. Consider narrowing the type to accurately reflect the implementation:♻️ Proposed fix
export function createLinkedNodeAtViewport( intent: CreateIntent, viewport: ViewportSnapshot ): { - type: 'card' | 'embed' + type: 'embed' linkedSchema: 'page' | 'database' | 'canvas' position: { x: number; y: number; width: number; height: number } } {Alternatively, if 'card' is intentionally supported, add the conditional logic to return it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/explorations/0104_`[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md around lines 548 - 568, The declared return type for createLinkedNodeAtViewport is too broad (type: 'card' | 'embed') while the implementation always returns type: 'embed'; either tighten the signature to return only 'embed' or implement branching logic in createLinkedNodeAtViewport to return 'card' where intended (for example based on intent values like 'card' or 'canvas-card'), and ensure linkedSchema mapping and position logic remain correct when adding that conditional.
511-520: Consider completing the validation checklist.The implementation checklist shows most items as complete, but the validation checklist remains entirely unchecked. Since the PR summary mentions validation steps were recorded (
pnpm typecheck,pnpm test), consider checking off applicable validation items or documenting which remain to be verified before merge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/explorations/0104_`[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md around lines 511 - 520, The validation checklist at the bottom is still all unchecked; please either mark each applicable item (e.g., "Verify the app always lands on a canvas", "Verify a new page/database appears at the current viewport center", "Verify keyboard-only flows still work", "Verify reduced-motion mode falls back", "Verify Playwright/Electron manual checks cover canvas home, page focus, database focus, and collaboration affordances") as completed or add notes documenting which items remain unverified, how to reproduce them, which test/command validated them (e.g., pnpm typecheck, pnpm test, Playwright/Electron), and who/when validated each item so reviewers can confirm readiness for merge.apps/electron/src/renderer/App.tsx (1)
34-40: Prefertypeoverinterfacefor object shapes.Per coding guidelines, use
typeinstead ofinterfacefor defining object shapes in TypeScript.Suggested change
-interface DocumentItem { +type DocumentItem = { id: string title: string type: DocType createdAt?: number updatedAt?: number }As per coding guidelines: "Prefer
typeoverinterfacefor defining object shapes in TypeScript"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/App.tsx` around lines 34 - 40, Replace the DocumentItem interface with a type alias per project conventions: change "interface DocumentItem { ... }" to "type DocumentItem = { id: string; title: string; type: DocType; createdAt?: number; updatedAt?: number }", keeping the same property names and optional modifiers (and update any imports/exports that reference DocumentItem if necessary).apps/electron/src/renderer/components/CanvasView.tsx (2)
28-38: Prefertypeoverinterfacefor internal object shapes.
ViewportSnapshotandCanvasViewPropsare not exported and don't require declaration merging. Per coding guidelines, prefertypefor object shapes.Suggested change
-interface ViewportSnapshot { +type ViewportSnapshot = { x: number y: number zoom: number } -interface CanvasViewProps { +type CanvasViewProps = { docId: string documents?: LinkedDocumentItem[] onOpenDocument?: (docId: string, docType: Exclude<LinkedDocType, 'canvas'>) => void }As per coding guidelines: "Prefer
typeoverinterfacefor defining object shapes in TypeScript"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/CanvasView.tsx` around lines 28 - 38, Replace the two internal interfaces with type aliases: change the declarations of ViewportSnapshot and CanvasViewProps to use type instead of interface while preserving the same property shapes (including docId, documents?: LinkedDocumentItem[], and onOpenDocument?: (docId: string, docType: Exclude<LinkedDocType, 'canvas'>) => void). Update any local references to these names if necessary; no export or behavior changes are needed.
5-26: Import order: external packages should precede internal packages.Per coding guidelines, the import order should be: type-only imports → external packages → internal packages → local relative imports. Currently,
lucide-reactandreact(external) come after@xnetjs/*(internal).Suggested reordering
import type { CanvasHandle, CanvasNode, Rect } from '@xnetjs/canvas' +import { Compass, Database, FileText, Maximize2, StickyNote } from 'lucide-react' +import React, { + forwardRef, + useEffect, + useState, + useCallback, + useRef, + useImperativeHandle, + useMemo +} from 'react' import { Canvas, createNode } from '@xnetjs/canvas' import { CanvasSchema } from '@xnetjs/data' import { useNode, useIdentity } from '@xnetjs/react' import { IconButton } from '@xnetjs/ui' -import { Compass, Database, FileText, Maximize2, StickyNote } from 'lucide-react' -import React, { - forwardRef, - useEffect, - useState, - useCallback, - useRef, - useImperativeHandle, - useMemo -} from 'react' import { createCanvasShellNoteProperties, isCanvasShellNote, shouldRenderCanvasShellCard, type LinkedDocType, type LinkedDocumentItem } from '../lib/canvas-shell'As per coding guidelines: "External package imports should follow type-only imports" and "Internal package imports should follow external packages"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/CanvasView.tsx` around lines 5 - 26, Reorder the imports in CanvasView.tsx so type-only imports (CanvasHandle, CanvasNode, Rect) remain first, then external packages (React and named imports like forwardRef, useEffect, useState, useCallback, useRef, useImperativeHandle, useMemo and lucide-react icons Compass, Database, FileText, Maximize2, StickyNote), followed by internal `@xnetjs` packages (Canvas, createNode, CanvasSchema, useNode, useIdentity, IconButton) and finally the local relative import (createCanvasShellNoteProperties, isCanvasShellNote, shouldRenderCanvasShellCard, LinkedDocType, LinkedDocumentItem from ../lib/canvas-shell); adjust import grouping/order but keep the same imported symbols and semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/electron/src/renderer/App.tsx`:
- Around line 296-301: The delayed overlay transition timer referenced by
transitionTimerRef can fire after navigation to settings; before calling
setShellState({ kind: 'settings' }) in the 'open-settings' command (id:
'open-settings') and in the SystemMenu callback, clear any pending timer by
checking transitionTimerRef.current and calling
clearTimeout(transitionTimerRef.current) (and then setting
transitionTimerRef.current = null) so the pending transition cannot run after
the settings overlay opens.
---
Nitpick comments:
In `@apps/electron/src/renderer/App.tsx`:
- Around line 34-40: Replace the DocumentItem interface with a type alias per
project conventions: change "interface DocumentItem { ... }" to "type
DocumentItem = { id: string; title: string; type: DocType; createdAt?: number;
updatedAt?: number }", keeping the same property names and optional modifiers
(and update any imports/exports that reference DocumentItem if necessary).
In `@apps/electron/src/renderer/components/CanvasView.tsx`:
- Around line 28-38: Replace the two internal interfaces with type aliases:
change the declarations of ViewportSnapshot and CanvasViewProps to use type
instead of interface while preserving the same property shapes (including docId,
documents?: LinkedDocumentItem[], and onOpenDocument?: (docId: string, docType:
Exclude<LinkedDocType, 'canvas'>) => void). Update any local references to these
names if necessary; no export or behavior changes are needed.
- Around line 5-26: Reorder the imports in CanvasView.tsx so type-only imports
(CanvasHandle, CanvasNode, Rect) remain first, then external packages (React and
named imports like forwardRef, useEffect, useState, useCallback, useRef,
useImperativeHandle, useMemo and lucide-react icons Compass, Database, FileText,
Maximize2, StickyNote), followed by internal `@xnetjs` packages (Canvas,
createNode, CanvasSchema, useNode, useIdentity, IconButton) and finally the
local relative import (createCanvasShellNoteProperties, isCanvasShellNote,
shouldRenderCanvasShellCard, LinkedDocType, LinkedDocumentItem from
../lib/canvas-shell); adjust import grouping/order but keep the same imported
symbols and semantics.
In
`@docs/explorations/0104_`[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md:
- Around line 548-568: The declared return type for createLinkedNodeAtViewport
is too broad (type: 'card' | 'embed') while the implementation always returns
type: 'embed'; either tighten the signature to return only 'embed' or implement
branching logic in createLinkedNodeAtViewport to return 'card' where intended
(for example based on intent values like 'card' or 'canvas-card'), and ensure
linkedSchema mapping and position logic remain correct when adding that
conditional.
- Around line 511-520: The validation checklist at the bottom is still all
unchecked; please either mark each applicable item (e.g., "Verify the app always
lands on a canvas", "Verify a new page/database appears at the current viewport
center", "Verify keyboard-only flows still work", "Verify reduced-motion mode
falls back", "Verify Playwright/Electron manual checks cover canvas home, page
focus, database focus, and collaboration affordances") as completed or add notes
documenting which items remain unverified, how to reproduce them, which
test/command validated them (e.g., pnpm typecheck, pnpm test,
Playwright/Electron), and who/when validated each item so reviewers can confirm
readiness for merge.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 834a9beb-e9df-4891-88b5-8f981d67fec9
📒 Files selected for processing (5)
apps/electron/src/renderer/App.tsxapps/electron/src/renderer/components/CanvasView.tsxapps/electron/src/renderer/components/SystemMenu.tsxdocs/explorations/0104_[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.mdpackages/canvas/src/hooks/useCanvas.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/canvas/src/hooks/useCanvas.ts
- apps/electron/src/renderer/components/SystemMenu.tsx
- add retryable home-canvas bootstrap recovery - limit palette recents and clear transitions before settings - hide non-actionable open affordances on canvas-linked cards - allow electron install scripts in fresh worktrees - record completed validation for the canvas-first exploration
There was a problem hiding this comment.
♻️ Duplicate comments (3)
docs/explorations/0104_[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md (1)
13-13:⚠️ Potential issue | 🟡 MinorUse “title bar” consistently in user-facing text.
Line 13, Line 54, and Line 420 still use “titlebar”; please normalize to “title bar” for consistency/readability.
Also applies to: 54-54, 420-420
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/explorations/0104_`[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md at line 13, The document uses the compound "titlebar" in multiple places; update every occurrence of the string "titlebar" to the two-word form "title bar" (ensure exact replacement of "titlebar" in the body, headings and metadata) so user-facing text is consistent—search for the token "titlebar" and replace with "title bar" across the file.apps/electron/src/renderer/App.tsx (2)
224-285:⚠️ Potential issue | 🟡 MinorClear pending transition timer before forcing canvas-home state.
focusDocumentschedules delayed overlay open (Line 197). In Line 237, Line 248, and Line 277, handlers switch shell state without clearing that timer, so a stale delayed overlay can still open afterward.⏱️ Proposed fix
const handleCreateLinkedDocument = useCallback( async (type: Exclude<DocType, 'canvas'>) => { + clearTransitionTimer() try { const schema = type === 'page' ? PageSchema : DatabaseSchema @@ } catch (error) { console.error('Failed to create linked document', toError(error)) } }, - [create, homeCanvasId, setActiveNodeId] + [clearTransitionTimer, create, homeCanvasId, setActiveNodeId] ) const handleCreateCanvasNote = useCallback(() => { + clearTransitionTimer() canvasViewRef.current?.addCanvasNote() setShellState({ kind: 'canvas-home' }) setActiveNodeId(homeCanvasId) -}, [homeCanvasId, setActiveNodeId]) +}, [clearTransitionTimer, homeCanvasId, setActiveNodeId]) const handleAddShared = useCallback( async (input: AddSharedInput) => { @@ if (input.docType === 'canvas') { + clearTransitionTimer() setHomeCanvasId(input.docId) setShellState({ kind: 'canvas-home' }) setActiveNodeId(input.docId) return } @@ }, - [focusDocument, setActiveNodeId] + [clearTransitionTimer, focusDocument, setActiveNodeId] )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/App.tsx` around lines 224 - 285, Call clearTransitionTimer() before forcing the canvas-home state to prevent any stale delayed overlay from opening; add that call at the start (before setShellState({ kind: 'canvas-home' })) in handleCreateLinkedDocument, handleCreateCanvasNote, and handleAddShared, and ensure clearTransitionTimer is included in each useCallback dependency array (handleCreateLinkedDocument's and handleAddShared's dependencies currently omit it). Also verify handleReturnHome still calls clearTransitionTimer (keep it) so all code paths that set canvas-home clear the pending timer.
264-285:⚠️ Potential issue | 🟠 MajorWrap
handleAddSharedin try/catch to prevent unhandled rejections.Line 267 awaits
configureShareSession(...)without local error handling. If this rejects, the async handler can fail noisily and skip subsequent state updates.🛡️ Proposed hardening
const handleAddShared = useCallback( async (input: AddSharedInput) => { - if (input.share) { - await window.__xnetIpcSyncManager?.configureShareSession({ - signalingUrl: input.share.endpoint, - ucanToken: input.share.token, - transport: input.share.transport, - iceServers: input.share.iceServers - }) - } - - if (input.docType === 'canvas') { - setHomeCanvasId(input.docId) - setShellState({ kind: 'canvas-home' }) - setActiveNodeId(input.docId) - return - } - - focusDocument(input.docId, input.docType, false) + try { + if (input.share) { + await window.__xnetIpcSyncManager?.configureShareSession({ + signalingUrl: input.share.endpoint, + ucanToken: input.share.token, + transport: input.share.transport, + iceServers: input.share.iceServers + }) + } + + if (input.docType === 'canvas') { + setHomeCanvasId(input.docId) + setShellState({ kind: 'canvas-home' }) + setActiveNodeId(input.docId) + return + } + + focusDocument(input.docId, input.docType, false) + } catch (error) { + console.error('Failed to add shared document', toError(error)) + } }, [focusDocument, setActiveNodeId] )Based on learnings, "Applies to **/*.{ts,tsx} : Use try-catch with type narrowing:
err instanceof Error ? err : new Error(String(err))".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/App.tsx` around lines 264 - 285, Wrap the await call to window.__xnetIpcSyncManager?.configureShareSession inside a try/catch in the handleAddShared async callback so a rejected promise doesn't skip the rest of the function; on catch narrow the error to an Error with the pattern (err instanceof Error ? err : new Error(String(err))) and log it (e.g., console.error or an existing logger), then continue to run the post-share logic (setHomeCanvasId, setShellState, setActiveNodeId, or focusDocument) as appropriate; update the handleAddShared function to perform this local error handling around configureShareSession so unhandled rejections are prevented.
🧹 Nitpick comments (5)
docs/explorations/0104_[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md (2)
523-524: Add trailing commas after the year for date-parenthetical style consistency.Consider “March 6, 2026, with …” and “March 6, 2026, confirmed …” for consistent prose style in these validation notes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/explorations/0104_`[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md around lines 523 - 524, Update the two validation-note sentences to insert a trailing comma after the year "2026" for consistent date-parenthetical style: change "March 6, 2026 with `pnpm typecheck`..." to "March 6, 2026, with `pnpm typecheck`..." and change "March 6, 2026 confirmed the first-run shell..." to "March 6, 2026, confirmed the first-run shell..."; these edits are in the two lines containing the automated and manual validation notes (the phrases starting "Automated validation completed on March 6, 2026 with..." and "Manual Electron validation on March 6, 2026 confirmed...").
608-613: Preserve prior mode when closing settings in the example reducer.
close-settingsalways returnscanvas-home, which drops context if settings was opened from a focused view. In the sample state model, this conflicts with the stated “restore context” interaction.♻️ Proposed refactor
export type ShellMode = | { kind: 'canvas-home'; canvasId: string } | { kind: 'page-focus'; canvasId: string; pageId: string; returnViewport: ViewportSnapshot } | { kind: 'database-focus' canvasId: string databaseId: string returnViewport: ViewportSnapshot } - | { kind: 'settings'; canvasId: string } + | { + kind: 'settings' + canvasId: string + previous: Exclude<ShellMode, { kind: 'settings' }> + } ... case 'open-settings': - return { kind: 'settings', canvasId: state.canvasId } + if (state.kind === 'settings') return state + return { kind: 'settings', canvasId: state.canvasId, previous: state } case 'close-settings': - return { kind: 'canvas-home', canvasId: state.canvasId } + return state.kind === 'settings' ? state.previous : state🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/explorations/0104_`[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md around lines 608 - 613, The reducer currently maps 'open-settings' to { kind: 'settings', canvasId: state.canvasId } and 'close-settings' to { kind: 'canvas-home', canvasId: state.canvasId }, which drops context; modify the state produced by the 'open-settings' branch to preserve the prior view (e.g., include previousKind or previousMode = state.kind) and update the 'close-settings' branch to restore that preserved value (return { kind: state.previousKind ?? 'canvas-home', canvasId: state.canvasId }) so closing settings returns the prior mode when present and falls back to 'canvas-home' otherwise.apps/electron/src/renderer/App.tsx (1)
5-19: Align import order with repo rules.External runtime imports (
react) should be placed before internal@xnetjs/*runtime imports after type-only imports.As per coding guidelines, "
Type-only imports must come first" and "Internal package imports should follow external packages".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/App.tsx` around lines 5 - 19, The import order in App.tsx violates repo rules: move the external runtime import "import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'" (and any other external packages) to appear before internal package imports (those starting with "@xnetjs/*"), while keeping type-only imports like "import type { PaletteCommand }" at the very top; rearrange lines so type-only imports come first, then external runtime imports (React), then internal `@xnetjs` imports (e.g., PageSchema, useDevTools, useQuery, CommandPalette), and finally local relative imports (components such as ActionDock, CanvasView, etc.).apps/electron/src/renderer/components/CanvasView.tsx (2)
28-45: Avoid duplicatingViewportSnapshotacross modules.
ViewportSnapshotis defined here and again inapps/electron/src/renderer/App.tsx(Line 22-26 there). Exporting and reusing one type will prevent silent drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/CanvasView.tsx` around lines 28 - 45, The type ViewportSnapshot is duplicated; extract it into a single shared type and import it where needed: create/export a shared type (e.g., export type ViewportSnapshot = { x: number; y: number; zoom: number }) in a common types module, replace the local declaration in CanvasView.tsx and the duplicate in App.tsx with an import of that shared type, and ensure CanvasViewHandle still references the imported ViewportSnapshot so the add/focus/restore signatures remain unchanged.
5-26: Reorder imports to match repository grouping rules.At Line 6-19, internal
@xnetjs/*runtime imports appear before external runtime imports (react,lucide-react). That breaks the configured import order.♻️ Proposed import regrouping
import type { CanvasHandle, CanvasNode, Rect } from '@xnetjs/canvas' -import { Canvas, createNode } from '@xnetjs/canvas' -import { CanvasSchema } from '@xnetjs/data' -import { useNode, useIdentity } from '@xnetjs/react' -import { IconButton } from '@xnetjs/ui' -import { Compass, Database, FileText, Maximize2, StickyNote } from 'lucide-react' import React, { forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState } from 'react' +import { Compass, Database, FileText, Maximize2, StickyNote } from 'lucide-react' +import { Canvas, createNode } from '@xnetjs/canvas' +import { CanvasSchema } from '@xnetjs/data' +import { useNode, useIdentity } from '@xnetjs/react' +import { IconButton } from '@xnetjs/ui'As per coding guidelines, "
Internal package imports should follow external packages" and "External package imports should follow type-only imports".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/CanvasView.tsx` around lines 5 - 26, Reorder the import block so type-only imports come first, then external runtime packages, then internal `@xnetjs/`* runtime imports: move React and lucide-react imports (React, forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState and Compass, Database, FileText, Maximize2, StickyNote) ahead of the `@xnetjs` runtime imports (Canvas, createNode, useNode, useIdentity, IconButton), while keeping the type imports (CanvasHandle, CanvasNode, Rect, CanvasSchema, LinkedDocType, LinkedDocumentItem) grouped as type-only; update the top-level import order around symbols Canvas, createNode, CanvasHandle, CanvasNode, Rect, CanvasSchema, useNode, useIdentity, IconButton, Compass, Database, FileText, Maximize2, StickyNote, and React accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/electron/src/renderer/App.tsx`:
- Around line 224-285: Call clearTransitionTimer() before forcing the
canvas-home state to prevent any stale delayed overlay from opening; add that
call at the start (before setShellState({ kind: 'canvas-home' })) in
handleCreateLinkedDocument, handleCreateCanvasNote, and handleAddShared, and
ensure clearTransitionTimer is included in each useCallback dependency array
(handleCreateLinkedDocument's and handleAddShared's dependencies currently omit
it). Also verify handleReturnHome still calls clearTransitionTimer (keep it) so
all code paths that set canvas-home clear the pending timer.
- Around line 264-285: Wrap the await call to
window.__xnetIpcSyncManager?.configureShareSession inside a try/catch in the
handleAddShared async callback so a rejected promise doesn't skip the rest of
the function; on catch narrow the error to an Error with the pattern (err
instanceof Error ? err : new Error(String(err))) and log it (e.g., console.error
or an existing logger), then continue to run the post-share logic
(setHomeCanvasId, setShellState, setActiveNodeId, or focusDocument) as
appropriate; update the handleAddShared function to perform this local error
handling around configureShareSession so unhandled rejections are prevented.
In
`@docs/explorations/0104_`[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md:
- Line 13: The document uses the compound "titlebar" in multiple places; update
every occurrence of the string "titlebar" to the two-word form "title bar"
(ensure exact replacement of "titlebar" in the body, headings and metadata) so
user-facing text is consistent—search for the token "titlebar" and replace with
"title bar" across the file.
---
Nitpick comments:
In `@apps/electron/src/renderer/App.tsx`:
- Around line 5-19: The import order in App.tsx violates repo rules: move the
external runtime import "import React, { useCallback, useEffect, useMemo,
useRef, useState } from 'react'" (and any other external packages) to appear
before internal package imports (those starting with "@xnetjs/*"), while keeping
type-only imports like "import type { PaletteCommand }" at the very top;
rearrange lines so type-only imports come first, then external runtime imports
(React), then internal `@xnetjs` imports (e.g., PageSchema, useDevTools, useQuery,
CommandPalette), and finally local relative imports (components such as
ActionDock, CanvasView, etc.).
In `@apps/electron/src/renderer/components/CanvasView.tsx`:
- Around line 28-45: The type ViewportSnapshot is duplicated; extract it into a
single shared type and import it where needed: create/export a shared type
(e.g., export type ViewportSnapshot = { x: number; y: number; zoom: number }) in
a common types module, replace the local declaration in CanvasView.tsx and the
duplicate in App.tsx with an import of that shared type, and ensure
CanvasViewHandle still references the imported ViewportSnapshot so the
add/focus/restore signatures remain unchanged.
- Around line 5-26: Reorder the import block so type-only imports come first,
then external runtime packages, then internal `@xnetjs/`* runtime imports: move
React and lucide-react imports (React, forwardRef, useCallback, useEffect,
useImperativeHandle, useMemo, useRef, useState and Compass, Database, FileText,
Maximize2, StickyNote) ahead of the `@xnetjs` runtime imports (Canvas, createNode,
useNode, useIdentity, IconButton), while keeping the type imports (CanvasHandle,
CanvasNode, Rect, CanvasSchema, LinkedDocType, LinkedDocumentItem) grouped as
type-only; update the top-level import order around symbols Canvas, createNode,
CanvasHandle, CanvasNode, Rect, CanvasSchema, useNode, useIdentity, IconButton,
Compass, Database, FileText, Maximize2, StickyNote, and React accordingly.
In
`@docs/explorations/0104_`[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md:
- Around line 523-524: Update the two validation-note sentences to insert a
trailing comma after the year "2026" for consistent date-parenthetical style:
change "March 6, 2026 with `pnpm typecheck`..." to "March 6, 2026, with `pnpm
typecheck`..." and change "March 6, 2026 confirmed the first-run shell..." to
"March 6, 2026, confirmed the first-run shell..."; these edits are in the two
lines containing the automated and manual validation notes (the phrases starting
"Automated validation completed on March 6, 2026 with..." and "Manual Electron
validation on March 6, 2026 confirmed...").
- Around line 608-613: The reducer currently maps 'open-settings' to { kind:
'settings', canvasId: state.canvasId } and 'close-settings' to { kind:
'canvas-home', canvasId: state.canvasId }, which drops context; modify the state
produced by the 'open-settings' branch to preserve the prior view (e.g., include
previousKind or previousMode = state.kind) and update the 'close-settings'
branch to restore that preserved value (return { kind: state.previousKind ??
'canvas-home', canvasId: state.canvasId }) so closing settings returns the prior
mode when present and falls back to 'canvas-home' otherwise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0e4fa06-027e-4b3e-bd70-2fc5746670ef
📒 Files selected for processing (5)
apps/electron/src/renderer/App.tsxapps/electron/src/renderer/components/CanvasView.tsxapps/electron/src/renderer/lib/canvas-shell.tsdocs/explorations/0104_[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.mdpackage.json
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- apps/electron/src/renderer/lib/canvas-shell.ts
- clear delayed overlay timers before forcing canvas-home state - catch shared-session configuration failures locally - align import order with repo conventions - clean up remaining exploration wording and reducer notes
- clear stale home canvas ids before re-running bootstrap - keep the canvas-first shell recoverable after external canvas deletion - revalidate the desktop renderer typecheck and build path
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/explorations/0104_[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md (1)
523-525: Make validation notes reproducible with durable references.Consider adding links/IDs to the exact CI runs (or committed artifacts) for these validation claims so future readers can verify them without relying on local paths or ephemeral context.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/explorations/0104_`[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md around lines 523 - 525, Replace the ephemeral validation notes with durable references: for the "Automated validation completed..." line, include the CI workflow run URL or run ID and the commit SHA and link to the build/artifact that contains the pnpm typecheck/test/build logs; for the "Manual Electron validation..." line, upload the Playwright screenshot and any electron test artifacts to the repository CI artifacts (or an S3/GC bucket) and replace the local path with that permanent URL plus the test run ID and environment variables used; for the "Manual Electron validation is still required..." line, add a tracking issue/PR or checklist URL that governs the remaining UX validations so readers can verify status. Ensure these edits target the exact strings in the doc ("Automated validation completed...", "Manual Electron validation on...", "Manual Electron validation is still required...") so the references are durable and reproducible.apps/electron/src/renderer/App.tsx (2)
121-417: Add one Electron Playwright E2E for bootstrap/retry and overlay transitions.Given the async bootstrap + timer-driven overlay behavior in this range, a single E2E that covers create-failure → retry-success and settings/open/return flows would harden regressions quickly.
Based on learnings "Test UI changes in Electron with Playwright after implementing".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/App.tsx` around lines 121 - 417, Add an Electron Playwright E2E that simulates the home canvas bootstrap failure and retry success and verifies overlay transitions: stub the create(CanvasSchema, ...) call to first throw and then resolve, launch the app, assert the "Unable to create your workspace canvas." UI appears, click the Retry button which should trigger bootstrapHomeCanvas to succeed and setHomeCanvasId; next open settings via the UI (triggering handleOpenSettings) and assert the settings overlay appears, then return home (triggering handleReturnHome) and assert shellState returns to 'canvas-home' and active node is the home canvas; finally exercise opening a linked document to trigger focusDocument/overlay open (respecting OVERLAY_OPEN_DELAY_MS) and assert the page/database overlay shows and restores viewport when returning. Include assertions around visible overlayTitle text and that the Retry button & settings open/close workflows complete.
6-10: Reorder imports to match repo conventions.On Line 6-Line 9, internal
@xnetjs/*value imports are placed before externalreacton Line 10. Please keep external package imports before internal package imports.Suggested diff
import type { PaletteCommand } from '@xnetjs/ui' +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { PageSchema, DatabaseSchema, CanvasSchema } from '@xnetjs/data' import { useDevTools } from '@xnetjs/devtools' import { useQuery, useMutate } from '@xnetjs/react' import { CommandPalette, useCommandPalette, usePrefersReducedMotion } from '@xnetjs/ui' -import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'As per coding guidelines "Internal package imports should follow external packages" and "External package imports should follow type-only imports".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/App.tsx` around lines 6 - 10, Reorder the import block so external packages come before internal `@xnetjs` imports: move the React import (React, useCallback, useEffect, useMemo, useRef, useState) to be above the `@xnetjs/`* imports that bring in PageSchema, DatabaseSchema, CanvasSchema, useDevTools, useQuery, useMutate, CommandPalette, useCommandPalette, and usePrefersReducedMotion; also ensure any type-only imports (if present) remain before external package imports per repo conventions.
🤖 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/electron/src/renderer/App.tsx`:
- Around line 147-150: The early return when canvases.length === 0 currently
skips bootstrap whenever homeCanvasId is truthy, which prevents re-bootstrap if
that id is stale; change the logic in the App component so that when canvases
becomes empty you call bootstrapHomeCanvas() unless a valid, current home canvas
exists: detect the transition by storing previous canvases length (e.g., a
useRef prevCanvasesLen or effect dependence on canvases) and if prevCanvasesLen
> 0 && canvases.length === 0 or if homeCanvasId is missing/invalid (and not when
homeCanvasBootstrapError is set) then invoke bootstrapHomeCanvas(); update the
check around homeCanvasId/homeCanvasBootstrapError and the early return so stale
homeCanvasId does not block bootstrapHomeCanvas().
---
Nitpick comments:
In `@apps/electron/src/renderer/App.tsx`:
- Around line 121-417: Add an Electron Playwright E2E that simulates the home
canvas bootstrap failure and retry success and verifies overlay transitions:
stub the create(CanvasSchema, ...) call to first throw and then resolve, launch
the app, assert the "Unable to create your workspace canvas." UI appears, click
the Retry button which should trigger bootstrapHomeCanvas to succeed and
setHomeCanvasId; next open settings via the UI (triggering handleOpenSettings)
and assert the settings overlay appears, then return home (triggering
handleReturnHome) and assert shellState returns to 'canvas-home' and active node
is the home canvas; finally exercise opening a linked document to trigger
focusDocument/overlay open (respecting OVERLAY_OPEN_DELAY_MS) and assert the
page/database overlay shows and restores viewport when returning. Include
assertions around visible overlayTitle text and that the Retry button & settings
open/close workflows complete.
- Around line 6-10: Reorder the import block so external packages come before
internal `@xnetjs` imports: move the React import (React, useCallback, useEffect,
useMemo, useRef, useState) to be above the `@xnetjs/`* imports that bring in
PageSchema, DatabaseSchema, CanvasSchema, useDevTools, useQuery, useMutate,
CommandPalette, useCommandPalette, and usePrefersReducedMotion; also ensure any
type-only imports (if present) remain before external package imports per repo
conventions.
In
`@docs/explorations/0104_`[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md:
- Around line 523-525: Replace the ephemeral validation notes with durable
references: for the "Automated validation completed..." line, include the CI
workflow run URL or run ID and the commit SHA and link to the build/artifact
that contains the pnpm typecheck/test/build logs; for the "Manual Electron
validation..." line, upload the Playwright screenshot and any electron test
artifacts to the repository CI artifacts (or an S3/GC bucket) and replace the
local path with that permanent URL plus the test run ID and environment
variables used; for the "Manual Electron validation is still required..." line,
add a tracking issue/PR or checklist URL that governs the remaining UX
validations so readers can verify status. Ensure these edits target the exact
strings in the doc ("Automated validation completed...", "Manual Electron
validation on...", "Manual Electron validation is still required...") so the
references are durable and reproducible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 632973cd-f68a-4add-8eeb-255deb280828
📒 Files selected for processing (2)
apps/electron/src/renderer/App.tsxdocs/explorations/0104_[_]_EXPLORE_DRAMATICALLY_SIMPLIFYING_THE_UX_AROUND_A_CANVAS_FIRST_PRIMARY_APP_INSPIRED_BY_AFFINE_MINIMIZING_BUTTONS_AND_CHROME_WITH_ZOOM_IN_DOCUMENTS_AND_DATABASES.md
Comment on lines
+64
to
+99
| const linkedType = document?.type ?? getLinkedType(node) ?? 'canvas' | ||
| const linkedTitle = document?.title ?? (node.properties.title as string) ?? 'Untitled' | ||
| const subtitle = | ||
| linkedType === 'page' | ||
| ? 'Document' | ||
| : linkedType === 'database' | ||
| ? 'Database' | ||
| : isCanvasShellNote(node) | ||
| ? 'Canvas note' | ||
| : 'Canvas' | ||
|
|
||
| const Icon = linkedType === 'page' ? FileText : linkedType === 'database' ? Database : StickyNote | ||
|
|
||
| return ( | ||
| <div className="flex h-full flex-col justify-between rounded-[24px] border border-border/70 bg-background/95 p-4 shadow-lg shadow-black/5"> | ||
| <div className="flex items-start justify-between gap-3"> | ||
| <span className="inline-flex items-center gap-2 rounded-full bg-muted px-2.5 py-1 text-[11px] font-medium uppercase tracking-[0.18em] text-muted-foreground"> | ||
| <Icon size={12} /> | ||
| {subtitle} | ||
| </span> | ||
| {node.linkedNodeId && linkedType !== 'canvas' ? ( | ||
| <span className="text-[11px] uppercase tracking-[0.18em] text-muted-foreground"> | ||
| Open | ||
| </span> | ||
| ) : null} | ||
| </div> | ||
|
|
||
| <div className="space-y-2"> | ||
| <div className="text-lg font-semibold leading-tight text-foreground">{linkedTitle}</div> | ||
| <p className="text-sm leading-relaxed text-muted-foreground"> | ||
| {linkedType === 'database' | ||
| ? 'Open a focused database surface from the canvas.' | ||
| : linkedType === 'page' | ||
| ? 'Open a focused writing surface from the canvas.' | ||
| : 'A lightweight note pinned directly to the workspace.'} | ||
| </p> |
Contributor
There was a problem hiding this comment.
Canvas-linked nodes show wrong icon and description text
When the documents prop includes canvas entries (which it does — App.tsx passes the full documents array including canvases), a canvas-linked node resolves to a LinkedDocumentItem with type: 'canvas'. This causes renderNodeCard to:
- Render the
StickyNoteicon (semantically wrong for a canvas) - Fall into the
elsebranch of the body text at line 97, which outputs'A lightweight note pinned directly to the workspace.'— this is the note-card description, not a canvas description
A user with a legacy canvas-linked node would see misleading copy. The linkedType === 'canvas' arm of the ternaries needs its own icon and description:
const Icon =
linkedType === 'page'
? FileText
: linkedType === 'database'
? Database
: linkedType === 'canvas'
? LayoutTemplate // or Layers / Grid2X2 from lucide-react
: StickyNote
// …inside the body text ternary:
: linkedType === 'canvas'
? 'Open this canvas as the home workspace.'
: 'A lightweight note pinned directly to the workspace.'
Comment on lines
+198
to
+202
| const nodesMap = doc.getMap<CanvasNode>('nodes') | ||
| const targetNode = Array.from(nodesMap.values()).find( | ||
| (node) => node.linkedNodeId === linkedDocumentId | ||
| ) | ||
| if (!targetNode) return null |
Contributor
There was a problem hiding this comment.
Linear node scan allocates a full array copy on every focus call
Array.from(nodesMap.values()).find(...) materialises a complete array from the Y.Map on every call to focusLinkedDocument. For a canvas with many nodes this is an O(n) allocation on what is a hot path (called on every double-click-to-open interaction).
Y.Map can be iterated directly without copying:
let targetNode: CanvasNode | undefined
for (const node of nodesMap.values()) {
if (node.linkedNodeId === linkedDocumentId) {
targetNode = node
break
}
}
if (!targetNode) return null
Comment on lines
+258
to
+268
| const handleReturnHome = useCallback(() => { | ||
| clearTransitionTimer() | ||
| if (shellState.kind === 'page-focus' || shellState.kind === 'database-focus') { | ||
| if (shellState.returnViewport) { | ||
| canvasViewRef.current?.restoreViewport(shellState.returnViewport) | ||
| } | ||
| } | ||
|
|
||
| setShellState({ kind: 'canvas-home' }) | ||
| setActiveNodeId(homeCanvasId) | ||
| }, [clearTransitionTimer, homeCanvasId, setActiveNodeId, shellState]) |
Contributor
There was a problem hiding this comment.
handleReturnHome reconstructed on every shell-state change
shellState is listed as a dependency so handleReturnHome gets a new reference every time any navigation happens (open document, close document, open settings…). Because handleReturnHome is passed directly as the onReturnHome prop of ActionDock, this triggers a full ActionDock re-render on each transition — during exactly the moments when smooth animation is most critical.
The shell state can be read at call-time through a ref to keep the callback stable:
const shellStateRef = useRef(shellState)
shellStateRef.current = shellState // keep in sync, no dep
const handleReturnHome = useCallback(() => {
clearTransitionTimer()
const currentState = shellStateRef.current
if (currentState.kind === 'page-focus' || currentState.kind === 'database-focus') {
if (currentState.returnViewport) {
canvasViewRef.current?.restoreViewport(currentState.returnViewport)
}
}
setShellState({ kind: 'canvas-home' })
setActiveNodeId(homeCanvasId)
}, [clearTransitionTimer, homeCanvasId, setActiveNodeId])
crs48
added a commit
that referenced
this pull request
Mar 7, 2026
* docs(exploration): add canvas-first minimal ux study - analyze the current Electron shell, view chrome, and canvas primitives - recommend a hybrid canvas-first shell with bottom dock and top-right system menu - capture migration phases, tradeoffs, validation steps, and implementation example code * docs(exploration): renumber canvas-first minimal ux study - rename the exploration from 0103 to 0104 - update the document title to match the new numbering * feat(electron): add canvas-first workspace shell - replace the sidebar-first renderer shell - keep a persistent home canvas with overlay focus surfaces - add a bottom action dock and top-right system menu - create linked page and database cards on the canvas - extend canvas viewport APIs for zoom-based transitions - update the exploration checklist for implemented items * feat(electron): tighten focused view chrome - add compact header controls for focused page views - reduce database toolbar density inside the overlay shell - wire compact mode from the new app shell - update the exploration checklist for the page chrome item * chore(repo): allow better-sqlite3 native builds - add pnpm onlyBuiltDependencies entry for better-sqlite3 - unblock sqlite adapter tests in fresh worktrees where native builds were skipped - keep the existing deps:node workflow able to rebuild the native binding before test runs * fix(electron): preserve default canvas node rendering - limit the canvas-first card renderer to linked documents and note cards - keep existing non-document canvas nodes on their default rendering path - reduce the risk of breaking older canvases that contain shapes or other rich node types * test(electron): cover canvas shell card selection - extract the canvas shell card-selection logic into a renderer helper - mark shell-created notes explicitly instead of styling every card node - add a regression test so generic canvas cards keep their default rendering * fix(electron): harden canvas shell review follow-ups - catch home-canvas creation failures - catch linked document creation failures - restore runtime canvas imports - hide onboarding after nodes exist - add a debug-panel action to the system menu - skip zoom-style shell transitions for reduced motion - sanitize restored viewport snapshots * fix(electron): polish canvas shell review follow-ups - add retryable home-canvas bootstrap recovery - limit palette recents and clear transitions before settings - hide non-actionable open affordances on canvas-linked cards - allow electron install scripts in fresh worktrees - record completed validation for the canvas-first exploration * fix(electron): harden canvas-home transition paths - clear delayed overlay timers before forcing canvas-home state - catch shared-session configuration failures locally - align import order with repo conventions - clean up remaining exploration wording and reducer notes * fix(electron): recover when the home canvas disappears - clear stale home canvas ids before re-running bootstrap - keep the canvas-first shell recoverable after external canvas deletion - revalidate the desktop renderer typecheck and build path --------- Co-authored-by: crs <crs@users.noreply.github.com>
crs48
added a commit
that referenced
this pull request
Mar 7, 2026
* docs(exploration): add canvas-first minimal ux study - analyze the current Electron shell, view chrome, and canvas primitives - recommend a hybrid canvas-first shell with bottom dock and top-right system menu - capture migration phases, tradeoffs, validation steps, and implementation example code * docs(exploration): renumber canvas-first minimal ux study - rename the exploration from 0103 to 0104 - update the document title to match the new numbering * feat(electron): add canvas-first workspace shell - replace the sidebar-first renderer shell - keep a persistent home canvas with overlay focus surfaces - add a bottom action dock and top-right system menu - create linked page and database cards on the canvas - extend canvas viewport APIs for zoom-based transitions - update the exploration checklist for implemented items * feat(electron): tighten focused view chrome - add compact header controls for focused page views - reduce database toolbar density inside the overlay shell - wire compact mode from the new app shell - update the exploration checklist for the page chrome item * chore(repo): allow better-sqlite3 native builds - add pnpm onlyBuiltDependencies entry for better-sqlite3 - unblock sqlite adapter tests in fresh worktrees where native builds were skipped - keep the existing deps:node workflow able to rebuild the native binding before test runs * fix(electron): preserve default canvas node rendering - limit the canvas-first card renderer to linked documents and note cards - keep existing non-document canvas nodes on their default rendering path - reduce the risk of breaking older canvases that contain shapes or other rich node types * test(electron): cover canvas shell card selection - extract the canvas shell card-selection logic into a renderer helper - mark shell-created notes explicitly instead of styling every card node - add a regression test so generic canvas cards keep their default rendering * fix(electron): harden canvas shell review follow-ups - catch home-canvas creation failures - catch linked document creation failures - restore runtime canvas imports - hide onboarding after nodes exist - add a debug-panel action to the system menu - skip zoom-style shell transitions for reduced motion - sanitize restored viewport snapshots * fix(electron): polish canvas shell review follow-ups - add retryable home-canvas bootstrap recovery - limit palette recents and clear transitions before settings - hide non-actionable open affordances on canvas-linked cards - allow electron install scripts in fresh worktrees - record completed validation for the canvas-first exploration * fix(electron): harden canvas-home transition paths - clear delayed overlay timers before forcing canvas-home state - catch shared-session configuration failures locally - align import order with repo conventions - clean up remaining exploration wording and reducer notes * fix(electron): recover when the home canvas disappears - clear stale home canvas ids before re-running bootstrap - keep the canvas-first shell recoverable after external canvas deletion - revalidate the desktop renderer typecheck and build path --------- Co-authored-by: crs <crs@users.noreply.github.com>
This was referenced Mar 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
better-sqlite3so the repo test/push path works in fresh worktreesValidation
Summary by CodeRabbit
New Features
Documentation
Tests
Chores