mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-10 08:07:03 +00:00
fix(frontend): restore heading sizes in note markdown and keep group notes on id change (#11047)
* fix(frontend): restore heading sizes in note markdown and keep group notes on id change Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * fix(frontend): scope the note cleanup deferral per note Address local review nits: the header comment overstated the heading ramp (h3 sits at body size in the xs and sm scales), and the mid-update deferral bailed out of cleanup for every group note rather than the one holding the unrendered module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * perf(frontend): traverse modules without collecting a discarded array Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * test(frontend): show the three markdown prose presets in the kitchen sink The Markdown tab rendered only the default preset, so a change to the shared heading scale could not be compared across the surfaces that use it. The chat sample gains headings for the same reason: it is the only place the assistant bubble renders at panel width without an AI provider. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * refactor(frontend): validate group notes against the flow, not the render A group note's members are module ids, so the flow's own module list decides whether one still exists. Validating against the rendered nodes instead needed a special case for collapsed groups, and still dropped a live module in the pass after its id changed. Both cases are the same mistake, and checking the source of truth removes them together with the collapsedModuleIds parameter. Path completion keeps working off the rendered graph, since it needs the edges, and now skips a note whose members it cannot all see rather than dropping them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * fix(frontend): name a new note "Note", not after its internal type The prefill lands inside the user's own note, and "Free note" / "Group note" are the serialized type, not words anyone says about a note they just drew. The menus that create them already say "Add note". The heading goes to h2 as well: h3 computes to the body size in the note scale, so the prefill's own title did not read as one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * docs(frontend): describe what collapsedModuleIds is still for Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Guilhem
Claude Opus 5
@@ -806,6 +806,19 @@
|
||||
}
|
||||
}
|
||||
}
|
||||
// Group notes reference their members by module id. A stale id here is not
|
||||
// merely cosmetic: cleanupGroupNotes drops ids it cannot resolve and deletes
|
||||
// the note once none are left.
|
||||
const notes = flowStore.val.value.notes
|
||||
if (notes) {
|
||||
for (const note of notes) {
|
||||
if (note.contained_node_ids) {
|
||||
note.contained_node_ids = note.contained_node_ids.map((nid) =>
|
||||
nid === id ? newId : nid
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
flowStateStore.val[newId] = flowStateStore.val[id]
|
||||
delete flowStateStore.val[id]
|
||||
refreshStateStore(flowStore)
|
||||
|
||||
@@ -855,7 +855,8 @@
|
||||
...aiToolNodesResult.toolNodes
|
||||
]
|
||||
|
||||
// Collect module IDs hidden inside collapsed groups so note cleanup preserves them
|
||||
// Module IDs hidden inside collapsed groups: a note whose members are all in here has
|
||||
// nothing on screen to wrap, so it is skipped.
|
||||
const collapsedModuleIds = new Set<string>()
|
||||
for (const n of finalNodes) {
|
||||
if (n.type === 'collapsedGroup') {
|
||||
|
||||
@@ -86,7 +86,7 @@
|
||||
// Create the actual note using NoteEditor context
|
||||
if (noteEditorContext?.noteEditor) {
|
||||
noteEditorContext.noteEditor.addNote({
|
||||
text: '### Free note\nDouble click to edit me',
|
||||
text: '## Note\nDouble click to edit me',
|
||||
position,
|
||||
size,
|
||||
color: DEFAULT_NOTE_COLOR,
|
||||
@@ -107,7 +107,7 @@
|
||||
if (!noteEditorContext?.noteEditor || !contextMenuPosition) return
|
||||
|
||||
noteEditorContext.noteEditor.addNote({
|
||||
text: '### Free note\nDouble click to edit me',
|
||||
text: '## Note\nDouble click to edit me',
|
||||
position: contextMenuPosition,
|
||||
size: { width: 300, height: 200 },
|
||||
color: DEFAULT_NOTE_COLOR,
|
||||
|
||||
@@ -68,7 +68,7 @@
|
||||
function handleAddStickyNote() {
|
||||
if (noteEditorContext?.noteEditor && pendingFlowPosition) {
|
||||
noteEditorContext.noteEditor.addNote({
|
||||
text: '### Free note\nDouble click to edit me',
|
||||
text: '## Note\nDouble click to edit me',
|
||||
position: {
|
||||
x: pendingFlowPosition.x,
|
||||
y: pendingFlowPosition.y - (graphContext?.yOffset || 0)
|
||||
|
||||
@@ -6,6 +6,7 @@ import { DEFAULT_GROUP_NOTE_COLOR, getNextAvailableColor } from './noteColors'
|
||||
import { generateId } from './util'
|
||||
import { getContext, setContext } from 'svelte'
|
||||
import { completeAndSplitGroup } from './groupDetectionUtils'
|
||||
import { forEachFlowModule } from '../flows/dfs'
|
||||
|
||||
/**
|
||||
* Utility class for editing flow notes via direct flowStore mutations
|
||||
@@ -158,10 +159,7 @@ export class NoteEditor {
|
||||
/**
|
||||
* Create a group note containing the specified node IDs
|
||||
*/
|
||||
createGroupNote(
|
||||
nodeIds: string[],
|
||||
text: string = '### Group note\nDouble click to edit me'
|
||||
): string {
|
||||
createGroupNote(nodeIds: string[], text: string = '## Note\nDouble click to edit me'): string {
|
||||
// Filter ids in case they contain subflow nodes
|
||||
let filteredNodeIds: string[] = nodeIds
|
||||
let subflowIds: string[] = []
|
||||
@@ -219,10 +217,7 @@ export class NoteEditor {
|
||||
/**
|
||||
* Clean up group notes using DAG path completion
|
||||
*/
|
||||
cleanupGroupNotes(
|
||||
flowNodes: { id: string; parentIds?: string[] }[],
|
||||
collapsedModuleIds?: Set<string>
|
||||
): void {
|
||||
cleanupGroupNotes(flowNodes: { id: string; parentIds?: string[] }[]): void {
|
||||
if (!this.isAvailable()) {
|
||||
return
|
||||
}
|
||||
@@ -232,19 +227,23 @@ export class NoteEditor {
|
||||
if (groupNotes.length === 0) return
|
||||
|
||||
let hasChanges = false
|
||||
const nodeSet = new Set(flowNodes.map((n) => n.id))
|
||||
const renderedIds = new Set(flowNodes.map((n) => n.id))
|
||||
|
||||
// Include collapsed module IDs as valid — they are hidden but still exist
|
||||
if (collapsedModuleIds) {
|
||||
for (const id of collapsedModuleIds) {
|
||||
nodeSet.add(id)
|
||||
}
|
||||
}
|
||||
// A note's members are module ids, and the flow's own modules are what say whether one
|
||||
// still exists. The rendered nodes are a view of them: a live module is absent from it
|
||||
// while its group is collapsed, and again in the pass after its id changed, so pruning
|
||||
// against the render alone deletes steps out of notes that are perfectly valid.
|
||||
const moduleIds = new Set<string>()
|
||||
forEachFlowModule(this.flowStore.val.value?.modules ?? [], (mod) => {
|
||||
moduleIds.add(mod.id)
|
||||
})
|
||||
|
||||
// Step 1: Clean invalid nodes from existing group notes
|
||||
for (const note of groupNotes) {
|
||||
const originalIds = note.contained_node_ids || []
|
||||
const validIds = originalIds.filter((id) => nodeSet.has(id))
|
||||
// Path completion below can add ids that exist only in the graph (group
|
||||
// boundaries), so a rendered node counts as valid alongside a live module.
|
||||
const validIds = originalIds.filter((id) => moduleIds.has(id) || renderedIds.has(id))
|
||||
|
||||
if (validIds.length !== originalIds.length) {
|
||||
note.contained_node_ids = validIds
|
||||
@@ -259,9 +258,9 @@ export class NoteEditor {
|
||||
const originalNodes = note.contained_node_ids || []
|
||||
if (originalNodes.length === 0) continue
|
||||
|
||||
// Skip path completion for notes that reference collapsed modules,
|
||||
// since the DAG is incomplete when groups are collapsed
|
||||
if (collapsedModuleIds && originalNodes.some((id) => collapsedModuleIds.has(id))) {
|
||||
// Path completion walks the rendered edges, so it can only place members it can
|
||||
// see; one it cannot would come back as unreachable and be dropped from the note.
|
||||
if (originalNodes.some((id) => !renderedIds.has(id))) {
|
||||
continue
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
import { describe, it, expect, vi } from 'vitest'
|
||||
|
||||
// Mock modules that transitively import CSS/Monaco
|
||||
vi.mock('monaco-editor', () => ({}))
|
||||
vi.mock('@xyflow/svelte', () => ({}))
|
||||
|
||||
import type { FlowModule, OpenFlow } from '$lib/gen'
|
||||
import type { StateStore } from '$lib/utils'
|
||||
import type { ExtendedOpenFlow } from '../flows/types'
|
||||
import { NoteEditor } from './noteEditor.svelte'
|
||||
|
||||
function makeFlowStore(
|
||||
moduleIds: string[],
|
||||
containedNodeIds: string[]
|
||||
): StateStore<ExtendedOpenFlow> {
|
||||
const modules: FlowModule[] = moduleIds.map((id) => ({
|
||||
id,
|
||||
value: { type: 'rawscript', content: '', language: 'bun' } as any
|
||||
}))
|
||||
const flow: OpenFlow = {
|
||||
summary: '',
|
||||
value: {
|
||||
modules,
|
||||
notes: [
|
||||
{
|
||||
id: 'note',
|
||||
text: 'note',
|
||||
color: 'yellow',
|
||||
type: 'group',
|
||||
contained_node_ids: containedNodeIds
|
||||
}
|
||||
]
|
||||
},
|
||||
schema: {}
|
||||
}
|
||||
return { val: flow as ExtendedOpenFlow } as StateStore<ExtendedOpenFlow>
|
||||
}
|
||||
|
||||
function containedIds(flowStore: StateStore<ExtendedOpenFlow>): string[] | undefined {
|
||||
return flowStore.val.value.notes?.[0]?.contained_node_ids
|
||||
}
|
||||
|
||||
describe('cleanupGroupNotes', () => {
|
||||
it('keeps a module the graph has not rendered', () => {
|
||||
// Both the pass after a module id changed and a collapsed group leave a live module
|
||||
// out of the rendered nodes.
|
||||
const flowStore = makeFlowStore(['renamed', 'b'], ['renamed', 'b'])
|
||||
new NoteEditor(flowStore).cleanupGroupNotes([{ id: 'b' }])
|
||||
|
||||
expect(containedIds(flowStore)).toEqual(['renamed', 'b'])
|
||||
})
|
||||
|
||||
it('drops a module that no longer exists in the flow', () => {
|
||||
const flowStore = makeFlowStore(['b'], ['deleted', 'b'])
|
||||
new NoteEditor(flowStore).cleanupGroupNotes([{ id: 'b' }])
|
||||
|
||||
expect(containedIds(flowStore)).toEqual(['b'])
|
||||
})
|
||||
})
|
||||
@@ -264,7 +264,7 @@ export function computeNoteNodes(
|
||||
|
||||
if (editMode) {
|
||||
if (noteEditorContext?.noteEditor?.isAvailable()) {
|
||||
noteEditorContext.noteEditor.cleanupGroupNotes(nodes, collapsedModuleIds)
|
||||
noteEditorContext.noteEditor.cleanupGroupNotes(nodes)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4,10 +4,14 @@
|
||||
* call site with layout-only classes (padding, width, bg); anything typographic
|
||||
* belongs here.
|
||||
*
|
||||
* - 'xs': micro scale for dense secondary panes (chat reasoning blocks)
|
||||
* - 'xs': micro scale for dense secondary panes (chat reasoning blocks, group notes)
|
||||
* - 'sm': compact chat-bubble scale (assistant messages, flow/app chat, settings)
|
||||
* - 'doc': same rhythm and body size as 'sm', with a taller heading ramp
|
||||
* (lg/base/sm) and semibold headings for document-like surfaces (artifacts)
|
||||
*
|
||||
* h1 and h2 step above the body size in every preset, so `#` and `##` read as
|
||||
* headings rather than bold body text. Below that the tight 'xs' and 'sm' scales
|
||||
* run out of room, and h3 down differentiates by weight and colour alone.
|
||||
*/
|
||||
|
||||
// Kept as literal template parts: Tailwind's scanner reads class names verbatim
|
||||
@@ -28,8 +32,8 @@ const bodyXs =
|
||||
'text-primary prose-p:text-primary prose-li:text-primary prose-p:text-xs prose-li:text-xs prose-code:text-xs prose-pre:text-xs prose-table:text-xs'
|
||||
|
||||
export const markdownProse = {
|
||||
xs: `${base} prose-sm leading-snug prose-p:text-2xs prose-li:text-2xs prose-code:text-2xs prose-pre:text-2xs prose-headings:font-medium prose-headings:text-secondary prose-headings:mt-2 prose-headings:mb-1 prose-h1:text-2xs prose-h2:text-2xs prose-h3:text-2xs prose-h4:text-2xs prose-h5:text-2xs prose-h6:text-2xs prose-strong:text-secondary`,
|
||||
sm: `${base} ${rhythm} ${bodyXs} prose-headings:mt-3 prose-headings:mb-1 prose-headings:font-medium prose-headings:text-emphasis prose-h1:text-sm prose-h2:text-xs prose-h3:text-xs prose-h4:text-xs prose-h5:text-xs prose-h6:text-xs`,
|
||||
xs: `${base} prose-sm leading-snug prose-p:text-2xs prose-li:text-2xs prose-code:text-2xs prose-pre:text-2xs prose-headings:font-medium prose-headings:text-secondary prose-headings:mt-2 prose-headings:mb-1 prose-h1:text-sm prose-h2:text-xs prose-h3:text-2xs prose-h4:text-2xs prose-h5:text-2xs prose-h6:text-2xs prose-strong:text-secondary`,
|
||||
sm: `${base} ${rhythm} ${bodyXs} prose-headings:mt-3 prose-headings:mb-1 prose-headings:font-medium prose-headings:text-emphasis prose-h1:text-base prose-h2:text-sm prose-h3:text-xs prose-h4:text-xs prose-h5:text-xs prose-h6:text-xs`,
|
||||
doc: `${base} ${rhythm} ${bodyXs} prose-headings:mt-8 prose-headings:mb-2 prose-headings:font-semibold prose-headings:text-emphasis prose-h1:text-lg prose-h2:text-base prose-h3:text-sm prose-h4:text-xs prose-h5:text-xs prose-h6:text-xs prose-pre:bg-transparent prose-pre:p-0`
|
||||
} as const
|
||||
|
||||
|
||||
@@ -24,6 +24,8 @@
|
||||
|
||||
## Heading 2
|
||||
|
||||
### Heading 3
|
||||
|
||||
Body text with **bold**, *italic*, a [link](https://windmill.dev), and \`inline code\` that must stay readable in both themes.
|
||||
|
||||
> A block quote should be legible too.
|
||||
@@ -68,7 +70,13 @@ Raw sanitized HTML (via rehypeRaw) must keep its content, not render empty:
|
||||
// AssistantMessage, and fenced code blocks go through CodeDisplay →
|
||||
// HighlightCode. Exercise several languages + prose so the code-block styling
|
||||
// can be tuned against the real render path, not an approximation.
|
||||
const chatSampleContent = `Here's how you'd wire up the trigger. First, some prose with \`inline code\`, a [link](https://windmill.dev), and **bold** text so we can see how code sits next to surrounding content.
|
||||
const chatSampleContent = `# Wiring up the trigger
|
||||
|
||||
Here's how you'd wire up the trigger. First, some prose with \`inline code\`, a [link](https://windmill.dev), and **bold** text so we can see how code sits next to surrounding content.
|
||||
|
||||
## The script
|
||||
|
||||
### A subsection
|
||||
|
||||
\`\`\`python
|
||||
def main(name: str = "world"):
|
||||
@@ -188,7 +196,19 @@ That's the full round-trip.`
|
||||
</div>
|
||||
</TabContent>
|
||||
<TabContent value="markdown" class="p-4">
|
||||
<GfmMarkdown md={sampleMarkdown} />
|
||||
<div class="text-xs text-tertiary mb-3">
|
||||
The three <code>markdownProse</code> presets, same source. Each is sized for its own
|
||||
surface: <code>xs</code> for group notes and chat reasoning, <code>sm</code> for chat
|
||||
bubbles, sticky notes and markdown job results, <code>doc</code> for artifacts.
|
||||
</div>
|
||||
<div class="grid gap-4 md:grid-cols-3">
|
||||
{#each ['xs', 'sm', 'doc'] as const as prose}
|
||||
<div class="border border-border-light rounded-lg p-3 bg-surface min-w-0">
|
||||
<div class="text-2xs text-tertiary font-mono mb-2">{prose}</div>
|
||||
<GfmMarkdown md={sampleMarkdown} {prose} />
|
||||
</div>
|
||||
{/each}
|
||||
</div>
|
||||
</TabContent>
|
||||
<TabContent value="chat" class="p-4">
|
||||
<div class="text-xs text-tertiary mb-3">
|
||||
|
||||
Reference in New Issue
Block a user