Skip to content

Fix: unified frames no longer land stacked on insert #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vibhavkatre merged 1 commit into from
Jul 20, 2026
Merged

Fix: unified frames no longer land stacked on insert #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 17 additions & 8 deletions frontend/src/diagram/schema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const NO_COLOR = null

export function createDiagramDocument(presetName = DEFAULT_PRESET_NAME, diagramType = DEFAULT_DIAGRAM_TYPE) {
const preset = findPreset(presetName)
return {
const document = {
schemaVersion: SCHEMA_VERSION,
diagramType,
canvas: {
Expand All @@ -54,6 +54,19 @@ export function createDiagramDocument(presetName = DEFAULT_PRESET_NAME, diagramT
flowchart: usesSubModel('flowchart', diagramType) ? createFlowchart() : null,
whiteboard: usesSubModel('whiteboard', diagramType) ? createWhiteboard() : null,
}
// A unified canvas holds every frame at once, so seed distinct frame origins —
// otherwise the mind map and flowchart both default to (0,0) and land stacked
// when inserted. They stay freely movable afterwards.
if (diagramType === UNIFIED_DIAGRAM_TYPE) applyUnifiedFrameOrigins(document)
return document
}

// The default spatial layout of a fresh unified canvas: mind map upper-centre,
// flowchart well below it, both clear of the block substrate around (0,0).
function applyUnifiedFrameOrigins(document) {
if (document.mindmap) document.mindmap.origin = { x: 600, y: 200 }
if (document.flowchart) document.flowchart.origin = { x: 600, y: 700 }
return document
}
Comment on lines +67 to 70

@greptile-apps greptile-apps Bot Jul 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The return document is unreferenced by the caller (mutation-in-place is what takes effect). Drop it to avoid implying the function is pure.

Suggested change
if (document.mindmap) document.mindmap.origin = { x: 600, y: 200 }
if (document.flowchart) document.flowchart.origin = { x: 600, y: 700 }
return document
}
if (document.mindmap) document.mindmap.origin = { x: 600, y: 200 }
if (document.flowchart) document.flowchart.origin = { x: 600, y: 700 }
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


// Whether a document of `diagramType` should carry the given sub-model: its own
Expand All @@ -63,14 +76,10 @@ function usesSubModel(subModel, diagramType) {
}

// Create a blank unified-canvas document (shared substrate + all sub-models).
// The auto-layout sub-models get distinct default frame origins so their content
// doesn't stack on top of the block substrate (which lives around 0,0) or on each
// other once they're populated.
// createDiagramDocument already seeds the distinct frame origins for a unified
// type; this stays as a named, intent-revealing entry point.
export function createUnifiedDocument(presetName = DEFAULT_PRESET_NAME) {
const document = createDiagramDocument(presetName, UNIFIED_DIAGRAM_TYPE)
document.mindmap.origin = { x: 600, y: 200 }
document.flowchart.origin = { x: 600, y: 700 }
return document
return createDiagramDocument(presetName, UNIFIED_DIAGRAM_TYPE)
}

// Parse a document that may arrive as a JSON string (from the API) or an object,
Expand Down
9 changes: 9 additions & 0 deletions frontend/src/diagram/unifiedDocument.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ describe('unified document', () => {
expect(doc.flowchart.origin).toEqual({ x: 600, y: 700 })
expect(doc.mindmap.origin).not.toEqual(doc.flowchart.origin)
})

it('seeds the distinct origins via the plain factory too (the create-flow path)', () => {
// The home "New diagram" flow builds the doc with
// createDiagramDocument(undefined, 'unified') — NOT createUnifiedDocument — so
// the offsets must live in the factory, else the frames land stacked at (0,0).
const doc = createDiagramDocument(undefined, UNIFIED_DIAGRAM_TYPE)
expect(doc.mindmap.origin).toEqual({ x: 600, y: 200 })
expect(doc.flowchart.origin).toEqual({ x: 600, y: 700 })
})
})

describe('legacy single-type documents are unchanged', () => {
Expand Down
Loading