fix(studio): make flat inspector's Stroke width free of style-name typing#2532
Conversation
…ping Stroke width committed border-width and border-style together from one free-text field, so setting a style meant typing an exact CSS keyword (e.g. "dashed") with no indication of which ones were valid — the row also duplicated the discoverable Stroke style select directly below it. Stroke width now only commits border-width; style changes go exclusively through the existing select.
miga-heygen
left a comment
miga-heygen
left a comment
There was a problem hiding this comment.
Review — #2532: make Stroke width free of style-name typing
Good simplification. The combined "1px solid" free-text field was asking users to type CSS keywords with no discoverability. Now: width-only FlatRow (numeric) + FlatSelectRow for style (dropdown with valid options).
Verified: formatStrokeSummary/parseStrokeSummary + their test file cleanly removed. buildStrokeWidthStyleUpdates handles the from-zero→solid edge case (width typed in from 0 needs a style to render). Tests updated: combined commit → width-only commit. fallow-ignore-next-line complexity removed (function is now simpler).
SSOT: stroke style is now ONLY set through the select dropdown, never parsed from free text. Width is ONLY set through the width field. Clean separation.
Code: clean.
Review by Miga
james-russo-rames-d-jusso
left a comment
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at d5a259a3. Applied removed-code-had-side-effect lens to the net-removal of formatStrokeSummary/parseStrokeSummary, traced buildStrokeStyleUpdates migration to FlatSelectRow.onChange, and verified the color affordance survives elsewhere in the section.
Blockers
(none)
Concerns
(none)
Nits
(none)
Green notes
🟢 Removed helpers were fully self-contained — buildStrokeStyleUpdates migrated cleanly to the FlatSelectRow onChange. formatStrokeSummary/parseStrokeSummary were only used by the combined-stroke FlatRow's onCommit. Their job (parse "Xpx style" text and dispatch both border-width and border-style writes) is now split by construction: the width FlatRow's onCommit calls buildStrokeWidthStyleUpdates(normalizedWidth, borderStyleValue) (width + style-preserving), and the separate FlatSelectRow label="Stroke style" onChange calls buildStrokeStyleUpdates(next, formatPxMetricValue(borderWidthValue)) (style + width-preserving). Both underlying producers are preserved; only the parser+combined-string layer went away.
🟢 Style-token validation invariant is now enforced by construction. The removed if (!STROKE_STYLE_OPTIONS.includes(parsed.style)) return; guard was needed because the user typed style as free text ("2px dashed") which could produce a bogus token. With the style now behind FlatSelectRow options={STROKE_STYLE_OPTIONS}, the user can only pick from the whitelist — the guard's job is done by the dropdown. The old "rejects bogus style" test is correctly gone; there's no code path that could produce an invalid token now. Removal of the check is safe because the invariant that gated it is stricter, not looser.
🟢 Color swatch removal is not a UX regression — the color is still shown as its own control. Grepping borderColorValue on the tip shows the same file (propertyPanelFlatStyleSections.tsx:213) renders a full label="Stroke color" FlatRow immediately after the width/style pair. The swatch suffix was redundant with the color-picker row below it. Cleaner surface.
🟢 resolveValueTier narrowing is correct for the width-only path. Old tier keyed on styles["border-width"] != null || styles["border-style"] != null ? summary : undefined vs formatStrokeSummary(0, "none") — a composite signal that mixed two properties. New tier resolveValueTier(styles["border-width"], "0px") keys on width alone, matching the input's semantics. The Stroke-style tier is separately computed at resolveValueTier(styles["border-style"], "none") for the select row.
🟢 From-zero default-to-solid is exercised in the new test. "defaults a from-zero width commit to solid, since a width with no style renders no visible border" (test lines around 195-205) locks in the "typing 3px into a 0px/none-style stroke should produce a visible border" behavior via buildStrokeWidthStyleUpdates. The comment in the onCommit explicitly documents that the current-style-preserving behavior only defaults to solid on none/hidden — dashed/dotted authored styles are preserved. Nice invariant to have pinned.
🟢 fallow-ignore-next-line complexity suppression correctly removed. The old combined-stroke path warranted the complexity waiver; the new split shape is simpler enough that the linter is happy again.
Verdict framing
Clean refactor with equal-or-better test coverage. Every consumer of the removed helpers has a replacement path. LGTM from my side.
3 participants
Summary
border-widthandborder-styletogether from one free-text field, so setting a style required typing an exact CSS keyword (dashed,groove, etc.) with no indication of which ones were valid — worse, the row duplicated the "Stroke style" select already sitting right below it.border-width(auto-defaultingborder-styletosolidwhen going from a zero width againstnone/hidden, via the existingbuildStrokeWidthStyleUpdateshelper). Style changes go exclusively through the discoverable select.formatStrokeSummary/parseStrokeSummaryhelpers and their dedicated test file.Test plan
bunx vitest run src/components/editor/propertyPanelFlatStyleSections.test.tsx(37/37)bunx vitest run(231 files / 2643 passed)tsc --noEmit,oxlint,oxfmt --checkclean on touched files