Skip to content

[DS 2.0] Render the default canvas port at the designed 8px - #122

Open
librowski wants to merge 2 commits into
ds2-canvas-edgesfrom
ds2-canvas-ports
Open

[DS 2.0] Render the default canvas port at the designed 8px#122
librowski wants to merge 2 commits into
ds2-canvas-edgesfrom
ds2-canvas-ports

Conversation

@librowski

@librowski librowski commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Problem

The default port declares content 4px + border 2px per side with box-sizing: content-box, intending an 8px outer size. The base stylesheet of @xyflow/react 12.10.0 imposes min-width: 5px; min-height: 5px on handles; with content-box the minimum applies to the content, so the effective size was 9x9px. Hover (content 12px) was already correct at 16px.

Change

  • handle.module.css: min-width: 0; min-height: 0 on the base handle rule.
  • check-built-css.ts: the built base .react-flow__handle rule must carry the complete geometry (width/height, border, box-sizing, zero minimums), so a regression fails the build check; built-css-pitfalls.md documents the inherited-minimum trap.
  • Changeset: @workflowbuilder/ui patch.

Verification

Build, check:built-css, ui tests and stylelint pass. Manual browser pass in the demo (Simple Workflow, 30 handles, layout px via offsetWidth): default and connectionindicator 8x8, handle hover 16x16, node hover indicators 16x16, connectingfrom 8x8 with crosshair, top/bottom handle rules only reposition (size unchanged). jsdom cannot measure layout, so the automated part is the built-CSS contract.

Follow-up in this PR: target port while connecting

React Flow captures the pointer on the dragged handle, so the node under the cursor never gets :hover and its ports stayed at 8px during a connection while the source showed 16px. .connectingto now shares the .connectingfrom rule: the target grows to the 16px active port, other ports stay 8px, drop still creates the edge (verified in the demo with the built package). Design has Default and Hover port states only; design is asked to confirm that Hover is the intended look for the connecting target. Changeset: @workflowbuilder/ui patch.

xyflow's 5px minimum applies to the content box and expands the bordered port to 9px.
Reset both minimums so 4px content plus 2px borders remains 8px.
React Flow captures the pointer on the dragged handle, so the node under
the cursor never receives :hover and its ports stayed at the 8px default
during a connection. The connectingto state now shares the connectingfrom
rule, so the target shows the same 16px active port as the source.
@librowski
librowski marked this pull request as ready for review September 14, 2026 06:41
shrink the handle in exactly these states. */
:global(.connectingfrom) {
shrink the handle in exactly these states. The target port needs its
own rule: React Flow captures the pointer on the dragged handle, so

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things about this comment. It now runs five lines, and CLAUDE.md caps comments at three. And the pointer-capture explanation does not match xyflow: XYHandle.onPointerDown only registers document-level mousemove / mouseup listeners, there is no setPointerCapture anywhere in @xyflow/system.

The stronger reason is simpler: connectingto is assigned by proximity. getClosestHandle picks any handle within connectionRadius (20px by default), so the pointer often is not over the target at all and :hover cannot represent it.

Suggest keeping the original three-line box-sizing note and adding one line:

/* .connectingto is assigned by proximity (connectionRadius), so :hover cannot cover the target. */

own rule: React Flow captures the pointer on the dragged handle, so
:hover never reaches the node under the cursor while connecting. */
:global(.connectingfrom),
:global(.connectingto) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One semantic wrinkle: xyflow sets connectingto on whichever handle it snapped to, valid or not. In isValidHandle, toHandle is filled independently of isValid, and valid is a separate class on the handle. With this rule a target rejected by isValidConnection (a public WorkflowBuilder.Root prop) also grows to the active look, which reads as droppable while the connection line reports invalid.

If that is not intended, :global(.connectingto.valid) limits the grow to valid targets with a one-token change. If design wants snap feedback regardless of validity, a word here or in the changeset would make it a decision rather than an accident. Either way, could the design question also cover the invalid-target case?

root.walkAtRules('import', (atRule) => add(file, atRule, 'Built CSS import'));

root.walkRules((rule) => {
if (!rule.selectors.some((selector) => selector.endsWith(' .react-flow__handle'))) return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

endsWith(' .react-flow__handle') catches any rule that targets the bare handle, not just the geometry rule. Today only the base rule matches, but a later .some-state .react-flow__handle { opacity: .5 } would fail with "Incorrect built handle geometry" even though geometry is untouched, and repeating the six declarations to appease the check would be wrong. The next PRs in this stack add node-state rules, so this will bite soon.

Suggest narrowing to rules that declare width (or box-sizing: content-box) on that selector, plus asserting that at least one such rule exists so a selector rename cannot silently disable the check.

Smaller note: esbuild already reshapes rules (the two hover rules are merged into one selector list in the SDK dist), so exact-string matching of border: var(...) solid var(...) against minified output is a soft spot. Fine for now, worth knowing.

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