Skip to content

Canvas unification — Phase 4a: frame origin data model - #7

Merged
vibhavkatre merged 1 commit into
mainfrom
feat/unified-canvas-frames-model
Jul 17, 2026
Merged

Canvas unification — Phase 4a: frame origin data model#7
vibhavkatre merged 1 commit into
mainfrom
feat/unified-canvas-frames-model

Conversation

@vibhavkatre

Copy link
Copy Markdown
Collaborator

What & why

First step of Phase 4 (mind maps & flowcharts as frames on the unified canvas). Data model only — each auto-layout sub-model gains a frame origin {x,y} (its top-left on the shared canvas). No rendering change yet; legacy single-type docs are unaffected since {0,0} = no offset (exactly how they render today).

Changes

  • mindmapModel / flowchartModel: createEmptyMindMap / createFlowchart include origin: {x:0,y:0}.
  • schema.js: createUnifiedDocument places frames at distinct default origins (mind map {600,200}, flowchart {600,700}) so populated frames don't stack on the block substrate or each other. Migration backfills origin {0,0} on older unified docs.

Testing

Build ✅ · 101 unit tests ✅ (adds frame-origin defaults + migration backfill cases). No model-shape regressions.

Next

Render the mind map / flowchart layers as positioned frames on the unified canvas (translate by origin), then wire in-frame interaction.

🤖 Generated with Claude Code

Foundation for mind maps & flowcharts as frames on the unified canvas: each
auto-layout sub-model gains a frame `origin` {x,y} — its top-left on the shared
canvas. Data model only (no rendering yet); legacy single-type docs are
unaffected because {0,0} = no offset, exactly how they render today.

- mindmapModel/flowchartModel: createEmptyMindMap / createFlowchart include
  origin {x:0,y:0}.
- schema: createUnifiedDocument places the frames at distinct default origins
  (mind map {600,200}, flowchart {600,700}) so their content doesn't stack on the
  block substrate or each other. Migration backfills origin {0,0} on older
  unified docs.

Verified: build + 101 unit tests (adds frame-origin + migration cases). Next:
render the mind map / flowchart layers as positioned frames on the unified canvas.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 17, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

Safe to merge for current rendering; the createMindMap() gap could bite Phase 4b when the renderer starts reading .origin.

createMindMap() ships without origin while createEmptyMindMap() has it. The schema only calls createEmptyMindMap() today, so nothing breaks now, but as soon as Phase 4b code reads .origin on any object that came from createMindMap(), it will silently get undefined rather than {0,0}.

frontend/src/diagram/mindmapModel.js — the createMindMap factory.

Reviews (1): Last reviewed commit: "Frappe Draw: canvas unification phase 4a..." | Re-trigger Greptile

Comment on lines 69 to 74
export function createUnifiedDocument(presetName = DEFAULT_PRESET_NAME) {
return createDiagramDocument(presetName, UNIFIED_DIAGRAM_TYPE)
const document = createDiagramDocument(presetName, UNIFIED_DIAGRAM_TYPE)
document.mindmap.origin = { x: 600, y: 200 }
document.flowchart.origin = { x: 600, y: 700 }
return document
}

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 createEmptyMindMap() and createFlowchart() already set origin: {x:0, y:0}, so createUnifiedDocument immediately overwrites those values. A clarifying comment (or passing origins as constructor params) would make the intent explicit rather than looking like a first-time set.

Suggested change
export function createUnifiedDocument(presetName = DEFAULT_PRESET_NAME) {
return createDiagramDocument(presetName, UNIFIED_DIAGRAM_TYPE)
const document = createDiagramDocument(presetName, UNIFIED_DIAGRAM_TYPE)
document.mindmap.origin = { x: 600, y: 200 }
document.flowchart.origin = { x: 600, y: 700 }
return document
}
export function createUnifiedDocument(presetName = DEFAULT_PRESET_NAME) {
const document = createDiagramDocument(presetName, UNIFIED_DIAGRAM_TYPE)
// Override the {0,0} defaults from the sub-model factories with distinct
// non-overlapping origins so frames don't stack on the block substrate.
document.mindmap.origin = { x: 600, y: 200 }
document.flowchart.origin = { x: 600, y: 700 }
return document
}

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!

@vibhavkatre
vibhavkatre merged commit 94fa820 into main Jul 17, 2026
3 checks passed
@vibhavkatre
vibhavkatre deleted the feat/unified-canvas-frames-model branch July 17, 2026 03:12
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