diff --git a/frontend/src/lib/components/common/contextmenu/ContextMenu.svelte b/frontend/src/lib/components/common/contextmenu/ContextMenu.svelte index 3446cfcb66..7302c4948a 100644 --- a/frontend/src/lib/components/common/contextmenu/ContextMenu.svelte +++ b/frontend/src/lib/components/common/contextmenu/ContextMenu.svelte @@ -45,8 +45,6 @@ console.log('Context menu event detected:', event) // Let Melt UI handle this } - - $inspect('dbg context menu', $open)
= writable(false) let showAssets: Writable = writable(true) @@ -264,7 +264,11 @@ type NodeDep = { id: string; parentIds?: string[]; offset?: number } type NodePos = { position: { x: number; y: number } } let lastNodes: [NodeDep[], (NodeDep & NodePos)[]] | undefined = undefined - function layoutNodes(nodes: NodeDep[]): (NodeDep & NodePos)[] { + function layoutNodes( + nodes: NodeDep[], + groupNotes: FlowNote[] = [], + noteTextHeights: Record = {} + ): (NodeDep & NodePos)[] { let lastResult = lastNodes?.[1] if (lastResult && deepEqual(nodes, lastNodes?.[0])) { console.debug('layoutNodes', 'same nodes') @@ -283,6 +287,9 @@ const nodes2: (NodeDep & NodePos)[] = nodes.map((n) => { return { ...n, position: { x: 0, y: 0 } } }) + + // Build spacing map for group notes - this integrates with D3's layout algorithm + const nodeSpacingMap = buildNodeSpacingMap(nodes, groupNotes, noteTextHeights, noteManager) for (const n of topologicalSort(nodes)) { const endId = n.id + '-end' @@ -304,9 +311,12 @@ .decross(nodes.length > 20 ? decrossTwoLayer() : decrossOpt()) .coord(coordCenter()) .nodeSize((d) => { + const nodeId = d?.data?.['id'] ?? '' + const baseHeight = NODE.height + NODE.gap.vertical + const extraSpacing = nodeSpacingMap[nodeId] ?? 0 return [ - (nodeWidths[d?.data?.['id'] ?? ''] ?? 1) * (NODE.width + NODE.gap.horizontal * 1), - NODE.height + NODE.gap.vertical + (nodeWidths[nodeId] ?? 1) * (NODE.width + NODE.gap.horizontal * 1), + baseHeight + extraSpacing ] as readonly [number, number] }) boxSize = layout(dag as any) @@ -314,7 +324,12 @@ const layout = sugiyama() .decross(decrossTwoLayer()) .coord(coordCenter()) - .nodeSize(() => [NODE.width + NODE.gap.horizontal, NODE.height + NODE.gap.vertical]) + .nodeSize((d) => { + const nodeId = d?.data?.['id'] ?? '' + const baseHeight = NODE.height + NODE.gap.vertical + const extraSpacing = nodeSpacingMap[nodeId] ?? 0 + return [NODE.width + NODE.gap.horizontal, baseHeight + extraSpacing] + }) boxSize = layout(dag as any) } @@ -332,7 +347,7 @@ NODE.width / 2 - (width - fullWidth) / 2 : 0, - y: (des.y || 0) + yOffset + y: (des.y || 0) + yOffset + (nodeSpacingMap[des.data.id] ?? 0) / 2 } })) } @@ -465,20 +480,11 @@ id: n.id, parentIds: n.parentIds, offset: n.data.offset ?? 0 - })) - ) - let newNodes: (Node & NodeLayout)[] = layoutedNodes.map((n) => ({ ...n, ...graph.nodes[n.id] })) - - // Apply group note spacing adjustments - const groupNoteSpacingResult = computeGroupNoteSpacing( - newNodes.map((n) => ({ id: n.id, position: n.position })), - notes ?? [], + })), + notes?.filter((note) => note.type === 'group') ?? [], noteTextHeights ) - newNodes = newNodes.map((n) => ({ - ...n, - position: groupNoteSpacingResult.newNodePositions[n.id] || n.position - })) + let newNodes: (Node & NodeLayout)[] = layoutedNodes.map((n) => ({ ...n, ...graph.nodes[n.id] })) let assetNodesResult = $showAssets ? computeAssetNodes( @@ -691,8 +697,6 @@ export function zoomOut() { viewportSynchronizer?.zoomOut() } - - $inspect('dbg notes & nodes', notes, nodes) {#if insertable} diff --git a/frontend/src/lib/components/graph/groupNoteSpacing.ts b/frontend/src/lib/components/graph/groupNoteSpacing.ts deleted file mode 100644 index 7964139943..0000000000 --- a/frontend/src/lib/components/graph/groupNoteSpacing.ts +++ /dev/null @@ -1,184 +0,0 @@ -import type { FlowNote } from '../../gen' -import { deepEqual } from 'fast-equals' -import { clone } from '../../utils' - -export type NodePosition = { - id: string - position: { x: number; y: number } -} - -export type GroupNoteSpacingResult = { - // Nodes need to be offset on the y axis to make space for group notes - newNodePositions: Record -} - -export type TextHeightCache = { - content: string - height: number -} - -// Cache for expensive group note spacing calculations -let computeGroupNoteSpacingCache: - | [NodePosition[], FlowNote[], Record, GroupNoteSpacingResult] - | undefined - -// Text height cache to avoid remeasuring unchanged note text -let textHeightCache: Record = {} - -const GROUP_NOTE_PADDING = 20 - -/** - * Caches text height measurements based on content hash - */ -export function cacheTextHeight(noteId: string, content: string, height: number): void { - textHeightCache[noteId] = { content, height } -} - -/** - * Gets cached text height if content hasn't changed, otherwise returns undefined - */ -export function getCachedTextHeight(noteId: string, content: string): number | undefined { - const cached = textHeightCache[noteId] - if (cached && cached.content === content) { - return cached.height - } - return undefined -} - -/** - * Gets the text height for a note, using cache if available or fallback - */ -function getTextHeight(note: FlowNote, noteTextHeights: Record): number { - // Try cached height first - const cachedHeight = getCachedTextHeight(note.id, note.text) - if (cachedHeight !== undefined) { - return cachedHeight - } - - // Fall back to runtime heights - const runtimeHeight = noteTextHeights[note.id] - if (runtimeHeight !== undefined) { - // Update cache for future use - cacheTextHeight(note.id, note.text, runtimeHeight) - return runtimeHeight - } - - // Default fallback - return 60 -} - -/** - * Finds the topmost node in a group by Y position - */ -function findTopmostNodeInGroup( - groupNote: FlowNote, - nodes: NodePosition[] -): NodePosition | undefined { - if (!groupNote.contained_node_ids?.length) { - return undefined - } - - const containedNodes = nodes.filter((node) => groupNote.contained_node_ids?.includes(node.id)) - - if (containedNodes.length === 0) { - return undefined - } - - return containedNodes.reduce((topMost, node) => - node.position.y < topMost.position.y ? node : topMost - ) -} - -/** - * Computes vertical spacing adjustments for nodes to accommodate group notes. - */ -export function computeGroupNoteSpacing( - nodes: NodePosition[], - notes: FlowNote[], - noteTextHeights: Record -): GroupNoteSpacingResult { - // Check cache first - if ( - computeGroupNoteSpacingCache && - deepEqual(nodes, computeGroupNoteSpacingCache[0]) && - deepEqual(notes, computeGroupNoteSpacingCache[1]) && - deepEqual(noteTextHeights, computeGroupNoteSpacingCache[2]) - ) { - return computeGroupNoteSpacingCache[3] - } - - // Filter to only group notes - const groupNotes = notes.filter((note) => note.type === 'group') - - if (groupNotes.length === 0) { - const result: GroupNoteSpacingResult = { - newNodePositions: Object.fromEntries(nodes.map((n) => [n.id, n.position])) - } - computeGroupNoteSpacingCache = [clone(nodes), clone(notes), clone(noteTextHeights), result] - return result - } - - // Map Y positions to required spacing - const ySpacingMap = new Map() - - // For each group note, determine the spacing needed at the topmost node's Y position - for (const groupNote of groupNotes) { - const topmostNode = findTopmostNodeInGroup(groupNote, nodes) - - if (topmostNode) { - const textHeight = getTextHeight(groupNote, noteTextHeights) - const requiredSpacing = textHeight + GROUP_NOTE_PADDING - - // If multiple group notes affect the same Y position, take the maximum spacing needed - const currentSpacing = ySpacingMap.get(topmostNode.position.y) || 0 - ySpacingMap.set(topmostNode.position.y, Math.max(currentSpacing, requiredSpacing)) - } - } - - // Sort nodes by Y position to apply cumulative spacing - const sortedNodes = [...nodes].sort((a, b) => a.position.y - b.position.y) - - // Apply cumulative spacing - let cumulativeOffset = 0 - let lastYPosition = -Infinity - const adjustedNodes = sortedNodes.map((node) => { - // When we encounter a new Y position, check if it needs additional spacing - if (node.position.y > lastYPosition) { - const spacingAtThisY = ySpacingMap.get(node.position.y) - if (spacingAtThisY !== undefined) { - cumulativeOffset += spacingAtThisY - } - lastYPosition = node.position.y - } - - return { - ...node, - position: { - ...node.position, - y: node.position.y + cumulativeOffset - } - } - }) - - const result: GroupNoteSpacingResult = { - newNodePositions: Object.fromEntries(adjustedNodes.map((n) => [n.id, n.position])) - } - - // Cache the result - computeGroupNoteSpacingCache = [clone(nodes), clone(notes), clone(noteTextHeights), result] - return result -} - -/** - * Clears the text height cache (useful for testing or when needed) - */ -export function clearTextHeightCache(): void { - textHeightCache = {} -} - -/** - * Clears the group note spacing cache (useful when nodes structure changes dramatically) - */ -export function clearGroupNoteSpacingCache(): void { - computeGroupNoteSpacingCache = undefined -} diff --git a/frontend/src/lib/components/graph/groupNoteUtils.ts b/frontend/src/lib/components/graph/groupNoteUtils.ts index d037d623e8..99e487d204 100644 --- a/frontend/src/lib/components/graph/groupNoteUtils.ts +++ b/frontend/src/lib/components/graph/groupNoteUtils.ts @@ -1,6 +1,11 @@ import type { Node } from '@xyflow/svelte' -import type { FlowNote } from '$lib/gen' import { NODE } from './util' +import type { FlowNote } from '../../gen' +import type { NoteManager } from './noteManager.svelte' + +type NodeDep = { id: string; parentIds?: string[]; offset?: number } + +const GROUP_NOTE_PADDING = 20 export interface GroupNoteBounds { x: number @@ -18,7 +23,7 @@ export function computeGroupNoteBounds( nodes: Node[], textHeight: number = 60 ): GroupNoteBounds { - const containedNodes = nodes.filter(node => containedNodeIds.includes(node.id)) + const containedNodes = nodes.filter((node) => containedNodeIds.includes(node.id)) if (containedNodes.length === 0) { throw new Error('No nodes contained in group note') @@ -30,7 +35,7 @@ export function computeGroupNoteBounds( let minY = Infinity let maxY = -Infinity - containedNodes.forEach(node => { + containedNodes.forEach((node) => { minX = Math.min(minX, node.position.x) maxX = Math.max(maxX, node.position.x + NODE.width) minY = Math.min(minY, node.position.y) @@ -43,39 +48,106 @@ export function computeGroupNoteBounds( return { x: minX - padding, y: minY - padding - textHeight, // Position text above the nodes - width: maxX - minX + (padding * 2), - height: maxY - minY + (padding * 2) + textHeight + width: maxX - minX + padding * 2, + height: maxY - minY + padding * 2 + textHeight } } /** - * Gets the topmost node from a list of contained nodes + * Finds the topmost node in a group based on topological ordering + * Uses parent-child relationships to determine hierarchy */ -export function getTopMostNode(containedNodeIds: string[], nodes: Node[]): Node | null { - const containedNodes = nodes.filter(node => containedNodeIds.includes(node.id)) +function findTopmostNodeInGroup(groupNote: FlowNote, nodes: NodeDep[]): NodeDep | undefined { + if (!groupNote.contained_node_ids?.length) { + return undefined + } + + const containedNodes = nodes.filter((node) => groupNote.contained_node_ids?.includes(node.id)) if (containedNodes.length === 0) { - return null + return undefined } - return containedNodes.reduce((topMost, node) => - node.position.y < topMost.position.y ? node : topMost - ) + // Find the node with the fewest parents (or no parents) - this will be topmost in the flow + const nodesByParentCount = containedNodes.map((node) => ({ + node, + parentCount: node.parentIds?.length || 0 + })) + + // Sort by parent count, then by appearance in nodes array to ensure deterministic results + nodesByParentCount.sort((a, b) => { + if (a.parentCount !== b.parentCount) { + return a.parentCount - b.parentCount + } + // If same parent count, use original node order as tiebreaker + const aIndex = nodes.findIndex((n) => n.id === a.node.id) + const bIndex = nodes.findIndex((n) => n.id === b.node.id) + return aIndex - bIndex + }) + + return nodesByParentCount[0]?.node } /** - * Validates that all contained nodes exist in the provided nodes array + * Gets the extra spacing needed for a specific node due to group notes + * Returns 0 if the node doesn't need extra spacing */ -export function validateGroupNote(note: FlowNote, nodes: Node[]): FlowNote { - if (note.type !== 'group' || !note.contained_node_ids) { - return note +export function getNodeGroupNoteSpacing( + nodeId: string, + groupNotes: FlowNote[], + nodes: NodeDep[], + noteTextHeights: Record, + noteManager: NoteManager +): number { + for (const groupNote of groupNotes) { + if (groupNote.contained_node_ids?.includes(nodeId)) { + const topmostNode = findTopmostNodeInGroup(groupNote, nodes) + + // Only the topmost node gets the spacing + if (topmostNode?.id === nodeId) { + const textHeight = noteManager.getTextHeight( + groupNote.id, + groupNote.text, + noteTextHeights, + 60 + ) + return textHeight + GROUP_NOTE_PADDING + } + } + } + return 0 +} + +/** + * Builds a map of node IDs to their required extra spacing for group notes + * This is called during the layout process to integrate with D3's nodeSize function + */ +export function buildNodeSpacingMap( + nodes: NodeDep[], + groupNotes: FlowNote[], + noteTextHeights: Record, + noteManager: NoteManager +): Record { + const spacingMap: Record = {} + + // Only process if we have group notes + if (groupNotes.length === 0) { + return spacingMap } - const validNodeIds = nodes.map(node => node.id) - const validContainedNodeIds = note.contained_node_ids.filter(id => validNodeIds.includes(id)) - - return { - ...note, - contained_node_ids: validContainedNodeIds + // For each node, calculate if it needs extra spacing + for (const node of nodes) { + const extraSpacing = getNodeGroupNoteSpacing( + node.id, + groupNotes, + nodes, + noteTextHeights, + noteManager + ) + if (extraSpacing > 0) { + spacingMap[node.id] = extraSpacing + } } -} \ No newline at end of file + + return spacingMap +} diff --git a/frontend/src/lib/components/graph/noteManager.svelte.ts b/frontend/src/lib/components/graph/noteManager.svelte.ts index ed19e3ca03..c08db0a065 100644 --- a/frontend/src/lib/components/graph/noteManager.svelte.ts +++ b/frontend/src/lib/components/graph/noteManager.svelte.ts @@ -2,20 +2,30 @@ import type { FlowNote } from '$lib/gen' import type { Node } from '@xyflow/svelte' import type { NoteColor } from './noteColors' import { calculateNodesBounds } from './util' -import { cacheTextHeight } from './groupNoteSpacing' export type NodePosition = { id: string position: { x: number; y: number } } +export type TextHeightCacheEntry = { + content: string + height: number +} + /** * Utility class for managing flow notes including regular and group notes * This is now a stateless utility that operates on passed note data */ export class NoteManager { + #cache: Record = $state({}) + constructor() {} + getCache(): Record { + return this.#cache + } + /** * Add a new note from the note tool */ @@ -126,7 +136,8 @@ export class NoteManager { // Find bounds of all contained nodes const bounds = calculateNodesBounds(containedNodes) - const padding = 20 + const padding = 16 + return { position: { x: bounds.minX - padding, @@ -178,7 +189,7 @@ export class NoteManager { onTextHeightChange: (textHeight: number) => { onTextHeightChange(note.id, textHeight) // Cache the text height for improved performance - cacheTextHeight(note.id, note.text, textHeight) + this.cacheTextHeight(note.id, note.text, textHeight) } } } @@ -215,4 +226,63 @@ export class NoteManager { } }) } + + /** + * Caches text height measurements based on content hash + */ + cacheTextHeight(noteId: string, content: string, height: number): void { + this.#cache[noteId] = { content, height } + } + + /** + * Gets cached text height if content hasn't changed, otherwise returns undefined + */ + getCachedTextHeight(noteId: string, content: string): number | undefined { + const cached = this.#cache[noteId] + if (cached && cached.content === content) { + return cached.height + } + return undefined + } + + /** + * Gets the text height for a note, using cache if available or fallback + */ + getTextHeight( + noteId: string, + content: string, + runtimeHeights: Record, + defaultHeight: number = 60 + ): number { + // Try cached height first + const cachedHeight = this.getCachedTextHeight(noteId, content) + if (cachedHeight !== undefined) { + return cachedHeight + } + + // Fall back to runtime heights + const runtimeHeight = runtimeHeights[noteId] + if (runtimeHeight !== undefined) { + // Update cache for future use + this.cacheTextHeight(noteId, content, runtimeHeight) + return runtimeHeight + } + + // Default fallback + return defaultHeight + } + + /** + * Clears the entire text height cache (useful for testing or when needed) + */ + clearTextHeightCache(): void { + this.#cache = {} + } + + /** + * Removes a specific note from the cache + */ + removeTextHeight(noteId: string): void { + delete this.#cache[noteId] + } } diff --git a/frontend/src/lib/components/graph/renderers/nodes/NoteNode.svelte b/frontend/src/lib/components/graph/renderers/nodes/NoteNode.svelte index 253d8f805b..6d37da9636 100644 --- a/frontend/src/lib/components/graph/renderers/nodes/NoteNode.svelte +++ b/frontend/src/lib/components/graph/renderers/nodes/NoteNode.svelte @@ -92,8 +92,11 @@ }) // Track content height and notify parent + let previousContainerHeight = $state(0) $effect(() => { - if (containerHeight > 0) { + if (containerHeight > 0 && containerHeight !== previousContainerHeight) { + console.log('dbg containerHeight', containerHeight) + previousContainerHeight = containerHeight data.onTextHeightChange?.(containerHeight) } }) @@ -188,8 +191,10 @@
{#if editMode} @@ -197,22 +202,23 @@ bind:this={textareaElement} bind:value={textContent} class={twMerge( - 'windmillapp w-full h-auto min-h-[60px] max-h-[400px] shadow-none resize-none text-xs overflow-y-auto border-none rounded-md bg-transparent transition-colors p-4', + 'windmillapp w-full shadow-none resize-none text-xs overflow-y-auto border-none rounded-md bg-transparent transition-colors p-4', colorConfig.text )} placeholder="Add your note here... (Markdown supported)" onblur={handleTextSave} spellcheck="false" - style="field-sizing: content;" + style:height={data.isGroupNote ? `${containerHeight > 0 ? containerHeight : 60}px` : '100%'} > {:else}
{#if data.text}