Skip to content

fix: address review follow-ups on the canvas stack - #150

Merged
librowski merged 2 commits into
ds2-tokens-refresh-0915from
ds2-review-follow-ups
Sep 16, 2026
Merged

librowski merged 2 commits into
ds2-tokens-refresh-0915from
ds2-review-follow-ups

Conversation

@librowski

@librowski librowski commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-ups from the second review round on the canvas stack (#124, #126, #136, #139, #140), plus the code side of the design answers from 16.09. The reviewed PRs are approved, so the fixes land here on top of #149.

Review follow-ups (commit 1)

Persist without measured (#124)

  • getNodesWithoutDynamicValues also strips the runtime dragging flag; the bundled Demo and AI Studio templates no longer carry dragging: false.
  • getStoreDataForIntegration gets two boundary tests; changeset mentions dragging.

Node text ellipsis (#126)

  • Docs and changeset state the contain: inline-size constraint (definite inline size on an ancestor; collapses inside inline-flex or fit-content).

Node body spacing (#136)

  • Width decision log rewritten with the current derivation and a ## Status section, no external tracker reference; tooltip changeset reworded.

Menu item selected (#139)

  • Menu.RadioGroup wrapper gets .group (column flex, --wb-public-list-box-gap), measured 4px between radio items; menu.spec.tsx and node-panel.spec.tsx unmount the React root after each test.

Palette node states (#140)

  • The custom node guide and the Demo multi-port template forward disabled to NodePanel.Root, NodeIcon and NodeDescription; node-panel.mdx lists disabled?: boolean; changeset written around WorkflowNodeTemplateProps.disabled with a migration note.

Design answers of 16.09 (commit 2)

  • Rows fill the section. In the horizontal layout Decision branches and AI tools take the section width like the design's Node / Row (width: 100%, min-width: 0, no cap); 205px at 241 is the reference value (design draws 209 because a Figma stroke takes no layout space). The derived cap and the Decision min-width: max-content rule now apply to the DOWN layout only, which has no design master. Branch rows and tool rows are spaced with --wb-ds-canvas-node-body-gap (8px; was 4px hard-coded in both places). Decision log updated, the connectable-item-design-width follow-up is closed.
  • Self-loop apex at 48px above the node's top edge, independent of node height and port position. SELF_CONNECTING_EDGE_LABEL_OFFSET is 48 and measured from the top edge, as its JSDoc always said; getSelfLoopHeight is exported. Measured in Demo: 48px in the RIGHT layout; 44px in DOWN, where xyflow anchors the bottom port on its outer edge.
  • DS Tooltip decided for 3.1: (follow-up: node-text-ds-tooltip) markers on the title attributes in NodeDescription and ConnectableItem.
  • Demo multi-port template forwards disabled to NodeIcon and NodeDescription (review remark on commit 1).

Checks

  • pnpm -F @workflowbuilder/ui build (includes check:built-css), vitest ui 74 and sdk 219 passing, stylelint ui/sdk clean, typecheck ui/sdk/demo/ai-studio clean.
  • Demo measurements (seed with long labels, RIGHT): Decision 241px, rows 205px with truncation, row gap 8px, tool gap 8px, loop apex 48px. DOWN unchanged (525px, capped row 205px).

Comment thread apps/demo/src/app/components/multi-port-node/multi-port-node-template.tsx Outdated
@librowski
librowski force-pushed the ds2-review-follow-ups branch from a2febe0 to f925eae Compare September 16, 2026 11:37
- persist: strip the runtime dragging flag alongside measured and selected,
  drop dragging from the bundled templates, cover getStoreDataForIntegration
- node description: document the contain: inline-size constraint
- connectable item: rewrite the width decision log with the current numbers
  and a status section; reword the tooltip changeset
- menu: lay out the RadioGroup wrapper as a column so the list-box gap applies
- ui specs: unmount the React root after each test
- palette: forward disabled in the custom template guide and the demo
  multi-port template, document it on NodePanel.Root, reword the changeset
- rows: in the horizontal layout Decision branches and AI tools fill the
  section width like the design's Node / Row; the derived cap and the
  Decision max-content rule now apply to the DOWN layout only; branch and
  tool rows are spaced with the node body gap
- self-loop: the apex sits 48px above the node's top edge regardless of
  node height and port position; getSelfLoopHeight is exported
- node text: follow-up markers for the DS Tooltip decided for 3.1
- demo: the multi-port template forwards disabled to icon and description
@librowski
librowski force-pushed the ds2-review-follow-ups branch from f925eae to 9674d72 Compare September 16, 2026 11:47
@librowski
librowski merged commit b8c26e5 into design-system-2.0 Sep 16, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants