diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b1ef392b..e448519a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,9 +17,9 @@ ### Added -- Check designs from the new Lint tab in the right panel: issues on the page, in the selection, or across the document are grouped by rule, hovering one highlights its layer on the canvas, clicking selects it and brings it into view, and one-click fixes bind colors to the variable they match and round subpixel geometry for a row or a whole group, and snap radius, spacing, and small text to the scale one row at a time. Rules can be turned off individually or switched between the Recommended, Strict, and Accessibility presets. +- Check designs from the new Lint tab in the right panel: issues on the page, in the selection, or across the document are grouped by rule, hovering one highlights its layer on the canvas, clicking selects it and brings it into view, and one-click fixes bind colors to the variable they match and round subpixel geometry for a row or a whole group, and snap radius, spacing, and small text to the scale, convert groups to frames, and delete hidden layers one row at a time. Rules can be turned off individually or switched between the Recommended, Strict, and Accessibility presets. - See which layer the code is about in the Code tab's Design JSX and Tailwind JSX: the element around the cursor marks its opening and closing tag and shows its layer on the canvas as a tinted box, apart from canvas hover, and design issues are underlined on the property that causes them, including while you edit Design JSX live. -- Fix lint issues outside the app: `openpencil lint --fix -o fixed.fig` binds colors to the variable they match and rounds subpixel geometry, and the `lint` and `lint_fix` tools let MCP clients and AI chat check a page and apply those fixes, plus radius, spacing, and text size suggestions on request. Lint messages carry each fix as data in `fix` and `suggestions`. +- Fix lint issues outside the app: `openpencil lint --fix -o fixed.fig` binds colors to the variable they match and rounds subpixel geometry, and the `lint` and `lint_fix` tools let MCP clients and AI chat check a page and apply those fixes, plus radius, spacing, text size, group-to-frame, and hidden-layer suggestions on request. Lint messages carry each fix as data in `fix` and `suggestions`. - Mark layers with errors and warnings on the canvas while you work, and pin those outside the view to the canvas edge in their direction, with matching marks in the Layers panel and error and warning counts in the page list; hover a marker or pin for its issues or click it to open them in Lint, and turn markers on or off with View → Design issues. - Follow collaborators and their AI agents from the avatars in the toolbar: an avatar counts that person's agents, hovering lists what each is doing and on which page, and clicking follows. A frame in their color and a “Following …” bar show whom you follow; Escape, clicking, scrolling, zooming, or switching pages stops it. Your own avatar renames your agents and leaves the room. - See where the built-in AI chat is working: while it replies, a cursor whose outlined label shows a sparkle and a callsign such as *Fern* marks the layers it edits. In a shared room, collaborators see each other's agents in the color of the person running them. @@ -53,6 +53,7 @@ ### Fixed +- Give paints set through the plugin API, `eval`, and AI and MCP scripts an opacity of 1 and make them visible when the script leaves those out, as Figma does. Such paints were stored without them, which the Design panel could not show. - Show variable-bound colours and numbers correctly when a `.fig` exported from OpenPencil opens in Figma. Figma draws the value a bound field stores until something makes it resolve the variable again, and exports stored the colour from before the binding, so a bound fill appeared in its old colour. Each bound field is now written as it resolves in its layer's mode, or in the collection's default mode when the layer sets none. - Open Figma files that use slots with each instance's own slot content instead of its component's default, keep slot properties, their settings, and instance content when saving back to `.fig`, and keep an instance's slot content when you switch its variant. - Stop showing a “signal is aborted without reason” error when you switch pages again before the previous page has finished loading. diff --git a/packages/core/src/editor/bridges/lint.ts b/packages/core/src/editor/bridges/lint.ts new file mode 100644 index 000000000..6c43a5761 --- /dev/null +++ b/packages/core/src/editor/bridges/lint.ts @@ -0,0 +1,34 @@ +import type { createClipboardActions } from '#core/editor/clipboard' +import type { createNodeActions } from '#core/editor/nodes' +import type { createStructureActions } from '#core/editor/structure' +import type { EditorContext } from '#core/editor/types' +import { applyLintFixes as applyFixes, type LintFixRequest } from '#core/lint/fixes' + +type NodeActions = ReturnType +type StructureActions = ReturnType +type ClipboardActions = ReturnType + +export function createLintFixBridge( + ctx: EditorContext, + nodes: NodeActions, + structure: StructureActions, + clipboard: ClipboardActions +) { + /** Applies lint fixes that still hold as one undo step; returns how many applied. */ + function applyLintFixes(requests: readonly LintFixRequest[]): number { + return ctx.undo.runBatch('Fix design issues', () => + applyFixes( + { + graph: ctx.graph, + updateNode: (id, changes) => nodes.updateNodeWithUndo(id, changes, 'Fix design issue'), + bindVariable: nodes.bindVariable, + convertToFrame: structure.convertGroupToFrame, + deleteNodes: (ids) => clipboard.deleteNodes(ids) + }, + requests + ) + ) + } + + return { applyLintFixes } +} diff --git a/packages/core/src/editor/clipboard.ts b/packages/core/src/editor/clipboard.ts index 13c8dbb7b..2c736957f 100644 --- a/packages/core/src/editor/clipboard.ts +++ b/packages/core/src/editor/clipboard.ts @@ -310,14 +310,18 @@ export function createClipboardActions(ctx: EditorContext) { return missingImageHashes(nodeIds).length > 0 } - function deleteSelected() { + /** + * Deletes layers with their children as one undo step, skipping locked ones. Deleted layers + * leave the selection, and undo restores the selection from before. + */ + function deleteNodes(nodeIds: Iterable, nextSelection?: ReadonlySet) { const entries: Array<{ id: string parentId: string index: number subtree: Map }> = [] - for (const id of ctx.state.selectedIds) { + for (const id of nodeIds) { const node = ctx.graph.getNode(id) if (!node || node.locked) continue const parentId = node.parentId ?? ctx.state.currentPageId @@ -336,13 +340,15 @@ export function createClipboardActions(ctx: EditorContext) { const prevSelection = new Set(ctx.state.selectedIds) for (const { id } of entries) ctx.graph.deleteNode(id) relayoutParents() + const selectionAfter = + nextSelection ?? new Set([...prevSelection].filter((id) => ctx.graph.getNode(id))) ctx.undo.push({ label: 'Delete', forward: () => { for (const { id } of entries) ctx.graph.deleteNode(id) relayoutParents() - ctx.setSelectedIds(new Set()) + ctx.setSelectedIds(new Set(selectionAfter)) }, inverse: () => { restoreDeletedEntries(ctx, entries) @@ -350,7 +356,11 @@ export function createClipboardActions(ctx: EditorContext) { ctx.setSelectedIds(prevSelection) } }) - ctx.setSelectedIds(new Set()) + ctx.setSelectedIds(new Set(selectionAfter)) + } + + function deleteSelected() { + deleteNodes(ctx.state.selectedIds, new Set()) } const copyActions = createClipboardCopyActions(ctx) @@ -368,6 +378,7 @@ export function createClipboardActions(ctx: EditorContext) { pasteSnapshot, pasteFromHTML, warnMissingImages, + deleteNodes, deleteSelected, ...assetActions, ...exportActions diff --git a/packages/core/src/editor/create.ts b/packages/core/src/editor/create.ts index 264fe3ced..ae3737bfd 100644 --- a/packages/core/src/editor/create.ts +++ b/packages/core/src/editor/create.ts @@ -20,6 +20,7 @@ import { fontResolver } from '#core/text/resolver' import { createAlignmentActions } from './alignment' import { createClipboardBridge } from './bridges/clipboard' import { createComponentBridge } from './bridges/components' +import { createLintFixBridge } from './bridges/lint' import { createStructureBridge } from './bridges/structure' import { createUndoBridge } from './bridges/undo' import { createClipboardActions } from './clipboard' @@ -235,6 +236,7 @@ export function createEditor(options?: EditorOptions) { const componentBridge = createComponentBridge(components, selection, structure, pages) const structureBridge = createStructureBridge(structure, selection) const undoBridge = createUndoBridge(undoActions, selection) + const lintFixBridge = createLintFixBridge(ctx, nodes, structure, clipboard) function setCanvasKit(ck: CanvasKit, renderer: SkiaRenderer) { _ck = ck @@ -371,7 +373,10 @@ export function createEditor(options?: EditorOptions) { ...componentBridge, // Structure — bridge functions that need selectedNodes - ...structureBridge + ...structureBridge, + + // Lint fixes, which touch nodes, structure and deletion in one undo step + ...lintFixBridge } } diff --git a/packages/core/src/editor/nodes.ts b/packages/core/src/editor/nodes.ts index a1a9552aa..56e060923 100644 --- a/packages/core/src/editor/nodes.ts +++ b/packages/core/src/editor/nodes.ts @@ -9,7 +9,6 @@ import { } from '@open-pencil/scene-graph' import { reconcileVariableLayouts } from '#core/layout/variables' -import { applyLintFixes as applyFixes, type LintFixRequest } from '#core/lint/fixes' import { createLayoutModeActions } from './layout-mode' import { createNodePreviewActions } from './node-preview' @@ -111,23 +110,8 @@ export function createNodeActions(ctx: EditorContext) { ) } - /** Applies lint fixes that still hold as one undo step; returns how many applied. */ - function applyLintFixes(requests: readonly LintFixRequest[]): number { - return ctx.undo.runBatch('Fix design issues', () => - applyFixes( - { - graph: ctx.graph, - updateNode: (id, changes) => updateNodeWithUndo(id, changes, 'Fix design issue'), - bindVariable: variableBindingActions.bindVariable - }, - requests - ) - ) - } - return { updateNode, - applyLintFixes, ...createNodePreviewActions(ctx, updateNode), updateNodeWithUndo, setOpacity, diff --git a/packages/core/src/editor/structure.ts b/packages/core/src/editor/structure.ts index a8cf988a3..4ee72e435 100644 --- a/packages/core/src/editor/structure.ts +++ b/packages/core/src/editor/structure.ts @@ -67,6 +67,24 @@ export function createStructureActions(ctx: EditorContext) { }) } + /** + * Turns a group into a frame in place. The layer keeps its id, children, bounds and look: a + * group has no fill and does not clip, and the frame starts the same way. + */ + function convertGroupToFrame(nodeId: string) { + if (ctx.graph.getNode(nodeId)?.type !== 'GROUP') return + const apply = (type: 'GROUP' | 'FRAME') => { + ctx.graph.updateNode(nodeId, { type }) + ctx.runLayoutForNode(nodeId) + } + apply('FRAME') + ctx.undo.push({ + label: 'Convert to frame', + forward: () => apply('FRAME'), + inverse: () => apply('GROUP') + }) + } + function booleanOperationSelected(selectedNodes: SceneNode[], operation: BooleanOperation) { return booleanOperationSelectedImpl(ctx, selectedNodes, operation) } @@ -142,6 +160,7 @@ export function createStructureActions(ctx: EditorContext) { wrapInAutoLayout, groupSelected, frameSelection, + convertGroupToFrame, booleanOperationSelected, ungroupSelected, flattenSelected, diff --git a/packages/core/src/figma-api/accessors/visual.ts b/packages/core/src/figma-api/accessors/visual.ts index eca0e86e7..16b547664 100644 --- a/packages/core/src/figma-api/accessors/visual.ts +++ b/packages/core/src/figma-api/accessors/visual.ts @@ -24,6 +24,11 @@ function styleReference( } } +/** Figma treats a paint's `opacity` and `visible` as optional, defaulting to 1 and `true`. */ +function paintDefaults(paint: Partial>) { + return { opacity: paint.opacity ?? 1, visible: paint.visible ?? true } +} + export function installVisualNodeProxyAccessors( prototype: object, internals: NodeProxyInternals, @@ -38,6 +43,7 @@ export function installVisualNodeProxyAccessors( updateNode(this, internals, { fills: value.map((fill) => ({ ...fill, + ...paintDefaults(fill), color: normalizeColor(fill.color), gradientStops: fill.gradientStops?.map((stop) => ({ ...stop, @@ -53,7 +59,11 @@ export function installVisualNodeProxyAccessors( }, set(this: ProxyThis, value: readonly Stroke[]) { updateNode(this, internals, { - strokes: value.map((stroke) => ({ ...stroke, color: normalizeColor(stroke.color) })) + strokes: value.map((stroke) => ({ + ...stroke, + ...paintDefaults(stroke), + color: normalizeColor(stroke.color) + })) }) } }, diff --git a/packages/core/src/lint/fixes.ts b/packages/core/src/lint/fixes.ts index 5be55b39a..45297ad08 100644 --- a/packages/core/src/lint/fixes.ts +++ b/packages/core/src/lint/fixes.ts @@ -2,6 +2,7 @@ import type { SceneGraph, SceneNode } from '@open-pencil/scene-graph' import { colorToHex } from '@open-pencil/scene-graph/color' import type { LintFix, LintMessage } from './types' +import { ownsComponentTree } from './utils' /** A fix for the layer it was reported on. A `LintMessage` with a `fix` is one. */ export interface LintFixRequest { @@ -17,13 +18,20 @@ export interface LintFixTarget { readonly graph: SceneGraph updateNode(nodeId: string, changes: Partial): void bindVariable(nodeId: string, path: string, variableId: string): void + convertToFrame(nodeId: string): void + /** Deletes layers with their children; none of them is inside another. */ + deleteNodes(nodeIds: readonly string[]): void } export function graphFixTarget(graph: SceneGraph): LintFixTarget { return { graph, updateNode: (nodeId, changes) => graph.updateNode(nodeId, changes), - bindVariable: (nodeId, path, variableId) => graph.bindVariable(nodeId, path, variableId) + bindVariable: (nodeId, path, variableId) => graph.bindVariable(nodeId, path, variableId), + convertToFrame: (nodeId) => graph.updateNode(nodeId, { type: 'FRAME' }), + deleteNodes: (nodeIds) => { + for (const nodeId of nodeIds) graph.deleteNode(nodeId) + } } } @@ -49,30 +57,76 @@ function canBind(graph: SceneGraph, node: SceneNode, path: string, variableId: s return colorToHex({ ...paint.color, a: 1 }) === colorToHex({ ...color, a: 1 }) } +function* ancestors(graph: SceneGraph, node: SceneNode): Generator { + for (let id = node.parentId; id;) { + const parent = graph.getNode(id) + if (!parent) return + yield parent + id = parent.parentId + } +} + +/** + * Structural fixes apply only where the layer's structure is the user's to change: not locked, + * and not inside a component or an instance. + */ +function canRestructure(graph: SceneGraph, node: SceneNode): boolean { + if (node.locked) return false + for (const parent of ancestors(graph, node)) if (ownsComponentTree(parent.type)) return false + return true +} + +function hasAncestorIn(graph: SceneGraph, node: SceneNode, ids: ReadonlySet): boolean { + for (const parent of ancestors(graph, node)) if (ids.has(parent.id)) return true + return false +} + /** * Applies fixes that still hold against the current graph. Changes to one layer merge into a - * single update. Returns the number of fixes applied. + * single update, and deletions run last so other fixes still find their layers. Returns the + * number of fixes applied. */ export function applyLintFixes(target: LintFixTarget, requests: readonly LintFixRequest[]): number { const { graph } = target const updates = new Map>() + const conversions = new Set() + const deletions = new Set() let applied = 0 for (const { nodeId, fix } of requests) { const node = graph.getNode(nodeId) if (!node) continue - if (fix.kind === 'bind-variable') { - if (!canBind(graph, node, fix.path, fix.variableId)) continue - target.bindVariable(nodeId, fix.path, fix.variableId) - applied++ - continue + switch (fix.kind) { + case 'bind-variable': + if (!canBind(graph, node, fix.path, fix.variableId)) continue + target.bindVariable(nodeId, fix.path, fix.variableId) + break + case 'set': { + const changes = Object.fromEntries( + Object.entries(fix.changes).filter(([key, value]) => Reflect.get(node, key) !== value) + ) + if (Object.keys(changes).length === 0) continue + updates.set(nodeId, { ...updates.get(nodeId), ...changes }) + break + } + case 'convert-to-frame': + if (node.type !== 'GROUP' || conversions.has(nodeId) || !canRestructure(graph, node)) + continue + conversions.add(nodeId) + break + case 'delete': + // Only a layer that is still hidden: one shown since the check is in use. + if (node.visible || deletions.has(nodeId) || !canRestructure(graph, node)) continue + deletions.add(nodeId) + break } - const changes = Object.fromEntries( - Object.entries(fix.changes).filter(([key, value]) => Reflect.get(node, key) !== value) - ) - if (Object.keys(changes).length === 0) continue - updates.set(nodeId, { ...updates.get(nodeId), ...changes }) applied++ } for (const [nodeId, changes] of updates) target.updateNode(nodeId, changes) + for (const nodeId of conversions) target.convertToFrame(nodeId) + const roots = [...deletions].filter((nodeId) => { + const node = graph.getNode(nodeId) + return node ? !hasAncestorIn(graph, node, deletions) : false + }) + if (roots.length > 0) target.deleteNodes(roots) return applied } diff --git a/packages/core/src/lint/rules/no-groups.ts b/packages/core/src/lint/rules/no-groups.ts index 3baf18927..3153c6374 100644 --- a/packages/core/src/lint/rules/no-groups.ts +++ b/packages/core/src/lint/rules/no-groups.ts @@ -1,4 +1,6 @@ import { defineRule } from '#core/lint/rule' +import { inComponentTree } from '#core/lint/utils' + export default defineRule({ meta: { id: 'no-groups', @@ -10,7 +12,10 @@ export default defineRule({ context.report({ node, message: 'Group should be converted to Frame', - suggest: 'Groups cannot use auto layout. Convert to Frame for better control.' + suggest: 'Groups cannot use auto layout. Convert to Frame for better control.', + ...(node.locked || inComponentTree(node) + ? {} + : { suggestions: [{ kind: 'convert-to-frame' }] }) }) } }) diff --git a/packages/core/src/lint/rules/no-hidden-layers.ts b/packages/core/src/lint/rules/no-hidden-layers.ts index e378789d7..a3ed7b4b7 100644 --- a/packages/core/src/lint/rules/no-hidden-layers.ts +++ b/packages/core/src/lint/rules/no-hidden-layers.ts @@ -1,4 +1,6 @@ import { defineRule } from '#core/lint/rule' +import { inComponentTree } from '#core/lint/utils' + export default defineRule({ meta: { id: 'no-hidden-layers', @@ -6,11 +8,12 @@ export default defineRule({ description: 'Hidden layers may indicate unused elements' }, check(node, context) { - if (!node.visible) - context.report({ - node, - message: 'Hidden layer detected', - suggest: 'Delete if unused or keep only if required for component states' - }) + if (node.visible) return + context.report({ + node, + message: 'Hidden layer detected', + suggest: 'Delete if unused or keep only if required for component states', + ...(node.locked || inComponentTree(node) ? {} : { suggestions: [{ kind: 'delete' }] }) + }) } }) diff --git a/packages/core/src/lint/types.ts b/packages/core/src/lint/types.ts index 543f9990d..6479a73b8 100644 --- a/packages/core/src/lint/types.ts +++ b/packages/core/src/lint/types.ts @@ -40,6 +40,10 @@ export type LintFixProperty = export type LintFix = | { kind: 'bind-variable'; path: string; variableId: string; variableName: string } | { kind: 'set'; changes: Readonly>> } + /** Turns a group into a frame in place, keeping its children, bounds and look. */ + | { kind: 'convert-to-frame' } + /** Deletes the layer with its children. */ + | { kind: 'delete' } export interface LintMessage { ruleId: string diff --git a/packages/core/src/lint/utils.ts b/packages/core/src/lint/utils.ts index eb0f5a149..90965e763 100644 --- a/packages/core/src/lint/utils.ts +++ b/packages/core/src/lint/utils.ts @@ -52,3 +52,26 @@ export function getNodePath(node: LintPathNode): string[] { } export const SPACING_SCALE = [0, 1, 2, 4, 8, 12, 16, 20, 24, 32, 40, 48, 56, 64, 80, 96, 128] + +const COMPONENT_TREE_TYPES = new Set(['COMPONENT', 'COMPONENT_SET', 'INSTANCE']) + +/** Layer types whose descendants' structure belongs to a component. */ +export function ownsComponentTree(type: string): boolean { + return COMPONENT_TREE_TYPES.has(type) +} + +interface LintTreeNode { + type: string + parent?: LintTreeNode +} + +/** + * Whether a layer sits inside a component or an instance, whose structure belongs to the + * component: instance layers cannot be removed, and a component property may show a hidden one. + */ +export function inComponentTree(node: LintTreeNode): boolean { + for (let current = node.parent; current; current = current.parent) { + if (ownsComponentTree(current.type)) return true + } + return false +} diff --git a/packages/core/src/tools/analyze/lint.ts b/packages/core/src/tools/analyze/lint.ts index 19ec816ce..fe220251b 100644 --- a/packages/core/src/tools/analyze/lint.ts +++ b/packages/core/src/tools/analyze/lint.ts @@ -91,8 +91,9 @@ export const lintFix = defineTool({ description: 'Apply lint fixes: bind colors to the variable they match and round geometry to whole pixels. ' + 'With suggestions: true, also apply each finding’s first suggestion (snap radius and spacing to the scale, ' + - 'raise text to the minimum size). Returns what was applied and what remains.', - execution: { kind: 'sync', mutation: 'properties' }, + 'raise text to the minimum size, convert groups to frames, delete hidden layers). ' + + 'Returns what was applied and what remains.', + execution: { kind: 'sync', mutation: 'document' }, input: v.object({ ...lintScopeInput, suggestions: v.optional( diff --git a/packages/core/tests/lint/fixes.test.ts b/packages/core/tests/lint/fixes.test.ts index 402883a13..bc94b4929 100644 --- a/packages/core/tests/lint/fixes.test.ts +++ b/packages/core/tests/lint/fixes.test.ts @@ -183,3 +183,116 @@ describe('editor.applyLintFixes', () => { expect(restored.boundVariables['fills/0/color']).toBeUndefined() }) }) + +function suggestionRequests(messages: readonly LintMessage[]) { + return messages.flatMap((m) => (m.suggestions ?? []).map((fix) => ({ nodeId: m.nodeId, fix }))) +} + +describe('structure suggestions', () => { + test('convert a group to a frame in place, keeping its layers and look', () => { + const graph = new SceneGraph() + const pageId = graph.getPages()[0].id + const group = graph.createNode('GROUP', pageId, { x: 40, y: 20, width: 30, height: 30 }) + const dot = graph.createNode('ELLIPSE', group.id, { width: 10, height: 10 }) + + const messages = lint(graph, 'no-groups') + expect(messages[0]?.suggestions).toEqual([{ kind: 'convert-to-frame' }]) + expect(safeFixes(messages)).toEqual([]) + + expect(applyLintFixes(graphFixTarget(graph), suggestionRequests(messages))).toBe(1) + const frame = getNodeOrThrow(graph, group.id) + expect(frame).toMatchObject({ type: 'FRAME', x: 40, y: 20, width: 30, height: 30 }) + expect(frame.childIds).toEqual([dot.id]) + expect(frame.fills).toEqual([]) + expect(frame.clipsContent).toBe(false) + expect(lint(graph, 'no-groups')).toEqual([]) + }) + + test('delete hidden layers once, together with hidden layers inside them', () => { + const graph = new SceneGraph() + const pageId = graph.getPages()[0].id + const panel = graph.createNode('FRAME', pageId, { visible: false }) + const inner = graph.createNode('RECTANGLE', panel.id, { visible: false }) + const shown = graph.createNode('RECTANGLE', pageId) + + const messages = lint(graph, 'no-hidden-layers') + expect(messages.map((m) => m.suggestions)).toEqual([[{ kind: 'delete' }], [{ kind: 'delete' }]]) + + expect(applyLintFixes(graphFixTarget(graph), suggestionRequests(messages))).toBe(2) + expect(graph.getNode(panel.id)).toBeUndefined() + expect(graph.getNode(inner.id)).toBeUndefined() + expect(graph.getNode(shown.id)).toBeDefined() + }) + + test('keep a layer shown again since the check', () => { + const graph = new SceneGraph() + const layer = graph.createNode('RECTANGLE', graph.getPages()[0].id, { visible: false }) + const messages = lint(graph, 'no-hidden-layers') + + graph.updateNode(layer.id, { visible: true }) + + expect(applyLintFixes(graphFixTarget(graph), suggestionRequests(messages))).toBe(0) + expect(graph.getNode(layer.id)).toBeDefined() + }) + + test('leave locked layers and the structure of components and instances alone', () => { + const graph = new SceneGraph() + const pageId = graph.getPages()[0].id + const lockedGroup = graph.createNode('GROUP', pageId, { locked: true }) + const lockedHidden = graph.createNode('RECTANGLE', pageId, { visible: false, locked: true }) + const button = graph.createNode('COMPONENT', pageId) + const icon = graph.createNode('GROUP', button.id) + const badge = graph.createNode('RECTANGLE', button.id, { visible: false }) + const instance = graph.createNode('INSTANCE', pageId, { componentId: button.id }) + const copy = graph.createNode('RECTANGLE', instance.id, { visible: false }) + + const messages = [...lint(graph, 'no-groups'), ...lint(graph, 'no-hidden-layers')] + // The linter does not look inside instances, so the copy is only checked when applying. + const reported = [lockedGroup, lockedHidden, icon, badge].map((node) => node.id) + expect(reported.every((id) => messages.some((m) => m.nodeId === id))).toBe(true) + expect(messages.every((m) => m.suggestions === undefined)).toBe(true) + + // Requests built elsewhere are checked against the graph as well. + const requests = [ + { nodeId: lockedGroup.id, fix: { kind: 'convert-to-frame' } as const }, + { nodeId: icon.id, fix: { kind: 'convert-to-frame' } as const }, + { nodeId: lockedHidden.id, fix: { kind: 'delete' } as const }, + { nodeId: copy.id, fix: { kind: 'delete' } as const } + ] + expect(applyLintFixes(graphFixTarget(graph), requests)).toBe(0) + expect(getNodeOrThrow(graph, icon.id).type).toBe('GROUP') + expect(graph.getNode(copy.id)).toBeDefined() + }) +}) + +describe('editor structure fixes', () => { + test('convert and delete as one undo step that restores layers and selection', () => { + const editor = createEditor() + const pageId = editor.graph.getPages()[0].id + const group = editor.graph.createNode('GROUP', pageId, { width: 30, height: 30 }) + editor.graph.createNode('ELLIPSE', group.id, { width: 10, height: 10 }) + const hidden = editor.graph.createNode('RECTANGLE', pageId, { visible: false }) + editor.select([hidden.id, group.id]) + + const messages = createLinter({ + rules: ['no-groups', 'no-hidden-layers'], + config: { rules: { 'no-groups': 'warning', 'no-hidden-layers': 'warning' } } + }).lintGraph(editor.graph).messages + + expect(editor.applyLintFixes(suggestionRequests(messages))).toBe(2) + expect(getNodeOrThrow(editor.graph, group.id).type).toBe('FRAME') + expect(editor.graph.getNode(hidden.id)).toBeUndefined() + expect([...editor.state.selectedIds]).toEqual([group.id]) + + editor.undoAction() + + expect(getNodeOrThrow(editor.graph, group.id).type).toBe('GROUP') + expect(getNodeOrThrow(editor.graph, hidden.id).visible).toBe(false) + expect(new Set(editor.state.selectedIds)).toEqual(new Set([hidden.id, group.id])) + + editor.redoAction() + + expect(getNodeOrThrow(editor.graph, group.id).type).toBe('FRAME') + expect(editor.graph.getNode(hidden.id)).toBeUndefined() + }) +}) diff --git a/packages/docs/user-guide/checking-designs.md b/packages/docs/user-guide/checking-designs.md index 584136d9c..1d0e8a7ba 100644 --- a/packages/docs/user-guide/checking-designs.md +++ b/packages/docs/user-guide/checking-designs.md @@ -25,6 +25,7 @@ Rows that can be fixed in one step show a button on hover: - A hardcoded color that matches one of the document's color variables binds to it (link button). The row names the variable. - Subpixel positions and sizes round to whole pixels. Values that auto layout or text resizing sets, and vector artwork and the parts of groups, are left alone. - Off-scale corner radius and spacing change to the nearest scale value, and text below the minimum size grows to it (wand button). These change the design, so they apply one row at a time. +- A group converts to a frame in place, keeping its layers, position, and look (frame button), and a hidden layer can be deleted (trash button). Neither is offered for locked layers or layers inside a component or instance, whose structure belongs to the component. Binding colors and rounding pixels keep the design as it looks, so their groups also offer **Bind all** or **Fix all**. Every fix is a single undo step. diff --git a/packages/vue/src/i18n/locales/de/design-check.json b/packages/vue/src/i18n/locales/de/design-check.json index 79486094b..da2c1f9a4 100644 --- a/packages/vue/src/i18n/locales/de/design-check.json +++ b/packages/vue/src/i18n/locales/de/design-check.json @@ -65,6 +65,8 @@ "fixRoundPixels": "Auf ganze Pixel runden", "fixUseValue": "Auf {value} ändern", "fixFullRadius": "Vollen Radius verwenden", + "fixConvertToFrame": "In Rahmen umwandeln", + "fixDeleteLayer": "Ebene löschen", "fixAll": "Alle {count} beheben", "ruleNoDefaultNames": "Standardname der Ebene", "ruleNoDefaultNamesHelp": "Aussagekräftige Namen machen Ebenen leichter auffindbar, übergebbar und wiederverwendbar.", diff --git a/packages/vue/src/i18n/locales/es/design-check.json b/packages/vue/src/i18n/locales/es/design-check.json index 25110129e..a3c7e6fbd 100644 --- a/packages/vue/src/i18n/locales/es/design-check.json +++ b/packages/vue/src/i18n/locales/es/design-check.json @@ -65,6 +65,8 @@ "fixRoundPixels": "Redondear a píxeles enteros", "fixUseValue": "Cambiar a {value}", "fixFullRadius": "Usar radio completo", + "fixConvertToFrame": "Convertir en marco", + "fixDeleteLayer": "Eliminar capa", "fixAll": "Corregir las {count}", "ruleNoDefaultNames": "Nombre de capa predeterminado", "ruleNoDefaultNamesHelp": "Los nombres descriptivos facilitan encontrar, entregar y reutilizar capas.", diff --git a/packages/vue/src/i18n/locales/fr/design-check.json b/packages/vue/src/i18n/locales/fr/design-check.json index f415872db..2b85ad6b2 100644 --- a/packages/vue/src/i18n/locales/fr/design-check.json +++ b/packages/vue/src/i18n/locales/fr/design-check.json @@ -65,6 +65,8 @@ "fixRoundPixels": "Arrondir au pixel entier", "fixUseValue": "Remplacer par {value}", "fixFullRadius": "Utiliser un rayon complet", + "fixConvertToFrame": "Convertir en cadre", + "fixDeleteLayer": "Supprimer le calque", "fixAll": "Tout corriger ({count})", "ruleNoDefaultNames": "Nom de calque par défaut", "ruleNoDefaultNamesHelp": "Des noms descriptifs facilitent la recherche, la transmission et la réutilisation des calques.", diff --git a/packages/vue/src/i18n/locales/it/design-check.json b/packages/vue/src/i18n/locales/it/design-check.json index 5b3f02929..a8cbea451 100644 --- a/packages/vue/src/i18n/locales/it/design-check.json +++ b/packages/vue/src/i18n/locales/it/design-check.json @@ -65,6 +65,8 @@ "fixRoundPixels": "Arrotonda a pixel interi", "fixUseValue": "Cambia in {value}", "fixFullRadius": "Usa raggio completo", + "fixConvertToFrame": "Converti in frame", + "fixDeleteLayer": "Elimina livello", "fixAll": "Correggi tutti ({count})", "ruleNoDefaultNames": "Nome del livello predefinito", "ruleNoDefaultNamesHelp": "Nomi descrittivi rendono i livelli più facili da trovare, consegnare e riutilizzare.", diff --git a/packages/vue/src/i18n/locales/ja/design-check.json b/packages/vue/src/i18n/locales/ja/design-check.json index 2d93790f7..848589439 100644 --- a/packages/vue/src/i18n/locales/ja/design-check.json +++ b/packages/vue/src/i18n/locales/ja/design-check.json @@ -65,6 +65,8 @@ "fixRoundPixels": "整数ピクセルに丸める", "fixUseValue": "{value} に変更", "fixFullRadius": "最大の角丸を使う", + "fixConvertToFrame": "フレームに変換", + "fixDeleteLayer": "レイヤーを削除", "fixAll": "{count} 件すべて修正", "ruleNoDefaultNames": "デフォルトのレイヤー名", "ruleNoDefaultNamesHelp": "わかりやすい名前を付けると、レイヤーを探しやすく、受け渡しや再利用もしやすくなります。", diff --git a/packages/vue/src/i18n/locales/pl/design-check.json b/packages/vue/src/i18n/locales/pl/design-check.json index 216703b86..1782b008d 100644 --- a/packages/vue/src/i18n/locales/pl/design-check.json +++ b/packages/vue/src/i18n/locales/pl/design-check.json @@ -65,6 +65,8 @@ "fixRoundPixels": "Zaokrąglij do pełnych pikseli", "fixUseValue": "Zmień na {value}", "fixFullRadius": "Użyj pełnego promienia", + "fixConvertToFrame": "Przekształć w ramkę", + "fixDeleteLayer": "Usuń warstwę", "fixAll": "Popraw wszystkie ({count})", "ruleNoDefaultNames": "Domyślna nazwa warstwy", "ruleNoDefaultNamesHelp": "Opisowe nazwy ułatwiają znajdowanie, przekazywanie i ponowne użycie warstw.", diff --git a/packages/vue/src/i18n/locales/ru/design-check.json b/packages/vue/src/i18n/locales/ru/design-check.json index a768df05c..dd24f6e07 100644 --- a/packages/vue/src/i18n/locales/ru/design-check.json +++ b/packages/vue/src/i18n/locales/ru/design-check.json @@ -65,6 +65,8 @@ "fixRoundPixels": "Округлить до целых пикселей", "fixUseValue": "Изменить на {value}", "fixFullRadius": "Использовать полное скругление", + "fixConvertToFrame": "Преобразовать во фрейм", + "fixDeleteLayer": "Удалить слой", "fixAll": "Исправить все ({count})", "ruleNoDefaultNames": "Стандартное имя слоя", "ruleNoDefaultNamesHelp": "Понятные имена помогают находить, передавать и переиспользовать слои.", diff --git a/packages/vue/src/i18n/locales/zh-cn/design-check.json b/packages/vue/src/i18n/locales/zh-cn/design-check.json index a5fbf17b0..5224fd2c3 100644 --- a/packages/vue/src/i18n/locales/zh-cn/design-check.json +++ b/packages/vue/src/i18n/locales/zh-cn/design-check.json @@ -65,6 +65,8 @@ "fixRoundPixels": "取整到整像素", "fixUseValue": "改为 {value}", "fixFullRadius": "使用完全圆角", + "fixConvertToFrame": "转换为画框", + "fixDeleteLayer": "删除图层", "fixAll": "全部修复（{count}）", "ruleNoDefaultNames": "默认图层名称", "ruleNoDefaultNamesHelp": "描述性的名称让图层更易查找、交付和复用。", diff --git a/packages/vue/src/i18n/messages/design-check.ts b/packages/vue/src/i18n/messages/design-check.ts index e38331370..0d0442185 100644 --- a/packages/vue/src/i18n/messages/design-check.ts +++ b/packages/vue/src/i18n/messages/design-check.ts @@ -73,6 +73,8 @@ export const designCheckMessageDefaults = { fixRoundPixels: 'Round to whole pixels', fixUseValue: params('Change to {value}'), fixFullRadius: 'Use full radius', + fixConvertToFrame: 'Convert to frame', + fixDeleteLayer: 'Delete layer', fixAll: params('Fix all {count}'), ruleNoDefaultNames: 'Default layer name', ruleNoDefaultNamesHelp: 'Descriptive names make layers easier to find, hand off and reuse.', diff --git a/src/app/code/live-preview.ts b/src/app/code/live-preview.ts index d912c52af..04cf735ed 100644 --- a/src/app/code/live-preview.ts +++ b/src/app/code/live-preview.ts @@ -1,7 +1,7 @@ import { renderTree } from '@open-pencil/core/design-jsx' import { computeAllLayouts } from '@open-pencil/core/layout' import { reconcileRenderedLayers, type TreeNode } from '@open-pencil/design-jsx' -import type { Vector } from '@open-pencil/scene-graph' +import type { SceneNode, Vector } from '@open-pencil/scene-graph' import { convertDesignJSXRoots } from '@/app/code/sandbox/convert' import { evaluateDesignJSX } from '@/app/code/sandbox/evaluate' @@ -140,6 +140,26 @@ export async function previewDesignJSX( } } +/** + * Applies canvas edits made while the code waited to render, which its preview drew over. They + * came after the code, so they win, and join the edit's undo step. Layers the preview removed + * are skipped. + */ +export function reapplyCanvasEdits( + store: EditorStore, + session: DesignJSXEditSession, + edits: ReadonlyMap> +): void { + const present = [...edits].filter(([id]) => store.graph.getNode(id)) + if (present.length === 0) return + const before = store.snapshotPage() + const selection = [...store.state.selectedIds] + for (const [id, changes] of present) store.graph.updateNode(id, changes) + computeAllLayouts(store.graph, store.state.currentPageId) + store.requestRender() + recordPreview(store, session, before, selection) +} + function sameIds(a: readonly string[], b: readonly string[]): boolean { return a.length === b.length && a.every((id, index) => id === b[index]) } diff --git a/src/app/editor/design-check/format.ts b/src/app/editor/design-check/format.ts index abdf54e69..b15b44928 100644 --- a/src/app/editor/design-check/format.ts +++ b/src/app/editor/design-check/format.ts @@ -185,6 +185,8 @@ function fixLabel(fix: LintFix, messages: Messages): string { if (fix.kind === 'bind-variable') { return messages.bindVariable({ variable: fix.variableName }) } + if (fix.kind === 'convert-to-frame') return messages.fixConvertToFrame + if (fix.kind === 'delete') return messages.fixDeleteLayer const changes = Object.entries(fix.changes) // Geometry changes only come from rounding to whole pixels. if (changes.length !== 1 || changes.some(([property]) => property in GEOMETRY_LABELS)) { diff --git a/src/components/CodePanel.vue b/src/components/CodePanel.vue index 184c57818..3e8af606e 100644 --- a/src/components/CodePanel.vue +++ b/src/components/CodePanel.vue @@ -7,12 +7,14 @@ import { onBeforeUnmount, ref, shallowRef, + nextTick, useTemplateRef, watch } from 'vue' import { JSX_REFERENCE } from '@open-pencil/design-jsx' -import { useI18n } from '@open-pencil/vue' +import type { SceneNode } from '@open-pencil/scene-graph' +import { useEditorEvent, useI18n } from '@open-pencil/vue' import { commitDOMCodeSession, @@ -26,6 +28,7 @@ import { useCodeLayers } from '@/app/code/layers/use' import { createDesignJSXEditSession, previewDesignJSX, + reapplyCanvasEdits, resetDesignJSXPreview, type DesignJSXEditSession, type DesignJSXLayerLine @@ -65,6 +68,30 @@ let previewing = 0 let previewedVersion: number | null = null /** The selection the code was generated for. */ let followedSelection: string | null = null +/** Code typed since the last preview started, which the canvas does not show yet. */ +let previewDue = false +/** + * Canvas edits made while code waits to render. Code replaced as a whole has no links to patch + * them into until its preview links it again, so they are applied once more after that preview. + */ +let canvasEdits = new Map>() + +useEditorEvent('node:updated', (id, changes) => { + if (!previewDue || previewing > 0 || source.value !== 'design-jsx') return + canvasEdits.set(id, { ...canvasEdits.get(id), ...structuredClone(changes) }) +}) + +function takeCanvasEdits(): Map> { + const edits = canvasEdits + canvasEdits = new Map() + previewDue = false + return edits +} + +/** Hands edits taken by a superseded or failed preview to the next one, behind any made since. */ +function returnCanvasEdits(edits: ReadonlyMap>) { + for (const [id, changes] of edits) canvasEdits.set(id, { ...changes, ...canvasEdits.get(id) }) +} /** Set by "Write JSX": the editor stays open for new layers until something is selected. */ const composing = ref(false) @@ -141,6 +168,7 @@ async function commitCurrentSession(): Promise { await pendingPreview await previewQueue updateVersion += 1 + takeCanvasEdits() const dom = domSession designSession.value = null domSession = null @@ -157,6 +185,7 @@ async function commitCurrentSession(): Promise { async function runPreview(version: number): Promise { if (version !== updateVersion || readOnly.value || !draft.value.trim()) return + const edits = takeCanvasEdits() status.value = 'updating' error.value = '' previewing += 1 @@ -178,14 +207,26 @@ async function runPreview(version: number): Promise { previewedVersion = store.state.sceneVersion followedSelection = selectionKey.value } - if (version !== updateVersion) return + if (version !== updateVersion) { + returnCanvasEdits(edits) + return + } if (previewLayers) codeLayers.showPreview(previewLayers) if (!result.ok) { status.value = 'error' error.value = result.error + // The code still waits to render, so its next preview applies these and later edits. + previewDue = true + returnCanvasEdits(edits) return } status.value = 'updated' + const session = designSession.value + if (!session || edits.size === 0) return + // Once the code is linked again, the edits reach it as canvas changes do. + await nextTick() + if (version === updateVersion) reapplyCanvasEdits(store, session, edits) + else returnCanvasEdits(edits) } const schedulePreview = useDebounceFn( @@ -204,6 +245,7 @@ function updateDraft(value: string, origin: 'user' | 'layers'): void { edited.value = true error.value = '' updateVersion += 1 + previewDue = true pendingPreview = schedulePreview(updateVersion) } @@ -211,6 +253,7 @@ async function resetDraft(): Promise { updateVersion += 1 await pendingPreview await previewQueue + takeCanvasEdits() const design = designSession.value const dom = domSession designSession.value = null @@ -255,7 +298,7 @@ function showCanvas(next: Exclude) { * generated again; code the person wrote is patched where layers changed, keeping the rest as * written. Another selection starts over with its generated code. */ -function followCanvas() { +function followCanvas(live = false) { const current = source.value if (!editorActive.value || current === 'html-css' || previewing > 0) return if (selectionKey.value !== followedSelection) { @@ -266,15 +309,27 @@ function followCanvas() { showCanvas(current) return } - if (store.state.sceneVersion === previewedVersion) return + // A live preview leaves the scene version alone, so it is followed whatever the version. + if (!live && store.state.sceneVersion === previewedVersion) return codeEditor.value?.patchFromLayers() } -watch([() => store.state.sceneVersion, selectionKey, source, editorActive], followCanvas, { +watch([() => store.state.sceneVersion, selectionKey, source, editorActive], () => followCanvas(), { immediate: true }) +/** Dragging or scrubbing a value previews it; the code follows once per frame, like the Design panel. */ +let livePreviewFrame = 0 +useEditorEvent('node:previewUpdated', () => { + if (livePreviewFrame) return + livePreviewFrame = requestAnimationFrame(() => { + livePreviewFrame = 0 + followCanvas(true) + }) +}) + onBeforeUnmount(() => { + cancelAnimationFrame(livePreviewFrame) void commitCurrentSession() }) diff --git a/src/components/design-check/IssueRow.vue b/src/components/design-check/IssueRow.vue index 8ec252764..d4aaa20ce 100644 --- a/src/components/design-check/IssueRow.vue +++ b/src/components/design-check/IssueRow.vue @@ -74,6 +74,16 @@ const styles = designCheck() class="size-3" aria-hidden="true" /> +