Selection toolbar fill swatch lies about transparent fill (and offers no way to reset to transparent) #143
Labels
No labels
prio_critical
prio_low
type_bug
type_contact
type_issue
type_lead
type_question
type_story
type_task
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
lhumina_code/hero_whiteboard#143
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Bug
When a Shape object has
fill = 'transparent'(the rect shows only its stroke, no interior color), the selection toolbar's Fill color swatch is rendered as a solid#2b3035(dark gray). This is misleading on two levels: it suggests the shape has a dark gray fill when it actually has none, and once the user picks any palette color the shape is now filled with no way to set it back to transparent (the palette has no transparent entry).Reproduction
fill: 'transparent'.Root cause
crates/hero_whiteboard_ui/static/web/js/whiteboard/selection_toolbar.js:857substitutes a fake color before passing to the swatch: The transparent state is hidden from the swatch instead of represented._buildColorTriggerat line 524 has no special rendering branch for'transparent'— it just setsswatch.style.background = initial || '#000', which paints the swatch with whatever string was passed.DEFAULT_COLOR_PALETTE(line 26) has no transparent entry, so even if the swatch and trigger handled transparent correctly, the popover still does not let the user pick it.Fix scope
_buildColorTrigger, wheninitial === 'transparent'(or any value that resolves to transparent) render the swatch with a clear 'no fill' visual — typically a checkered background plus a thin diagonal red strike, the same convention Figma/Miro/Excalidraw use. Keep the same indicator logic inactiveColorSetterso swapping the value back to transparent re-renders the indicator._buildColorPopover, add a leading 'no fill' / transparent option that returns'transparent'to theonPickcallback. Render it with the same checkered + strike indicator so it is recognizable._renderShape, drop the fake-color substitution at line 857 and passbg.fill()(which may be'transparent') straight through to_colorTriggerWithTitle.fill-like value through_colorTriggerWithTitleand decide per-site whether transparent is a valid option there. At minimum: shape Fill (line 886) and frame Background (line 1018) should accept transparent. Sticky Note background, Document background, Text color, Stroke color, etc. should not — keep them as-is.Affected files
crates/hero_whiteboard_ui/static/web/js/whiteboard/selection_toolbar.js— the only file in scope.Acceptance criteria
fill: 'transparent') shows a 'no fill' swatch in the selection toolbar (checkered + strike), not a solid dark color.bg.fill('transparent')and the swatch re-renders to the 'no fill' indicator.bg.fill()returns whatever was last picked, including'transparent'.Implementation Spec for Issue #143
Objective
Make the Fill color swatch in the selection toolbar honestly represent a
transparentfill (using the standard checker + diagonal-strike "no fill" indicator) and give users a way to set fill back to transparent via a "No fill" entry at the top of the Fill popover. The new option must be opt-in per call site so Stroke / Text / Sticky color triggers are unchanged.Requirements
'transparent'and the call site has opted in, render the "no fill" visual instead ofbackground: transparent.'transparent'.activeColorSetterindirection must use the same indicator helper, so programmatic re-drives keep the swatch correct._renderShapeline 886) and Document Background (_renderDocument) opt in. Sticky color, Text color, Stroke color, Border color, Drawing stroke, Document text remain unchanged.'#2b3035'lie at_renderShapeline 857 and passbg.fill()straight through.bg.fill('transparent')already round-trips.Files to Modify/Create
crates/hero_whiteboard_ui/static/web/js/whiteboard/selection_toolbar.js— add_applySwatchVisualhelper, thread anallowTransparentoption through_colorTriggerWithTitle->_buildColorTrigger->_buildColorPopover, fix_renderShapefill handling, opt the two relevant call sites in.crates/hero_whiteboard_ui/static/web/css/selection_toolbar.css(or wherever the toolbar CSS lives) — add.is-no-fillstyling reusable by both the trigger swatch and a popover swatch.Implementation Plan
Step 1: Add the "no fill" indicator visual (CSS)
Files: toolbar CSS file
.is-no-fill(scoped under both.wb-pt-color-trigger .swatchand.wb-pt-color-swatch) that paints a small checkered background using twolinear-gradientlayers + a diagonal red strike using a thirdlinear-gradient. Keep the existing 1px border.Dependencies: none.
Step 2: Add
_applySwatchVisualhelper inselection_toolbar.jsFiles:
selection_toolbar.js_applySwatchVisual(swatchEl, value):value === 'transparent':swatchEl.classList.add('is-no-fill'), clearswatchEl.style.background.swatchEl.style.background = value || '#000'.Dependencies: Step 1.
Step 3: Thread
allowTransparentthrough trigger / popover buildersFiles:
selection_toolbar.js_buildColorPopover(currentValue, palette, onPick, allowTransparent): whenallowTransparent, prepend awb-pt-color-swatch is-no-fillbutton (title=No fill,aria-label=No fill); markedis-selectedwhencurrentValue === 'transparent'; click ->onPick('transparent')then close._buildColorTrigger(initial, onPick, palette, allowTransparent): replaceswatch.style.background = initial || '#000'with_applySwatchVisual(swatch, initial). Same in the click handler and inactiveColorSetter. PassallowTransparentto_buildColorPopover._colorTriggerWithTitle(initial, onPick, palette, tooltip, allowTransparent)and pass through. Defaultfalseso all existing call sites are byte-identical.Dependencies: Step 2.
Step 4: Fix
_renderShapeFill pickerFiles:
selection_toolbar.jsvar fill = bg ? bg.fill() : 'transparent';(no fake substitution).trueto the Fill_colorTriggerWithTitlecall.Dependencies: Step 3.
Step 5: Opt Document Background in
Files:
selection_toolbar.js_renderDocumentBackground-color call: appendtrueas the fifth argument._colorTriggerWithTitlecall sites (sticky, text, drawing stroke, kanban, etc.) keep the four-arg form.Dependencies: Step 3.
Step 6: Manual verification
fill: 'transparent'): Fill swatch renders "no fill" indicator.bg.fill('transparent')set, swatch updates viaactiveColorSetterpath.Acceptance Criteria
#2b3035.bg.fill('transparent')and the trigger swatch re-renders via_applySwatchVisual.activeColorSetterprogrammatic updates correctly toggle between solid and no-fill visuals.Notes
DEFAULT_COLOR_PALETTEstays untouched — the "No fill" cell is injected at popover-build time only whenallowTransparentis true.allowTransparentboolean keeps the Stroke/Text/Sticky paths untouched and makes future additions a one-flag change.<input type="color">row in the popover stays. It cannot represent transparency, so picking from it always commits a solid color — that is the expected fallback.Test Results
cargo test --workspace --lib— PASScargo fmt --check— PASScargo clippy --workspace --all-targets -- -D warnings— PASSNotes
This fix is JS+CSS only (
crates/hero_whiteboard_ui/static/web/js/whiteboard/selection_toolbar.jsandcrates/hero_whiteboard_ui/static/web/css/selection_toolbar.css). The Rust workspace tests do not exercise this code path; they confirm the workspace still builds, formats, and lints cleanly. Manual UI verification is required for the user-facing behavior (no-fill indicator on transparent shapes; "No fill" entry in the Fill and Document Background popovers; other color triggers unaffected).Implementation Summary
Fix for the misleading Fill swatch on transparent shapes and the inability to set fill back to transparent.
Changes
crates/hero_whiteboard_ui/static/web/css/selection_toolbar.css— added.is-no-fillrule (scoped under both.wb-pt-color-trigger .swatchand.wb-pt-color-swatch) that paints a checkered background plus a diagonal red strike, sized in percentages so it scales for both the 18x18 trigger swatch and the 24x24 popover swatch.crates/hero_whiteboard_ui/static/web/js/whiteboard/selection_toolbar.js:_applySwatchVisual(swatchEl, value)helper — single source of truth for color-string vs no-fill rendering. Used everywhere the trigger swatch updates (initial render, popover pick,activeColorSetter).allowTransparentflag threaded through_colorTriggerWithTitle->_buildColorTrigger->_buildColorPopover. When set, the popover prepends a "No fill" cell that picks'transparent'. Default is falsy so all existing 4-arg call sites are byte-identical._renderShape: dropped the'#2b3035'substitution at line 857 — fill value now flows frombg.fill()straight through. Fill picker call site opts in withallowTransparent: true._renderDocument: Background-color picker opts in. Border color and Document text color untouched.DEFAULT_COLOR_PALETTEis unchanged — the "No fill" cell is injected per call site at popover-build time, not added to the palette.Test Results
cargo test --workspace --lib— PASScargo fmt --check— PASScargo clippy --workspace --all-targets -- -D warnings— PASSManual Verification Required
The fix is JS+CSS only. Please verify: