Skip to content

fix: connector_detached now skips fully dash-hidden connector shafts #3912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
miga-heygen wants to merge 2 commits into
base: main
Choose a base branch
from
+261 −144
Open

fix: connector_detached now skips fully dash-hidden connector shafts #3912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8921a8
fix(cli): connector_detached now skips dash-hidden connector shafts
miga-heygen Sep 12, 2026
58aefbb
fix(cli): connector dash gate reads the whole stroke-dasharray; one o…
miga-heygen Sep 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
323 changes: 193 additions & 130 deletions packages/cli/src/commands/layout-audit.browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1259,7 +1259,10 @@
);
}

/** Raw `d`-space endpoints (no CTM) — the mapping authors use when they paste screen coords into `d`. */
/**
* Raw `d`-space endpoints (no CTM) — the mapping authors use when they paste screen coords into
* `d` — plus the path length they were sampled from, so callers never re-query the geometry.
*/
function pathUserEndpoints(path) {
if (typeof path.getTotalLength !== "function" || typeof path.getPointAtLength !== "function") {
return null;
Expand All @@ -1273,7 +1276,7 @@
if (!Number.isFinite(total) || total <= 0) return null;
const start = path.getPointAtLength(0);
const end = path.getPointAtLength(total);
return { start: { x: start.x, y: start.y }, end: { x: end.x, y: end.y } };
return { start: { x: start.x, y: start.y }, end: { x: end.x, y: end.y }, total };
}

// Screen endpoints via getScreenCTM (viewBox, preserveAspectRatio, group transforms).
Expand Down Expand Up @@ -1325,86 +1328,110 @@
return { compact, painted };
}

// Flag only the documented bug: rendered endpoints miss, but user-space-as-screen would attach.
function connectorDetachmentIssues(root, rootRect, time) {
const issues = [];
let anchors = null;
// Attach near-miss tolerance (screen px). Separate from the closed-glyph chord floor.
const threshold = Math.max(32, Math.min(rootRect.width, rootRect.height) * 0.02);
const MIN_CONNECTOR_CHORD_PX = 8;
// Attach near-miss tolerance (screen px), shared by both connector findings. Separate from
// the closed-glyph chord floor.
function connectorAttachThreshold(rootRect) {
return Math.max(32, Math.min(rootRect.width, rootRect.height) * 0.02);
}

/**
* The one owner of connector enumeration. Both connector findings (`connector_detached`,
* `connector_orphan`) consume this so a change to what counts as a connector lands in both.
*
* A candidate is a `<path>` inside a visible `<svg>` with connector intent (marker or name),
* outside decoration-only containers, whose endpoints resolve in both user and screen space.
* Dash-hidden shafts (see `shaftDashHidden`) are filtered out: nothing renders, so no finding
* about where it renders can apply. `painted` (display/visibility/opacity, see
* `shaftIsPainted`) is reported rather than filtered — `connector_orphan` is defined on a
* visible shaft and skips unpainted ones, `connector_detached` does not yet gate on it.
* `chord` is the rendered span in screen px: closed/glyph paths collapse to one point, so
* callers threshold it in px, not user units.
*/
function* connectorShafts(root) {
for (const svg of Array.from(root.querySelectorAll("svg"))) {
if (!isVisibleElement(svg)) continue;
for (const path of Array.from(svg.querySelectorAll("path"))) {
if (path.closest(CONNECTOR_SKIP_CONTAINERS)) continue;
if (!isConnectorPath(svg, path)) continue;
const user = pathUserEndpoints(path);
if (!user || shaftDashHidden(path, user.total)) continue;
const rendered = pathScreenEndpoints(svg, path, user);
if (!user || !rendered) continue;
// Closed/glyph paths collapse to one point — compare in screen px (not user units).
const renderedChord = Math.hypot(
if (!rendered) continue;
const chord = Math.hypot(
rendered.end.x - rendered.start.x,
rendered.end.y - rendered.start.y,
);
if (renderedChord < MIN_CONNECTOR_CHORD_PX) continue;
if (anchors === null) anchors = connectorAnchorRects(root, rootRect);
if (anchors.compact.length < 2) return issues;
// Stable DOM identity across painted (inside) and compact (near-miss) tiers.
const attachmentKey = (point) => {
for (const anchor of anchors.painted) {
if (!anchor.element.contains(svg) && distanceToRect(point, anchor.rect) === 0) {
return anchor.element;
}
}
for (const anchor of anchors.compact) {
if (distanceToRect(point, anchor.rect) <= threshold) return anchor.element;
}
return null;
};
const attached = (point) => attachmentKey(point) !== null;
// Half-attached as drawn is allowed; only full render-miss proceeds.
if (attached(rendered.start) || attached(rendered.end)) continue;
// Paste-into-`d` bug: both raw endpoints land on distinct anchors as screen pixels.
const userStartKey = attachmentKey(user.start);
const userEndKey = attachmentKey(user.end);
const pasteBug = Boolean(userStartKey && userEndKey && userStartKey !== userEndKey);
// Guessed marked shaft: both frames miss. Same-anchor grazes attach in user-space
// and must stay skipped. Name-only decorative flow/arrow paths stay skipped.
// 80px keeps short marker glyphs (chevrons, tips) out.
const markedMiss =
renderedChord >= 80 &&
!userStartKey &&
!userEndKey &&
(path.hasAttribute("marker-start") || path.hasAttribute("marker-end"));
if (!pasteBug && !markedMiss) continue;
const gap = Math.round(
Math.min(
Math.min(...anchors.compact.map((a) => distanceToRect(rendered.start, a.rect))),
Math.min(...anchors.compact.map((a) => distanceToRect(rendered.end, a.rect))),
),
);
issues.push({
code: "connector_detached",
severity: "warning",
time,
selector: selectorFor(path),
containerSelector: selectorFor(svg),
message: pasteBug
? `Connector path endpoints render ${gap}px from the nearest anchorable element, but the path's user-space coordinates would attach if read as screen pixels — screen/viewport numbers were likely written into SVG \`d\` without inverting the CTM.`
: `Connector path endpoints render ${gap}px from the nearest anchorable element — a marked shaft that meets no node.`,
rect: toRect({
left: Math.min(rendered.start.x, rendered.end.x),
top: Math.min(rendered.start.y, rendered.end.y),
right: Math.max(rendered.start.x, rendered.end.x),
bottom: Math.max(rendered.start.y, rendered.end.y),
width: Math.abs(rendered.end.x - rendered.start.x),
height: Math.abs(rendered.end.y - rendered.start.y),
}),
fixHint: pasteBug
? "Convert measured screen coordinates into the SVG's user space (subtract the SVG rect / invert getScreenCTM) before writing path `d`, and keep the SVG a direct child of the stage."
: "Measure the settled node boxes and write `d` in the SVG's user space (invert getScreenCTM), or grow a layout-owned shaft from the source node.",
});
yield { svg, path, user, rendered, chord, painted: shaftIsPainted(path) };
}
}
}

// Flag only the documented bug: rendered endpoints miss, but user-space-as-screen would attach.
function connectorDetachmentIssues(root, rootRect, time) {
const issues = [];
let anchors = null;
const threshold = connectorAttachThreshold(rootRect);
const MIN_CONNECTOR_CHORD_PX = 8;
for (const { svg, path, user, rendered, chord } of connectorShafts(root)) {
if (chord < MIN_CONNECTOR_CHORD_PX) continue;
if (anchors === null) anchors = connectorAnchorRects(root, rootRect);
if (anchors.compact.length < 2) return issues;
// Stable DOM identity across painted (inside) and compact (near-miss) tiers.
const attachmentKey = (point) => {
for (const anchor of anchors.painted) {
if (!anchor.element.contains(svg) && distanceToRect(point, anchor.rect) === 0) {
return anchor.element;
}
}
for (const anchor of anchors.compact) {
if (distanceToRect(point, anchor.rect) <= threshold) return anchor.element;
}
return null;
};
const attached = (point) => attachmentKey(point) !== null;
// Half-attached as drawn is allowed; only full render-miss proceeds.
if (attached(rendered.start) || attached(rendered.end)) continue;
// Paste-into-`d` bug: both raw endpoints land on distinct anchors as screen pixels.
const userStartKey = attachmentKey(user.start);
const userEndKey = attachmentKey(user.end);
const pasteBug = Boolean(userStartKey && userEndKey && userStartKey !== userEndKey);
// Guessed marked shaft: both frames miss. Same-anchor grazes attach in user-space
// and must stay skipped. Name-only decorative flow/arrow paths stay skipped.
// 80px keeps short marker glyphs (chevrons, tips) out.
const markedMiss =
chord >= 80 &&
!userStartKey &&
!userEndKey &&
(path.hasAttribute("marker-start") || path.hasAttribute("marker-end"));
if (!pasteBug && !markedMiss) continue;
const gap = Math.round(
Math.min(
Math.min(...anchors.compact.map((a) => distanceToRect(rendered.start, a.rect))),
Math.min(...anchors.compact.map((a) => distanceToRect(rendered.end, a.rect))),
),
);
issues.push({
code: "connector_detached",
severity: "warning",
time,
selector: selectorFor(path),
containerSelector: selectorFor(svg),
message: pasteBug
? `Connector path endpoints render ${gap}px from the nearest anchorable element, but the path's user-space coordinates would attach if read as screen pixels — screen/viewport numbers were likely written into SVG \`d\` without inverting the CTM.`
: `Connector path endpoints render ${gap}px from the nearest anchorable element — a marked shaft that meets no node.`,
rect: toRect({
left: Math.min(rendered.start.x, rendered.end.x),
top: Math.min(rendered.start.y, rendered.end.y),
right: Math.max(rendered.start.x, rendered.end.x),
bottom: Math.max(rendered.start.y, rendered.end.y),
width: Math.abs(rendered.end.x - rendered.start.x),
height: Math.abs(rendered.end.y - rendered.start.y),
}),
fixHint: pasteBug
? "Convert measured screen coordinates into the SVG's user space (subtract the SVG rect / invert getScreenCTM) before writing path `d`, and keep the SVG a direct child of the stage."
: "Measure the settled node boxes and write `d` in the SVG's user space (invert getScreenCTM), or grow a layout-owned shaft from the source node.",
});
}
return issues;
}

Expand All @@ -1421,19 +1448,67 @@
return opacityChain(path) >= 0.2;
}

function shaftDashHidden(path) {
if (typeof path.getTotalLength !== "function") return false;
let total;
try {
total = path.getTotalLength();
} catch {
return false;
}
if (!Number.isFinite(total) || total <= 0) return false;
/**
* True when the stroke's dash pattern currently paints nothing: the window of the pattern the
* shaft shows, `[dashoffset, dashoffset + length]`, sits inside a single gap — a draw-on
* entrance (`dasharray >= length; dashoffset >= length`) not yet advanced. Up to 10% of the
* length may still poke into a neighbouring dash (the tail of a nearly finished tween). Any
* whole dash inside the window means the stroke paints, so dashed patterns, `4 0`, a bare `0`
* (renders solid) and `none` are painted. A zero-length dash paints only as a line cap: `0 4`
* is a dotted line under `stroke-linecap: round | square` and invisible under the default
* `butt` (the state a finished draw-off tween leaves behind: `0px, 999999px`). Caps that a
* non-zero dash would add at the window edges are ignored.
*/
function shaftDashHidden(path, total) {
const style = getComputedStyle(path);
const offset = Number.parseFloat(style.strokeDashoffset || "0");
const dash = Number.parseFloat(String(style.strokeDasharray || "").split(/[\s,]+/)[0] || "0");
return offset >= total * 0.9 && dash >= total * 0.9;
const dashes = dashArrayLengths(style.strokeDasharray, path);
if (dashes === null) return false;
const dotsPaint = (style.strokeLinecap || "butt") !== "butt";
const dashPaints = (index) => index % 2 === 0 && (dashes[index] > 0 || dotsPaint);
if (!dashes.some((_, index) => dashPaints(index))) return true; // only butt-capped dots
const period = dashes.reduce((sum, length) => sum + length, 0);
if (total > period) return false; // a full period of dash paints — call it visible
const offset = dashLength(style.strokeDashoffset, path); // unparseable reads as 0
const start = Number.isFinite(offset) ? ((offset % period) + period) % period : 0;
const end = start + total;
let painted = 0;
let segmentStart = 0;
// Two periods cover any window that starts inside the first.
for (let i = 0; i < dashes.length * 2; i++) {
const segmentEnd = segmentStart + dashes[i % dashes.length];
if (dashPaints(i % dashes.length)) {
if (segmentStart >= start && segmentEnd <= end) return false;
painted += Math.max(0, Math.min(segmentEnd, end) - Math.max(segmentStart, start));
}
segmentStart = segmentEnd;
}
return painted <= total * 0.1;
}

// Computed `stroke-dasharray` as an even-length list of user-unit lengths, or null when the
// stroke is solid: `none`, an all-zero list, or any negative/unparseable entry (which the spec
// renders as `none`). Odd lists repeat, per spec.
function dashArrayLengths(value, path) {
const text = String(value || "none").trim();
if (text === "none") return null;
const lengths = text.split(/[\s,]+/).map((token) => dashLength(token, path));
if (lengths.some((length) => !Number.isFinite(length) || length < 0)) return null;
if (lengths.every((length) => length === 0)) return null;
return lengths.length % 2 === 0 ? lengths : lengths.concat(lengths);
}

// One dash length in user units. Computed lengths are already px; a percentage is relative to
// the normalised diagonal of the owning SVG viewport (viewBox when set, else the layout box).
function dashLength(token, path) {
const text = String(token).trim();
const value = Number.parseFloat(text);
if (!Number.isFinite(value) || !text.endsWith("%")) return value;
const svg = path.ownerSVGElement;
if (!svg) return NaN;
const box = svg.viewBox && svg.viewBox.baseVal;
const { width, height } =
box && box.width > 0 && box.height > 0 ? box : svg.getBoundingClientRect();
return (value / 100) * (Math.hypot(width, height) / Math.SQRT2);
}

function connectorEndpointCandidates(root, rootRect) {
Expand All @@ -1456,57 +1531,45 @@
function connectorOrphanIssues(root, rootRect, time) {
const issues = [];
let candidates = null;
const threshold = Math.max(32, Math.min(rootRect.width, rootRect.height) * 0.02);
for (const svg of Array.from(root.querySelectorAll("svg"))) {
if (!isVisibleElement(svg)) continue;
for (const path of Array.from(svg.querySelectorAll("path"))) {
if (path.closest(CONNECTOR_SKIP_CONTAINERS)) continue;
if (!isConnectorPath(svg, path)) continue;
if (!shaftIsPainted(path) || shaftDashHidden(path)) continue;
const user = pathUserEndpoints(path);
const rendered = pathScreenEndpoints(svg, path, user);
if (!user || !rendered) continue;
const renderedChord = Math.hypot(
rendered.end.x - rendered.start.x,
rendered.end.y - rendered.start.y,
);
if (renderedChord < 80) continue;
if (candidates === null) candidates = connectorEndpointCandidates(root, rootRect);
const dark = [];
for (const point of [rendered.start, rendered.end]) {
let best = null;
let attached = false;
for (const candidate of candidates) {
const gap = distanceToRect(point, candidate.rect);
if (gap > threshold) continue;
if (isVisibleElement(candidate.element)) {
attached = true;
break;
}
if (best === null || gap < best.gap) best = { gap, candidate };
const threshold = connectorAttachThreshold(rootRect);
for (const { svg, path, rendered, chord, painted } of connectorShafts(root)) {
if (!painted) continue;
if (chord < 80) continue;
if (candidates === null) candidates = connectorEndpointCandidates(root, rootRect);
const dark = [];
for (const point of [rendered.start, rendered.end]) {
let best = null;
let attached = false;
for (const candidate of candidates) {
const gap = distanceToRect(point, candidate.rect);
if (gap > threshold) continue;
if (isVisibleElement(candidate.element)) {
attached = true;
break;
}
if (!attached && best !== null) dark.push(best.candidate);
if (best === null || gap < best.gap) best = { gap, candidate };
}
if (dark.length === 0) continue;
issues.push({
code: "connector_orphan",
severity: "warning",
time,
selector: selectorFor(path),
containerSelector: selectorFor(svg),
message: `Connector shaft is visible while ${dark.length === 2 ? "both endpoints are" : `its endpoint ${selectorFor(dark[0].element)} is`} not on stage.`,
rect: toRect({
left: Math.min(rendered.start.x, rendered.end.x),
top: Math.min(rendered.start.y, rendered.end.y),
right: Math.max(rendered.start.x, rendered.end.x),
bottom: Math.max(rendered.start.y, rendered.end.y),
width: Math.abs(rendered.end.x - rendered.start.x),
height: Math.abs(rendered.end.y - rendered.start.y),
}),
fixHint:
"Show the shaft only after both ends are on, and hide it with the earlier exit. Do not give the line its own clock.",
});
if (!attached && best !== null) dark.push(best.candidate);
}
if (dark.length === 0) continue;
issues.push({
code: "connector_orphan",
severity: "warning",
time,
selector: selectorFor(path),
containerSelector: selectorFor(svg),
message: `Connector shaft is visible while ${dark.length === 2 ? "both endpoints are" : `its endpoint ${selectorFor(dark[0].element)} is`} not on stage.`,
rect: toRect({
left: Math.min(rendered.start.x, rendered.end.x),
top: Math.min(rendered.start.y, rendered.end.y),
right: Math.max(rendered.start.x, rendered.end.x),
bottom: Math.max(rendered.start.y, rendered.end.y),
width: Math.abs(rendered.end.x - rendered.start.x),
height: Math.abs(rendered.end.y - rendered.start.y),
}),
fixHint:
"Show the shaft only after both ends are on, and hide it with the earlier exit. Do not give the line its own clock.",
});
}
return issues;
}
Expand Down
Loading
Loading