Skip to content

Commit 7022e2e

Browse files
authored
fix(workflow): stop the coloured knob leaving a barb at each shoulder tip (#6632)
The connection knob is a recolour of one stretch of the card outline, so its path has to be the outline's own path. Two things pulled it off: Its span was cut at the exact point the bulge falls under the visibility threshold, which is not one of the points the silhouette sampled — so the knob sat on a grid of its own. And a span clamped its first and last control points to the bare perimeter tangent, where the silhouette derives every control point from the samples either side of it, so those two segments bowed differently from the curve they were painted over. The knob was left still flat where the silhouette had already begun its descent, and the uncovered sliver of dark stroke read as a small spur poking off the shoulder. It is clearest on the Error output's outer shoulder, against the card's bottom-right corner. Measure the span against the interval the silhouette resamples the bulge over rather than in whole pixels, and sample a step wide on each side before trimming back, so every emitted segment has the neighbours the silhouette had. The knob's commands now come out identical to the silhouette's, which the tests pin — including for merged intervals and odd tab lengths, where measuring in whole pixels would still have landed half a step off. The painted footprint is unchanged.
1 parent c411b1d commit 7022e2e

2 files changed

Lines changed: 109 additions & 22 deletions

File tree

packages/workflow-renderer/src/workflow-block/workflow-block-border-mount.test.tsx

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,62 @@ describe('WorkflowBlockBorder mount', () => {
265265
expect(path?.getAttribute('d')?.length ?? 0).toBeGreaterThan(0)
266266
})
267267

268+
/**
269+
* The knob is a recolour of one stretch of the outline, so its commands have
270+
* to BE the outline's commands. When the two were generated off different
271+
* sample grids they disagreed by a fraction of a pixel at the shoulder tip,
272+
* and the sliver of dark stroke the knob failed to cover read as a barb
273+
* hanging off it.
274+
*
275+
* Only holds where a knob's own bulge is the tallest thing under it: a knob
276+
* is painted from its own feature alone, while the silhouette takes the max
277+
* over all of them, so two bulges tall enough to overlap genuinely part
278+
* company. Every port layout the editor builds keeps them clear of one
279+
* another.
280+
*/
281+
const expectKnobsOnSilhouette = (host: HTMLElement) => {
282+
const silhouette = host.querySelector('svg > path')?.getAttribute('d') ?? ''
283+
const knobs = Array.from(host.querySelectorAll('svg > g[clip-path] path'))
284+
expect(knobs.length).toBeGreaterThan(0)
285+
286+
for (const knob of knobs) {
287+
const commands = (knob.getAttribute('d') ?? '').match(/C[^MLAC]+/g) ?? []
288+
expect(commands.length).toBeGreaterThan(0)
289+
for (const command of commands) {
290+
expect(silhouette).toContain(command.trim())
291+
}
292+
}
293+
}
294+
295+
it('paints every coloured knob on the silhouette’s own curve', () => {
296+
const { host } = mount(
297+
<div style={{ width: 250, height: 136 }}>
298+
<WorkflowBlockBorder ports={ports} hasRing={false} ringStyles='' height={136} />
299+
</div>
300+
)
301+
expectKnobsOnSilhouette(host)
302+
})
303+
304+
it('keeps knobs on the curve when their resampled intervals merge', () => {
305+
/* Two coloured ports close enough that `relativeIntervals` merges them
306+
resample as one stretch, on a grid neither port's own bounds predict.
307+
The odd tab length is the other half of the same trap: a knob measured
308+
in whole pixels rather than against the interval it sits in lands half a
309+
step off whenever the bulge does not divide evenly. */
310+
const crowdedPorts: WorkflowBorderPort[] = [
311+
{ id: 'target', side: 'left', position: 'center', plateau: 33 },
312+
{ id: 'row-a', side: 'right', position: 60, plateau: 24, color: 'var(--brand-accent)' },
313+
{ id: 'row-b', side: 'right', position: 87.5, plateau: 24, color: 'var(--text-error)' },
314+
{ id: 'row-c', side: 'right', position: 140, plateau: 23, color: 'var(--warning)' },
315+
]
316+
const { host } = mount(
317+
<div style={{ width: 250, height: 260 }}>
318+
<WorkflowBlockBorder ports={crowdedPorts} hasRing={false} ringStyles='' height={260} />
319+
</div>
320+
)
321+
expectKnobsOnSilhouette(host)
322+
})
323+
268324
it('paints a tall selector card across floating-point segment seams', () => {
269325
const selectorPorts: WorkflowBorderPort[] = [
270326
{

packages/workflow-renderer/src/workflow-block/workflow-block-border.tsx

Lines changed: 53 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,18 @@ const findQuietStart = (intervals: ActiveInterval[], perimeterLength: number) =>
602602
return largestGap > 0 ? start : 0
603603
}
604604

605+
/** Flat run resampled either side of a bulge, so its tail rejoins the edge. */
606+
const BULGE_INTERVAL_SLACK_PX = 4
607+
608+
/**
609+
* How far either side of a bulge's centre the outline is resampled — wider than
610+
* the bulge itself, so the curve has flat perimeter to settle onto. It also
611+
* fixes the sample grid a knob has to land on, which is why `visibleBulgeHalf`
612+
* measures against it.
613+
*/
614+
const bulgeIntervalHalf = (plateau: number, shoulder: number) =>
615+
plateau / 2 + shoulder + BULGE_INTERVAL_SLACK_PX
616+
605617
const relativeIntervals = (features: BulgeFeature[], startS: number, perimeterLength: number) => {
606618
const intervals = features
607619
.filter(
@@ -614,7 +626,7 @@ const relativeIntervals = (features: BulgeFeature[], startS: number, perimeterLe
614626
)
615627
.map((feature) => {
616628
const center = modulo(feature.center - startS, perimeterLength)
617-
const half = feature.plateau / 2 + feature.shoulder + 4
629+
const half = bulgeIntervalHalf(feature.plateau, feature.shoulder)
618630
return { start: center - half, end: center + half }
619631
})
620632
.filter((interval) => interval.end > 0 && interval.start < perimeterLength)
@@ -671,19 +683,32 @@ const displacementAt = (
671683
* Where a bulge stops being drawn, by inverting the shoulder's easing at the
672684
* visibility threshold. The mathematical footprint (`plateau/2 + shoulder`)
673685
* overshoots this, because the tail is cut off once it flattens out.
686+
*
687+
* Pulled back to the silhouette's own sample points. `relativeIntervals`
688+
* resamples a bulge over `bulgeIntervalHalf` either side of its centre, split
689+
* into whole steps of about `SAMPLE_SPACING_PX` — so the crossing itself falls
690+
* between two of them. A knob cut there sits on a grid of its own and drifts
691+
* off the curve it is recolouring; see `buildSpanPath` for what that costs.
692+
* Retreating to the last sample at or beyond the crossing keeps the knob on
693+
* the silhouette's points whatever the bulge measures.
674694
*/
675695
const visibleBulgeHalf = (plateau: number, shoulder: number, peak: number) => {
676696
const plateauHalf = plateau / 2
677-
if (peak <= BULGE_VISIBLE_THRESHOLD_PX || shoulder <= 0) return plateauHalf
678-
const target = 1 - BULGE_VISIBLE_THRESHOLD_PX / peak
679-
let low = 0
680-
let high = 1
681-
for (let step = 0; step < 24; step++) {
682-
const mid = (low + high) / 2
683-
if (smootherstep(mid) < target) low = mid
684-
else high = mid
697+
let crossing = plateauHalf
698+
if (peak > BULGE_VISIBLE_THRESHOLD_PX && shoulder > 0) {
699+
const target = 1 - BULGE_VISIBLE_THRESHOLD_PX / peak
700+
let low = 0
701+
let high = 1
702+
for (let step = 0; step < 24; step++) {
703+
const mid = (low + high) / 2
704+
if (smootherstep(mid) < target) low = mid
705+
else high = mid
706+
}
707+
crossing = plateauHalf + high * shoulder
685708
}
686-
return plateauHalf + high * shoulder
709+
const intervalHalf = bulgeIntervalHalf(plateau, shoulder)
710+
const step = (intervalHalf * 2) / Math.max(2, Math.ceil((intervalHalf * 2) / SAMPLE_SPACING_PX))
711+
return intervalHalf - Math.floor((intervalHalf - crossing) / step) * step
687712
}
688713

689714
const appendExactInterval = (
@@ -764,6 +789,7 @@ const appendActiveInterval = (
764789
`C${control1.x.toFixed(2)} ${control1.y.toFixed(2)} ${control2.x.toFixed(2)} ${control2.y.toFixed(2)} ${next.x.toFixed(2)} ${next.y.toFixed(2)}`
765790
)
766791
}
792+
return points
767793
}
768794

769795
/**
@@ -777,6 +803,16 @@ const appendActiveInterval = (
777803
* off its own knob, leaving a crescent of base colour showing inside it. Giving
778804
* the knob its own path removes the arc-length bookkeeping altogether: the
779805
* colour is drawn on the same points the silhouette was.
806+
*
807+
* Sampled a step wide on each side and then trimmed back to the span. Every
808+
* control point is derived from the samples either side of it, so a span that
809+
* stopped at its own ends would have to clamp its first and last to the bare
810+
* perimeter tangent — and those two segments would bow differently from the
811+
* outline they are painted over. The knob was then still flat where the
812+
* silhouette had begun its descent, and the uncovered dark stroke read as a
813+
* barb off the shoulder tip. Borrowing a sample beyond each end gives every
814+
* emitted segment the neighbours the silhouette had, so the two agree command
815+
* for command.
780816
*/
781817
const buildSpanPath = (
782818
geometry: PerimeterGeometry,
@@ -787,18 +823,13 @@ const buildSpanPath = (
787823
) => {
788824
const length = toS - fromS
789825
if (length <= 0) return ''
790-
const located = pointAtArcLength(geometry, fromS)
791-
const displacement = displacementAt(
792-
modulo(fromS, geometry.length),
793-
features,
794-
geometry.length,
795-
maximum
796-
)
797-
const originX = located.point.x + located.point.nx * displacement
798-
const originY = located.point.y + located.point.ny * displacement
799-
const commands = [`M${originX.toFixed(2)} ${originY.toFixed(2)}`]
800-
appendActiveInterval(commands, geometry, features, maximum, fromS, { start: 0, end: length })
801-
return commands.join(' ')
826+
const commands: string[] = []
827+
const points = appendActiveInterval(commands, geometry, features, maximum, fromS, {
828+
start: -SAMPLE_SPACING_PX,
829+
end: length + SAMPLE_SPACING_PX,
830+
})
831+
const origin = points[1]
832+
return [`M${origin.x.toFixed(2)} ${origin.y.toFixed(2)}`, ...commands.slice(1, -1)].join(' ')
802833
}
803834

804835
const buildPiecewisePath = (

0 commit comments

Comments
 (0)