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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 20 additions & 40 deletions apps/electron/src/renderer/components/CanvasView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ 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 { Database, FileText, StickyNote } from 'lucide-react'
import React, {
forwardRef,
useCallback,
Expand All @@ -19,6 +18,8 @@ import React, {
} from 'react'
import {
createCanvasShellNoteProperties,
getCanvasShellNotePlacement,
getLinkedDocumentPlacement,
isCanvasShellNote,
shouldRenderCanvasShellCard,
type LinkedDocType,
Expand Down Expand Up @@ -154,12 +155,7 @@ export const CanvasView = forwardRef<CanvasViewHandle, CanvasViewProps>(function
const nodesMap = doc.getMap<CanvasNode>('nodes')
const noteNode = createNode(
'card',
{
x: viewport.x - 160,
y: viewport.y - 100,
width: 320,
height: 180
},
getCanvasShellNotePlacement(viewport),
createCanvasShellNoteProperties()
)

Expand All @@ -172,19 +168,10 @@ export const CanvasView = forwardRef<CanvasViewHandle, CanvasViewProps>(function

const viewport = canvasRef.current.getViewportSnapshot()
const nodesMap = doc.getMap<CanvasNode>('nodes')
const linkedNode = createNode(
'embed',
{
x: viewport.x - (document.type === 'database' ? 220 : 180),
y: viewport.y - 120,
width: document.type === 'database' ? 440 : 360,
height: document.type === 'database' ? 260 : 220
},
{
title: document.title,
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), {

title: document.title,
linkedType: document.type
})
Comment on lines +171 to +174

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.

linkedNode.linkedNodeId = document.id
nodesMap.set(linkedNode.id, linkedNode)
},
Expand Down Expand Up @@ -257,25 +244,6 @@ export const CanvasView = forwardRef<CanvasViewHandle, CanvasViewProps>(function
</div>
) : null}

<div className="pointer-events-none absolute bottom-24 right-6 z-20 flex items-center gap-2">
<div className="pointer-events-auto rounded-[24px] border border-border/70 bg-background/80 p-2 shadow-xl backdrop-blur-xl">
<IconButton
icon={<Maximize2 size={16} />}
label="Fit canvas to content"
onClick={() => canvasRef.current?.fitToContent(80)}
className="h-10 w-10 rounded-2xl"
/>
</div>
<div className="pointer-events-auto rounded-[24px] border border-border/70 bg-background/80 p-2 shadow-xl backdrop-blur-xl">
<IconButton
icon={<Compass size={16} />}
label="Reset canvas view"
onClick={() => canvasRef.current?.resetView()}
className="h-10 w-10 rounded-2xl"
/>
</div>
</div>

<div className="h-full">
<Canvas
ref={canvasRef}
Expand All @@ -287,6 +255,18 @@ export const CanvasView = forwardRef<CanvasViewHandle, CanvasViewProps>(function
minZoom: 0.1,
maxZoom: 4
}}
showNavigationTools
navigationToolsPosition="bottom-right"
navigationToolsShowZoomLabel={false}
navigationToolsStyle={{
bottom: 24,
right: 24,
borderRadius: 24,
background: 'rgba(255, 255, 255, 0.8)',
backdropFilter: 'blur(16px)',
boxShadow: '0 18px 38px rgba(15, 23, 42, 0.12)',
border: '1px solid rgba(148, 163, 184, 0.28)'
}}
renderNode={(node) => {
const linkedDocument = node.linkedNodeId
? documentMap.get(node.linkedNodeId)
Expand Down
233 changes: 233 additions & 0 deletions apps/electron/src/renderer/components/DatabaseView.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
/**
* @vitest-environment jsdom
*/

import { fireEvent, render, screen } from '@testing-library/react'
import React from 'react'
import { describe, beforeEach, expect, it, vi } from 'vitest'
import * as Y from 'yjs'
import { DatabaseView } from './DatabaseView'

const mockUpdate = vi.fn()
const mockCreate = vi.fn()

const mockUseNode = vi.fn()
const mockUseIdentity = vi.fn()
const mockUseMutate = vi.fn()
const mockUseQuery = vi.fn()
const mockUseDatabaseComments = vi.fn()

vi.mock('@xnetjs/data', () => ({
DatabaseSchema: {
schema: {
'@id': 'xnet://schema/database'
}
},
decodeAnchor: vi.fn(() => ({})),
buildDatabaseSchema: vi.fn((docId: string) => ({
'@id': `xnet://schema/${docId}`
})),
createInitialSchemaMetadata: vi.fn((name: string) => ({
version: 1,
name,
createdAt: 0,
updatedAt: 0,
history: []
})),
bumpSchemaVersion: vi.fn((metadata: { version: number }) => ({
...metadata,
version: metadata.version + 1
})),
getVersionBumpType: vi.fn(() => 'minor'),
cloneSchema: vi.fn((columns: unknown[]) => columns),
createVersionEntry: vi.fn(() => ({
version: 1,
timestamp: 0,
changes: []
})),
pruneVersionHistory: vi.fn((history: unknown[]) => history)
}))

vi.mock('@xnetjs/react', () => ({
useNode: (...args: unknown[]) => mockUseNode(...args),
useIdentity: (...args: unknown[]) => mockUseIdentity(...args),
useMutate: (...args: unknown[]) => mockUseMutate(...args),
useQuery: (...args: unknown[]) => mockUseQuery(...args)
}))

vi.mock('@xnetjs/views', () => ({
TableView: () => <div data-testid="table-view">table view</div>,
BoardView: () => <div data-testid="board-view">board view</div>,
CardDetailModal: () => null,
AddColumnModal: () => null,
SchemaInfoModal: () => null,
CloneSchemaModal: () => null,
useDatabaseComments: (...args: unknown[]) => mockUseDatabaseComments(...args)
}))

vi.mock('@xnetjs/ui', async () => {
const ReactModule = await import('react')

function Menu({
trigger,
children
}: {
trigger: React.ReactNode
children: React.ReactNode
}): React.ReactElement {
const [open, setOpen] = ReactModule.useState(false)

if (!ReactModule.isValidElement(trigger)) {
return <div>{children}</div>
}

return (
<div>
{ReactModule.cloneElement(trigger, {
onClick: () => setOpen((prev) => !prev)
})}
{open ? <div data-testid="mock-menu">{children}</div> : null}
</div>
)
}

return {
CommentPopover: () => null,
CommentsSidebar: ({ open }: { open: boolean }) => (
<div data-testid="comments-sidebar">{open ? 'open' : 'closed'}</div>
),
Menu,
MenuItem: ({ children, onSelect }: { children: React.ReactNode; onSelect?: () => void }) => (
<button type="button" onClick={onSelect}>
{children}
</button>
),
MenuLabel: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
MenuSeparator: () => <hr />
}
})

vi.mock('./ShareButton', () => ({
ShareButton: ({ docId }: { docId: string }) => (
<button type="button" data-testid="share-button">
Share {docId}
</button>
)
}))

vi.mock('./PresenceAvatars', () => ({
PresenceAvatars: () => <div data-testid="presence-avatars">presence</div>
}))

function createAwarenessMock() {
return {
clientID: 1,
getStates: () => new Map<number, Record<string, unknown>>(),
setLocalStateField: vi.fn(),
on: vi.fn(),
off: vi.fn()
}
}

function createDatabaseDoc(): Y.Doc {
const doc = new Y.Doc()
const dataMap = doc.getMap('data')

dataMap.set('columns', [
{
id: 'title',
name: 'Title',
type: 'text'
},
{
id: 'status',
name: 'Status',
type: 'select',
options: ['Todo', 'Done']
}
])
dataMap.set('rows', [
{
id: 'row-1',
values: {
title: 'First row',
status: 'Todo'
}
}
])
dataMap.set('schema', {
version: 1,
name: 'Focus DB',
createdAt: 0,
updatedAt: 0,
history: []
})

return doc
}

describe('DatabaseView minimal chrome', () => {
beforeEach(() => {
mockUpdate.mockReset()
mockCreate.mockReset()

mockUseIdentity.mockReturnValue({ did: 'did:xnet:test' })
mockUseMutate.mockReturnValue({ create: mockCreate })
mockUseQuery.mockReturnValue({ data: [] })
mockUseDatabaseComments.mockReturnValue({
threads: [],
cellCommentCounts: new Map(),
unresolvedCount: 2,
commentOnCell: vi.fn(),
getThreadsForCell: vi.fn(() => []),
replyTo: vi.fn(),
resolveThread: vi.fn(),
reopenThread: vi.fn(),
deleteComment: vi.fn(),
editComment: vi.fn()
})
mockUseNode.mockReturnValue({
data: { title: 'Focus DB' },
doc: createDatabaseDoc(),
loading: false,
update: mockUpdate,
presence: [{ did: 'did:xnet:peer', color: '#22c55e' }],
awareness: createAwarenessMock()
})
})

it('renders the compact focus controls and moves sharing into the overflow menu', async () => {
render(<DatabaseView docId="db-1" minimalChrome />)

expect(await screen.findByDisplayValue('Focus DB')).toBeTruthy()
expect(screen.getByTestId('presence-avatars')).toBeTruthy()
expect(screen.getByRole('button', { name: /add row/i })).toBeTruthy()
expect(screen.getByRole('button', { name: /comments/i })).toBeTruthy()
expect(screen.getByRole('button', { name: /open database actions/i })).toBeTruthy()
expect(screen.queryByTestId('share-button')).toBeNull()

fireEvent.click(screen.getByRole('button', { name: /open database actions/i }))

expect(screen.getByText('Schema info')).toBeTruthy()
expect(screen.getByText('Clone schema')).toBeTruthy()
expect(screen.getByTestId('share-button')).toBeTruthy()
})

it('keeps comments accessible from the compact toolbar', async () => {
render(<DatabaseView docId="db-1" minimalChrome />)

expect((await screen.findByTestId('comments-sidebar')).textContent).toContain('closed')

fireEvent.click(screen.getByRole('button', { name: /comments/i }))

expect(screen.getByTestId('comments-sidebar').textContent).toContain('open')
})

it('preserves the richer header outside focused mode', async () => {
render(<DatabaseView docId="db-1" />)

expect(await screen.findByDisplayValue('Focus DB')).toBeTruthy()
expect(screen.getByTestId('share-button')).toBeTruthy()
expect(screen.queryByRole('button', { name: /open database actions/i })).toBeNull()
})
})
Loading
Loading