Skip to content

feat(electron): finish canvas-first shell chrome#6

Merged
crs48 merged 2 commits intomainfrom
codex/finish-canvas-shell-chrome
Mar 6, 2026
Merged

feat(electron): finish canvas-first shell chrome#6
crs48 merged 2 commits intomainfrom
codex/finish-canvas-shell-chrome

Conversation

@crs48
Copy link
Copy Markdown
Owner

@crs48 crs48 commented Mar 6, 2026

Summary

  • compress the focused database overlay header into direct actions plus one overflow menu
  • replace custom canvas fit/reset controls with shared NavigationTools integration
  • center new canvas-linked documents and notes on the current viewport and update the 0104 exploration checklist

Testing

  • pnpm --dir apps/electron exec vitest run src/renderer/components/DatabaseView.test.tsx src/renderer/lib/canvas-shell.test.ts
  • pnpm --dir packages/canvas exec vitest run src/__tests__/canvas-navigation-shell.test.tsx src/__tests__/navigation-tools.test.ts
  • pnpm exec vitest run packages/data/src/store/sqlite-adapter.test.ts packages/storage/src/adapters/sqlite.test.ts
  • pre-commit hooks
  • pre-push hooks (pnpm typecheck && pnpm test)

Summary by CodeRabbit

  • New Features

    • Added configurable navigation tools to canvas with position, zoom label visibility, and styling options.
    • Introduced database view mode toggle for switching between table and board views.
    • Added overflow menu in database toolbar with schema info, clone, and share options.
    • Enhanced minimal chrome mode with comments button and compact controls.
  • Tests

    • Added test suites for DatabaseView and Canvas navigation tools.
  • Documentation

    • Updated exploration checklist status.

crs added 2 commits March 6, 2026 11:43
- compress the focused database header into direct actions
  plus an overflow menu
- replace custom canvas fit/reset controls with shared
  NavigationTools integration
- add automated coverage for database focused chrome,
  shared canvas navigation, and viewport-centered placement
- update the exploration checklist to reflect implemented
  and validated items
- alias @xnetjs/sqlite/memory to the workspace source adapter
- allow root vitest runs and pre-push hooks to resolve sqlite memory tests without a package build
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 6, 2026

📝 Walkthrough

Walkthrough

The changes implement canvas-first UI improvements by introducing placement helper functions for canvas nodes, replacing the dock navigation with built-in NavigationTools, adding minimalChrome mode support to DatabaseView with new internal components, extending Vitest configurations with module path aliases, and adding comprehensive test coverage for DatabaseView and canvas navigation tools.

Changes

Cohort / File(s) Summary
Canvas Navigation & Placement
apps/electron/src/renderer/components/CanvasView.tsx, packages/canvas/src/components/NavigationTools.tsx, packages/canvas/src/renderer/Canvas.tsx, apps/electron/src/renderer/lib/canvas-shell.ts
Introduces new placement helpers (getCanvasShellNotePlacement, getLinkedDocumentPlacement) with CanvasViewportSnapshot type; replaces hard-coded node coordinates with placement functions; removes dock navigation and enables built-in NavigationTools with configurable props (position, zoom label, styling); adds internal viewport state and change handler to sync navigation actions back to canvas.
Canvas Shell Tests
apps/electron/src/renderer/lib/canvas-shell.test.ts, packages/canvas/src/__tests__/canvas-navigation-shell.test.tsx
Adds comprehensive test suites for placement helpers validating centering and sizing behavior; adds canvas navigation shell tests verifying NavigationTools rendering and navigation action callbacks.
DatabaseView UI Enhancements
apps/electron/src/renderer/components/DatabaseView.tsx, apps/electron/src/renderer/components/DatabaseView.test.tsx
Introduces new internal components DatabaseViewModeToggle and DatabaseOverflowMenu; adds minimalChrome mode with conditional rendering paths for toolbar, comments, and share button; adds comprehensive test suite covering minimal chrome rendering, comments accessibility, and header behavior.
Vitest Configuration
vitest.config.ts, apps/electron/vitest.config.ts, packages/canvas/vitest.config.ts
Adds module path aliases for workspace packages (@xnetjs/canvas, @xnetjs/data, @xnetjs/react, etc.) to resolve dependencies during tests; expands electron app test patterns to include .tsx files.
Documentation
docs/explorations/0104_...md
Updates exploration checklist with completion status for canvas-first UX items.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

enhancement

Poem

🐰 Canvas nodes find their place,
No more fixed coordinates—helpers embrace!
NavigationTools float with grace,
DatabaseView dons minimalChrome face,
Tests ensure the magic stays in place! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(electron): finish canvas-first shell chrome' clearly and specifically summarizes the main changes: completing the canvas-first UI shell with simplified chrome/header design, NavigationTools integration, and viewport-centered node placement.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/finish-canvas-shell-chrome

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 6, 2026

Preview: https://xnet.fyi/pr/6/app/

github-actions bot added a commit that referenced this pull request Mar 6, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 6, 2026

Greptile Summary

This PR completes the canvas-first shell chrome by compressing the focused DatabaseView header into a compact minimalChrome toolbar (with an overflow menu for Share/Schema/Clone), replacing the custom fit/reset icon buttons in CanvasView with the shared NavigationTools component, and extracting viewport-centered placement logic for new notes and linked documents into testable helper functions. It also broadens the root and per-package vitest configs to resolve workspace packages from source.

Key changes and findings:

  • canvas-shell.ts — new getCanvasShellNotePlacement and getLinkedDocumentPlacement helpers correctly center nodes on the viewport; getLinkedDocumentPlacement intentionally excludes the 'canvas' doc type via Exclude<LinkedDocType, 'canvas'>.
  • CanvasView.tsx — the call getLinkedDocumentPlacement(viewport, document.type) passes the wider LinkedDocType (which includes 'canvas') to a parameter typed as Exclude<LinkedDocType, 'canvas'>. This is a TypeScript error and will throw a runtime TypeError if a canvas-typed document is ever passed.
  • DatabaseView.tsxDatabaseViewModeToggle and DatabaseOverflowMenu are cleanly extracted; the two independent render paths (empty state vs. loaded state) both correctly gate minimalChrome controls.
  • NavigationTools.tsx — viewport prop is relaxed to a Pick type and a style override prop is added, both non-breaking.
  • apps/electron/vitest.config.ts — the @testing-library/react alias is hard-wired to packages/editor/node_modules/@testing-library/react, which is fragile to package-manager hoisting or restructuring.
  • CanvasView.tsx stylenavigationToolsStyle uses hardcoded rgba(255,255,255,0.8) and slate colour values that will not adapt to dark mode the way the rest of the Tailwind-based shell does.

Confidence Score: 3/5

  • Not safe to merge until the LinkedDocType / getLinkedDocumentPlacement type mismatch is resolved; the fragile test alias and dark-mode colour values are lower-priority but worth fixing.
  • The overall refactor is well-structured and test coverage is solid, but there is a concrete TypeScript type error in CanvasView.tsx where document.type: LinkedDocType (includes 'canvas') is passed directly to getLinkedDocumentPlacement which only accepts Exclude<LinkedDocType, 'canvas'>. At runtime this produces a TypeError for any canvas-type document. The @testing-library/react cross-package alias is also brittle. These issues lower confidence from what would otherwise be a clean 4.
  • apps/electron/src/renderer/components/CanvasView.tsx (type mismatch at line 171) and apps/electron/vitest.config.ts (fragile cross-package alias).

Important Files Changed

Filename Overview
apps/electron/src/renderer/components/CanvasView.tsx Refactors note/document placement into shared helper functions and replaces custom fit/reset buttons with the shared NavigationTools integration. Contains a type mismatch: document.type (LinkedDocType, includes 'canvas') is passed directly to getLinkedDocumentPlacement which only accepts Exclude<LinkedDocType, 'canvas'>, causing a TypeScript error and a potential runtime crash. navigationToolsStyle also uses hardcoded light-mode colour values that will not adapt to dark mode.
apps/electron/src/renderer/components/DatabaseView.tsx Compresses the focused database overlay header into a compact toolbar: extracts DatabaseViewModeToggle and DatabaseOverflowMenu sub-components, moves Share into the overflow menu for minimalChrome mode, and adds accessible Comments button. Code is clean and well-structured; the two render paths (empty state vs. loaded state) each correctly gate minimalChrome controls.
apps/electron/src/renderer/lib/canvas-shell.ts Adds getCanvasShellNotePlacement and getLinkedDocumentPlacement helpers that correctly center new nodes on the current viewport. The Exclude<LinkedDocType, 'canvas'> constraint on getLinkedDocumentPlacement is appropriate, but its call site in CanvasView.tsx passes the wider LinkedDocType without narrowing.
packages/canvas/src/renderer/Canvas.tsx Adds five new navigationTools* props to Canvas and wires them to the NavigationTools component via handleNavigationViewportChange. The canvasBounds memo correctly lists nodes as a trigger dependency. Integration looks solid.
apps/electron/vitest.config.ts Adds .tsx to the test glob and introduces workspace package aliases. The @testing-library/react alias is hard-wired to packages/editor/node_modules/@testing-library/react, making it fragile to dependency hoisting or restructuring. All other aliases point to workspace source files which is a standard monorepo pattern.

Sequence Diagram

sequenceDiagram
    participant User
    participant CanvasView
    participant Canvas
    participant NavigationTools
    participant CanvasShell

    User->>CanvasView: click "Add note"
    CanvasView->>Canvas: getViewportSnapshot()
    Canvas-->>CanvasView: { x, y, zoom }
    CanvasView->>CanvasShell: getCanvasShellNotePlacement(viewport)
    CanvasShell-->>CanvasView: { x, y, width:320, height:180 }
    CanvasView->>Canvas: createNode('card', placement, shellProps)

    User->>CanvasView: click "Add linked document"
    CanvasView->>Canvas: getViewportSnapshot()
    Canvas-->>CanvasView: { x, y, zoom }
    CanvasView->>CanvasShell: getLinkedDocumentPlacement(viewport, doc.type)
    CanvasShell-->>CanvasView: { x, y, width, height }
    CanvasView->>Canvas: createNode('embed', placement, { title, linkedType })

    User->>NavigationTools: click "Fit to content"
    NavigationTools->>Canvas: onViewportChange({ x: boundsCenter.x, y: boundsCenter.y, zoom })
    Canvas->>Canvas: setViewportSnapshot(merged)

    User->>NavigationTools: click "Reset view"
    NavigationTools->>Canvas: onViewportChange({ x:0, y:0, zoom:1 })
    Canvas->>Canvas: setViewportSnapshot({ x:0, y:0, zoom:1 })
Loading

Comments Outside Diff (1)

  1. apps/electron/src/renderer/components/CanvasView.tsx, line 95-103 (link)

    Hardcoded colours in navigationToolsStyle will break dark mode

    background: 'rgba(255, 255, 255, 0.8)', boxShadow: '0 18px 38px rgba(15, 23, 42, 0.12)', and border: '1px solid rgba(148, 163, 184, 0.28)' are hard-coded light-mode values passed via the inline style prop to NavigationTools. Because they bypass the CSS-variable / Tailwind token system used everywhere else in the shell (e.g. bg-background/80, border-border/70), the navigation toolbar will remain white in dark mode while the rest of the UI adapts.

    Consider exposing a navigationToolsClassName prop path instead, so Tailwind utility classes (and their dark-mode variants) can be used, or at minimum use CSS variables like var(--color-background) for the colour values.

Last reviewed commit: ba3fa74

linkedType: document.type
}
)
const linkedNode = createNode('embed', getLinkedDocumentPlacement(viewport, document.type), {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type mismatch will crash at runtime for 'canvas'-type documents

document is typed as LinkedDocumentItem, whose type field is LinkedDocType = 'page' | 'database' | 'canvas'. getLinkedDocumentPlacement was purposefully declared with type: Exclude<LinkedDocType, 'canvas'> (i.e. 'page' | 'database'), so TypeScript should already flag this line as a type error.

More critically, if addLinkedDocumentNode is ever called with a document whose type === 'canvas', LINKED_DOCUMENT_SIZES['canvas'] returns undefined, and the subsequent size.width access will throw a TypeError at runtime.

A straightforward fix is to guard against the 'canvas' case before the call:

Suggested change
const linkedNode = createNode('embed', getLinkedDocumentPlacement(viewport, document.type), {
const linkedNode = createNode('embed', getLinkedDocumentPlacement(viewport, document.type as Exclude<LinkedDocType, 'canvas'>), {

Or more safely, add an explicit guard:

if (document.type === 'canvas') return
const linkedNode = createNode('embed', getLinkedDocumentPlacement(viewport, document.type), {

Comment on lines +16 to +18
},
resolve: {
alias: {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fragile cross-package node_modules path for @testing-library/react

The alias resolves @testing-library/react to the copy installed inside packages/editor/node_modules. This will silently break if:

  • packages/editor installs or removes @testing-library/react independently
  • The package manager hoists or deduplicates the dep and removes the nested copy
  • The monorepo structure changes

The robust fix is to add @testing-library/react as a direct devDependency in apps/electron/package.json and let the normal resolution pick it up, then remove this alias entirely.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

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)

1527-1595: ⚠️ Potential issue | 🔴 Critical

The empty-state branch short-circuits all overlay-driven actions.

This branch returns before CommentsSidebar, AddColumnModal, and SchemaInfoModal are mounted later in the file. In practice, the new minimal-chrome Comments/Schema actions are no-ops here, and the Add Column flow in the same branch cannot open anything either.

🐛 Safer structure
-  if (columns.length === 0) {
-    return (
-      <div ref={containerRef} className="flex-1 flex flex-col overflow-hidden">
-        ...
-      </div>
-    )
-  }
-
+  const isEmpty = columns.length === 0
   return (
     <div ref={containerRef} className="flex-1 flex flex-col overflow-hidden">
-      ...
+      {isEmpty ? (
+        ...
+      ) : (
+        ...
+      )}
+
+      <CommentsSidebar ... />
+      <AddColumnModal ... />
+      <SchemaInfoModal ... />
+      <CloneSchemaModal ... />
     </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 1527 -
1595, The empty-state block currently short-circuits the component tree so
overlay-driven components (CommentsSidebar, AddColumnModal, SchemaInfoModal)
never mount; remove the early return/short-circuit and instead render the
empty-state UI inline (keep the "This database has no columns yet." message and
Add Column button) while allowing the rest of the component to continue
mounting; ensure the handlers used by those overlays (e.g., handleAddColumn,
setSidebarOpen, setSchemaInfoModalOpen, setCloneSchemaModalOpen) remain intact
and that CommentsSidebar, AddColumnModal, and SchemaInfoModal stay rendered
later in the file so the minimal-chrome buttons and Add Column flow function
correctly.
🧹 Nitpick comments (2)
packages/canvas/src/__tests__/canvas-navigation-shell.test.tsx (1)

1-4: Add the required package file header.

This new packages/* test file is missing the repo’s file-level JSDoc banner.

✍️ Suggested header
+/** `@xnetjs/canvas` - Canvas navigation shell tests for xNet */
 import { fireEvent, render, screen } from '@testing-library/react'

As per coding guidelines, packages/**/*.{ts,tsx}: Include file-level JSDoc at top of file: /** @xnetjs/sync - Unified sync primitives for xNet */

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/canvas/src/__tests__/canvas-navigation-shell.test.tsx` around lines
1 - 4, Add the required file-level JSDoc banner to the top of this test file by
inserting the exact header /** `@xnetjs/sync` - Unified sync primitives for xNet
*/ above the existing imports (before the import of React/fireEvent etc.);
ensure the banner appears at the very top of packages/* test files so the test
file (which contains imports like render, screen from '@testing-library/react'
and Y from 'yjs') conforms to the repository coding guideline.
apps/electron/src/renderer/components/DatabaseView.tsx (1)

31-54: Reorder the updated import block to match repo rules.

CommentThreadData should come from an import type, and the external imports (lucide-react, react, yjs) should sit above the @xnetjs/* imports.

As per coding guidelines, **/*.{ts,tsx}: Type-only imports must come first: import type { Foo } from './types' and Internal package imports should follow external packages: import { hash } from '@xnetjs/crypto'

🤖 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 31 - 54,
Reorder and adjust the imports so external packages come before internal
`@xnetjs/`* imports and use type-only imports for type symbols: move imports from
"lucide-react", "react" (and "yjs" if present) above the `@xnetjs/ui` and
`@xnetjs/views` blocks, and change CommentThreadData to a type-only import (import
type { CommentThreadData } from '@xnetjs/ui'); ensure other types from
`@xnetjs/views` (ViewConfig, TableRow, CellPresence, NewColumnDefinition) remain
as type imports if appropriate and keep runtime imports (CommentPopover,
TableView, BoardView, etc.) as plain imports.
🤖 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/components/CanvasView.tsx`:
- Around line 171-174: The call to getLinkedDocumentPlacement is passing
document.type which may be 'canvas' but that function expects
Exclude<LinkedDocType, 'canvas'>; add a guard before creating linkedNode in
addLinkedDocumentNode (or the surrounding function) to early-return or handle
document.type === 'canvas' so only 'page' or 'database' reach
createNode('embed', getLinkedDocumentPlacement(...)), or alternatively narrow
the parameter type of addLinkedDocumentNode to exclude 'canvas'; update
references to linkedNode, createNode, and getLinkedDocumentPlacement accordingly
so the placement call never receives a 'canvas' value.

In `@apps/electron/vitest.config.ts`:
- Around line 19-22: Remove the manual alias entry for '@testing-library/react'
in the vitest config (the resolve(...) block that points to
'../../packages/editor/node_modules/@testing-library/react') and instead add
'@testing-library/react' as a dependency/devDependency in the apps/electron
package.json so Vitest can resolve it normally; update the vitest config to stop
forcing that module path and rely on normal module resolution.

In `@packages/canvas/src/renderer/Canvas.tsx`:
- Around line 688-697: Canvas's NavigationTools is clamping to hardcoded 0.1–4,
causing mismatch with the canvas config; thread the canvas zoom limits through
the toolbar by passing config.minZoom and config.maxZoom into the
NavigationTools instance in Canvas.tsx (add props like minZoom={config.minZoom}
and maxZoom={config.maxZoom} to the component render), and update
NavigationTools' props/types to accept and use those minZoom/maxZoom values
instead of the hardcoded 0.1/4 so the toolbar slider/buttons respect the
configured bounds.

In `@vitest.config.ts`:
- Around line 7-34: The workspaceAliases entries are using URL.pathname which
can break on Windows and with percent-encoded paths; update every alias value in
workspaceAliases (all entries including '@xnetjs/sqlite/memory') to wrap the URL
with fileURLToPath(new URL(...)) instead of using .pathname, and likewise change
the mermaid alias to use fileURLToPath(new URL(...)); also add the import for
fileURLToPath from 'url' at the top of the file if it’s missing so all aliases
resolve correctly across platforms.

---

Outside diff comments:
In `@apps/electron/src/renderer/components/DatabaseView.tsx`:
- Around line 1527-1595: The empty-state block currently short-circuits the
component tree so overlay-driven components (CommentsSidebar, AddColumnModal,
SchemaInfoModal) never mount; remove the early return/short-circuit and instead
render the empty-state UI inline (keep the "This database has no columns yet."
message and Add Column button) while allowing the rest of the component to
continue mounting; ensure the handlers used by those overlays (e.g.,
handleAddColumn, setSidebarOpen, setSchemaInfoModalOpen,
setCloneSchemaModalOpen) remain intact and that CommentsSidebar, AddColumnModal,
and SchemaInfoModal stay rendered later in the file so the minimal-chrome
buttons and Add Column flow function correctly.

---

Nitpick comments:
In `@apps/electron/src/renderer/components/DatabaseView.tsx`:
- Around line 31-54: Reorder and adjust the imports so external packages come
before internal `@xnetjs/`* imports and use type-only imports for type symbols:
move imports from "lucide-react", "react" (and "yjs" if present) above the
`@xnetjs/ui` and `@xnetjs/views` blocks, and change CommentThreadData to a type-only
import (import type { CommentThreadData } from '@xnetjs/ui'); ensure other types
from `@xnetjs/views` (ViewConfig, TableRow, CellPresence, NewColumnDefinition)
remain as type imports if appropriate and keep runtime imports (CommentPopover,
TableView, BoardView, etc.) as plain imports.

In `@packages/canvas/src/__tests__/canvas-navigation-shell.test.tsx`:
- Around line 1-4: Add the required file-level JSDoc banner to the top of this
test file by inserting the exact header /** `@xnetjs/sync` - Unified sync
primitives for xNet */ above the existing imports (before the import of
React/fireEvent etc.); ensure the banner appears at the very top of packages/*
test files so the test file (which contains imports like render, screen from
'@testing-library/react' and Y from 'yjs') conforms to the repository coding
guideline.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4acfca92-0258-42b2-8954-4f1f83861381

📥 Commits

Reviewing files that changed from the base of the PR and between aa44ca1 and ba3fa74.

📒 Files selected for processing (12)
  • apps/electron/src/renderer/components/CanvasView.tsx
  • apps/electron/src/renderer/components/DatabaseView.test.tsx
  • apps/electron/src/renderer/components/DatabaseView.tsx
  • apps/electron/src/renderer/lib/canvas-shell.test.ts
  • apps/electron/src/renderer/lib/canvas-shell.ts
  • apps/electron/vitest.config.ts
  • 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
  • packages/canvas/src/__tests__/canvas-navigation-shell.test.tsx
  • packages/canvas/src/components/NavigationTools.tsx
  • packages/canvas/src/renderer/Canvas.tsx
  • packages/canvas/vitest.config.ts
  • vitest.config.ts

Comment on lines +171 to +174
const linkedNode = createNode('embed', getLinkedDocumentPlacement(viewport, document.type), {
title: document.title,
linkedType: document.type
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C2 \
  "export type LinkedDocType|export type LinkedDocumentItem|export function getLinkedDocumentPlacement|addLinkedDocumentNode|getLinkedDocumentPlacement\\(viewport, document\\.type\\)" \
  apps/electron/src/renderer/lib/canvas-shell.ts \
  apps/electron/src/renderer/components/CanvasView.tsx

Repository: crs48/xNet

Length of output: 3212


🏁 Script executed:

rg -n "export type LinkedDocumentItem" -A 10 apps/electron/src/renderer/lib/canvas-shell.ts

Repository: crs48/xNet

Length of output: 250


Add a guard to exclude 'canvas' documents from this placement call.

getLinkedDocumentPlacement() explicitly expects Exclude<LinkedDocType, 'canvas'>, but document.type has type LinkedDocType which includes 'canvas'. Guard the call to ensure only 'page' or 'database' documents reach this code, or narrow the parameter type of addLinkedDocumentNode to exclude canvas documents.

🤖 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 171 - 174,
The call to getLinkedDocumentPlacement is passing document.type which may be
'canvas' but that function expects Exclude<LinkedDocType, 'canvas'>; add a guard
before creating linkedNode in addLinkedDocumentNode (or the surrounding
function) to early-return or handle document.type === 'canvas' so only 'page' or
'database' reach createNode('embed', getLinkedDocumentPlacement(...)), or
alternatively narrow the parameter type of addLinkedDocumentNode to exclude
'canvas'; update references to linkedNode, createNode, and
getLinkedDocumentPlacement accordingly so the placement call never receives a
'canvas' value.

Comment on lines +19 to +22
'@testing-library/react': resolve(
__dirname,
'../../packages/editor/node_modules/@testing-library/react'
),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python - <<'PY'
import json, pathlib

def dep_version(manifest, name):
    return (
        manifest.get('dependencies', {}).get(name)
        or manifest.get('devDependencies', {}).get(name)
        or manifest.get('peerDependencies', {}).get(name)
    )

for rel in ('apps/electron/package.json', 'packages/editor/package.json'):
    path = pathlib.Path(rel)
    if not path.exists():
        print(f'{rel}: missing')
        continue
    manifest = json.loads(path.read_text())
    print(f'{rel}: `@testing-library/react` -> {dep_version(manifest, "@testing-library/react")}')
PY

Repository: crs48/xNet

Length of output: 176


Remove the @testing-library/react alias that depends on packages/editor's installation.

Electron tests do not declare @testing-library/react in their own package.json, but the Vitest config resolves it from packages/editor/node_modules/@testing-library/react. This creates an implicit cross-package dependency that will break if packages/editor ever removes or updates this library, or in pruned installs.

Add @testing-library/react to apps/electron/package.json and allow Vitest to resolve it normally.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/electron/vitest.config.ts` around lines 19 - 22, Remove the manual alias
entry for '@testing-library/react' in the vitest config (the resolve(...) block
that points to '../../packages/editor/node_modules/@testing-library/react') and
instead add '@testing-library/react' as a dependency/devDependency in the
apps/electron package.json so Vitest can resolve it normally; update the vitest
config to stop forcing that module path and rely on normal module resolution.

Comment on lines +688 to +697
{showNavigationTools && (
<NavigationTools
viewport={viewport}
canvasBounds={canvasBounds}
onViewportChange={handleNavigationViewportChange}
position={navigationToolsPosition}
showZoomLabel={navigationToolsShowZoomLabel}
className={navigationToolsClassName}
style={navigationToolsStyle}
/>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Thread config.minZoom / config.maxZoom into this toolbar.

packages/canvas/src/components/NavigationTools.tsx still clamps its buttons and slider to 0.1–4. Wiring it in here without the canvas's configured bounds makes the shared controls disagree with wheel/pinch and shortcut behavior for any consumer using non-default zoom limits.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/canvas/src/renderer/Canvas.tsx` around lines 688 - 697, Canvas's
NavigationTools is clamping to hardcoded 0.1–4, causing mismatch with the canvas
config; thread the canvas zoom limits through the toolbar by passing
config.minZoom and config.maxZoom into the NavigationTools instance in
Canvas.tsx (add props like minZoom={config.minZoom} and maxZoom={config.maxZoom}
to the component render), and update NavigationTools' props/types to accept and
use those minZoom/maxZoom values instead of the hardcoded 0.1/4 so the toolbar
slider/buttons respect the configured bounds.

Comment on lines +7 to +34
const workspaceAliases = {
'@xnetjs/canvas': new URL('./packages/canvas/src/index.ts', import.meta.url).pathname,
'@xnetjs/cli': new URL('./packages/cli/src/index.ts', import.meta.url).pathname,
'@xnetjs/core': new URL('./packages/core/src/index.ts', import.meta.url).pathname,
'@xnetjs/crypto': new URL('./packages/crypto/src/index.ts', import.meta.url).pathname,
'@xnetjs/data': new URL('./packages/data/src/index.ts', import.meta.url).pathname,
'@xnetjs/data-bridge': new URL('./packages/data-bridge/src/index.ts', import.meta.url).pathname,
'@xnetjs/devtools': new URL('./packages/devtools/src/index.ts', import.meta.url).pathname,
'@xnetjs/editor': new URL('./packages/editor/src/index.ts', import.meta.url).pathname,
'@xnetjs/formula': new URL('./packages/formula/src/index.ts', import.meta.url).pathname,
'@xnetjs/history': new URL('./packages/history/src/index.ts', import.meta.url).pathname,
'@xnetjs/hub': new URL('./packages/hub/src/index.ts', import.meta.url).pathname,
'@xnetjs/identity': new URL('./packages/identity/src/index.ts', import.meta.url).pathname,
'@xnetjs/network': new URL('./packages/network/src/index.ts', import.meta.url).pathname,
'@xnetjs/plugins': new URL('./packages/plugins/src/index.ts', import.meta.url).pathname,
'@xnetjs/query': new URL('./packages/query/src/index.ts', import.meta.url).pathname,
'@xnetjs/react': new URL('./packages/react/src/index.ts', import.meta.url).pathname,
'@xnetjs/sdk': new URL('./packages/sdk/src/index.ts', import.meta.url).pathname,
'@xnetjs/sqlite/memory': new URL('./packages/sqlite/src/adapters/memory.ts', import.meta.url)
.pathname,
'@xnetjs/sqlite': new URL('./packages/sqlite/src/index.ts', import.meta.url).pathname,
'@xnetjs/storage': new URL('./packages/storage/src/index.ts', import.meta.url).pathname,
'@xnetjs/sync': new URL('./packages/sync/src/index.ts', import.meta.url).pathname,
'@xnetjs/telemetry': new URL('./packages/telemetry/src/index.ts', import.meta.url).pathname,
'@xnetjs/ui': new URL('./packages/ui/src/index.ts', import.meta.url).pathname,
'@xnetjs/vectors': new URL('./packages/vectors/src/index.ts', import.meta.url).pathname,
'@xnetjs/views': new URL('./packages/views/src/index.ts', import.meta.url).pathname
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Node.js ESM, what problems does new URL('./file', import.meta.url).pathnamehave for filesystem paths compared withfileURLToPath(new URL('./file', import.meta.url)), especially on Windows or when the path contains spaces?

💡 Result:

Using new URL('./file', import.meta.url).pathname gives you the URL path component, not a real platform filesystem path, so it breaks in a few common cases that fileURLToPath(new URL('./file', import.meta.url)) handles correctly:

  • Percent-encoding is left in place: spaces and non-ASCII characters stay encoded (e.g. "hello world""/hello%20world", "你好.txt""/%E4%BD%A0%E5%A5%BD.txt"), which is not a valid literal pathname for many filesystem APIs unless you manually decode it. fileURLToPath() decodes correctly. [1]
  • Windows drive letters get an extra leading / and wrong separators: file:///C:/path/ becomes "/C:/path/" via .pathname, not C:\path\. fileURLToPath() returns the proper Windows path. [1][2]
  • UNC paths lose the host: file://nas/foo.txt becomes "/foo.txt" via .pathname (host dropped), while fileURLToPath() returns \\nas\foo.txt. [1]

In short: .pathname is for URL semantics; fileURLToPath() is the Node-supported way to turn a file: URL into a correct, decoded, platform-specific filesystem path. [1]


🏁 Script executed:

# Check if vitest.config.ts exists and examine the full file
if [ -f "vitest.config.ts" ]; then
  echo "=== File exists ==="
  wc -l vitest.config.ts
  echo ""
  echo "=== Full content ==="
  cat -n vitest.config.ts
else
  echo "vitest.config.ts not found"
fi

Repository: crs48/xNet

Length of output: 6972


Use fileURLToPath for all workspace and mock aliases.

URL.pathname does not decode percent-encoded characters (spaces become %20, non-ASCII stays encoded) and mishandles Windows drive letters and UNC paths. This breaks alias resolution on Windows and in paths with spaces. Switch all 26 workspaceAliases entries and the mermaid alias (line 92) to use fileURLToPath(new URL(...)).

🛠️ Fix pattern
-import { defineConfig } from 'vitest/config'
+import { fileURLToPath } from 'node:url'
+import { defineConfig } from 'vitest/config'

 const workspaceAliases = {
-  '@xnetjs/canvas': new URL('./packages/canvas/src/index.ts', import.meta.url).pathname,
+  '@xnetjs/canvas': fileURLToPath(new URL('./packages/canvas/src/index.ts', import.meta.url)),
   // ...apply to all remaining aliases
 }

 // Also fix line 92:
-            mermaid: new URL('./packages/canvas/src/__mocks__/mermaid.ts', import.meta.url).pathname
+            mermaid: fileURLToPath(new URL('./packages/canvas/src/__mocks__/mermaid.ts', import.meta.url))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vitest.config.ts` around lines 7 - 34, The workspaceAliases entries are using
URL.pathname which can break on Windows and with percent-encoded paths; update
every alias value in workspaceAliases (all entries including
'@xnetjs/sqlite/memory') to wrap the URL with fileURLToPath(new URL(...))
instead of using .pathname, and likewise change the mermaid alias to use
fileURLToPath(new URL(...)); also add the import for fileURLToPath from 'url' at
the top of the file if it’s missing so all aliases resolve correctly across
platforms.

@crs48 crs48 merged commit 156576e into main Mar 6, 2026
8 checks passed
@crs48 crs48 deleted the codex/finish-canvas-shell-chrome branch March 6, 2026 21:09
crs48 added a commit that referenced this pull request Mar 7, 2026
* feat(electron): finish canvas-first shell chrome

- compress the focused database header into direct actions
  plus an overflow menu
- replace custom canvas fit/reset controls with shared
  NavigationTools integration
- add automated coverage for database focused chrome,
  shared canvas navigation, and viewport-centered placement
- update the exploration checklist to reflect implemented
  and validated items

* fix(test): resolve sqlite memory subpath in root vitest

- alias @xnetjs/sqlite/memory to the workspace source adapter
- allow root vitest runs and pre-push hooks to resolve sqlite memory tests without a package build

---------

Co-authored-by: crs <crs@users.noreply.github.com>
crs48 added a commit that referenced this pull request Mar 7, 2026
* feat(electron): finish canvas-first shell chrome

- compress the focused database header into direct actions
  plus an overflow menu
- replace custom canvas fit/reset controls with shared
  NavigationTools integration
- add automated coverage for database focused chrome,
  shared canvas navigation, and viewport-centered placement
- update the exploration checklist to reflect implemented
  and validated items

* fix(test): resolve sqlite memory subpath in root vitest

- alias @xnetjs/sqlite/memory to the workspace source adapter
- allow root vitest runs and pre-push hooks to resolve sqlite memory tests without a package build

---------

Co-authored-by: crs <crs@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants