feat: add set_solid_fills for multi-node paint updates - #46
Conversation
Theming a component's variants currently costs one WebSocket round-trip per node, which does not scale past a handful of layers. set_solid_fills takes an `items` array — the same field name and per-item shape as set_node_visibility, including the fillHex/fillOpacity aliases from gethopp#39 — and applies them all in a single round-trip. Items are independent: a failing node is reported in its own result entry and does not abort the rest, so a partially-stale node list still does useful work.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughChangesBatch solid-fill updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change adds a localized batch paint-update capability with no actionable merge-blocking risk remaining after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPTool
participant Plugin
participant FigmaNodes
Client->>MCPTool: submit set_solid_fills items
MCPTool->>Plugin: forward validated batch
Plugin->>FigmaNodes: apply fill or stroke per item
FigmaNodes-->>Plugin: return item result
Plugin-->>Client: return batch results
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hey @julioservan . I am now reading the PR and the batch shape itself is right. 1/ The alias handling belongs in the schema, not the plugin
This PR uses const hex = typeof item.hex === "string" ? item.hex : item.fillHex;
const rawOpacity =
typeof item.opacity === "number" ? item.opacity : item.fillOpacity;The alias rule now exists twice, in two layers (server + plugin), and the two sibling handlers disagree about how many spellings they accept. Whenever the next alias gets added to Switching the item schema to a 2/ Two tools one letter apart, both described as "Replace the fill (or stroke)" will lead to models picking the wrong one fairly often, and both mistakes are silent ones. Reach for the singular in a loop and you get exactly the latency this PR exists to remove, with nothing in the response to hint at it. This is the first of three PRs setting the pattern (as you mentioned), so I'd rather decide it now than inherit it. Two options worth weighing: let 3/ Cleanup The patch has quite a few random blank lines in it |
What
Adds
set_solid_fills, which applies solid fills or strokes to many nodes in a single round-trip.Why
Theming a component's variants costs one WebSocket round-trip per node today. That is fine for three layers and unworkable for the hundred a real theming pass touches — the latency is entirely in the round-trips, not the local work.
Shape
It follows the repo's existing multi-target idiom rather than inventing one: an
itemsarray, same asset_node_visibilityandsave_screenshots. Each item mirrorsset_solid_fillfield for field, including thefillHex/fillOpacityaliases added in #39 — so a caller can lift a single-node call straight intoitemswithout renaming anything.Items are independent. A bad
nodeIdfails only its own entry and lands in that entry'serror; the rest still apply. The response is{ results: [...] }, matchingset_node_visibility.Notes
set_solid_fillsis registered inEDIT_REQUEST_TYPES, so it returns the usual clear error in Dev Mode.Testing
tsc --noEmitis clean on the server. The plugin's 5 pre-existing type errors are unchanged (they reproduce onmainwithout this patch). Both packages build.This is the first of three stacked PRs adding a design-system write surface; the other two (variables/modes/bindings, and styles) build on this one and I'll open them once this lands — or sooner if you'd rather review them together.
Summary by CodeRabbit