-
Notifications
You must be signed in to change notification settings - Fork 4.1k
feat(lint): add connector_motion_detached layout check #2745
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.
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
Changes from all commits
f5daa69
7ea9c46
7223858
41f8bcc
69f3d45
c1e50ae
d62b8f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -198,7 +198,9 @@ | |
| return false; | ||
| } | ||
| const rect = element.getBoundingClientRect(); | ||
| if (rect.width <= 0.5 || rect.height <= 0.5) return false; | ||
| // Stroke is excluded from an SVG shape's client rect, so axis-aligned geometry measures 0 on one axis. | ||
| const stroke = typeof element.getBBox === "function" ? parseFloat(style.strokeWidth) || 0 : 0; | ||
| if (rect.width + stroke <= 0.5 || rect.height + stroke <= 0.5) return false; | ||
| return probeClipPath === false || !isClippedAway(element); | ||
| } | ||
|
|
||
|
|
@@ -1233,10 +1235,15 @@ | |
| const mapped = point.matrixTransform(matrix); | ||
| return { x: mapped.x, y: mapped.y }; | ||
| }; | ||
| return { | ||
| start: toScreen(path.getPointAtLength(0)), | ||
| end: toScreen(path.getPointAtLength(total)), | ||
| }; | ||
| // getPointAtLength can throw on a degenerate path even after getTotalLength; skip that path instead of aborting the whole per-frame check. | ||
| try { | ||
| return { | ||
| start: toScreen(path.getPointAtLength(0)), | ||
| end: toScreen(path.getPointAtLength(total)), | ||
| }; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| function distanceToRect(point, rect) { | ||
|
|
@@ -1266,6 +1273,16 @@ | |
| return { compact, painted }; | ||
| } | ||
|
|
||
| /** Connector naming for the motion sampler, tolerant of camelCase ids the word-bounded name test would miss. */ | ||
| function isMotionConnectorCandidate(svg, path) { | ||
| if (isConnectorPath(svg, path)) return true; | ||
| const spaced = `${connectorNameFor(svg)} ${connectorNameFor(path)}`.replace( | ||
| /([a-z])([A-Z0-9])/g, | ||
| "$1 $2", | ||
| ); | ||
| return CONNECTOR_NAME.test(spaced); | ||
| } | ||
|
|
||
| function isConnectorPath(svg, path) { | ||
| if (path.hasAttribute("marker-start") || path.hasAttribute("marker-end")) return true; | ||
| return ( | ||
|
|
@@ -1734,4 +1751,174 @@ | |
| } | ||
| return samples; | ||
| }; | ||
|
|
||
| // connector_motion_detached sampling: per frame, report each connector's screen endpoints and every node bbox; icon-sized SVGs and short strokes are filtered out. | ||
| const CONNECTOR_MIN_SVG_PX = 100; | ||
| const CONNECTOR_MIN_LEN_PX = 60; | ||
| // Gauge needles/pointers/ticks are one-end-anchored indicators owned by a separate check; skip by id/class of the line or any group ancestor. | ||
| // Word-bounded so `sticky`/`ticker` do not match, and pointer-events is excluded: a Tailwind utility must not mute a whole subtree. | ||
| const CONNECTOR_INDICATOR_NAME = /\b(needle|pointer(?!-events)|gauge|tick|indicator)\b/i; | ||
| const CONNECTOR_NODE_MIN_AREA = 400; | ||
| // SVG dots/markers are small; keep the floor low but above sub-pixel decoration. | ||
| const CONNECTOR_NODE_MIN_DOT_AREA = 16; | ||
| const CONNECTOR_NODE_CAP = 300; | ||
| // Recognize near-circular stroke <path> arcs as ring nodes so connectors attaching to them aren't false-flagged and the gauge exclusion sees arc-drawn dials. | ||
| const CONNECTOR_RING_MIN_LEN = 120; | ||
| const CONNECTOR_RING_MIN_RADIUS = 20; | ||
| const CONNECTOR_RING_MAX_RESIDUAL_FRAC = 0.15; | ||
| // Phantom-radius guard: a shallow arc fits an enormous circle with a tiny residual, so reject when the fitted diameter runs past this factor of the bbox. | ||
| const CONNECTOR_RING_MAX_DIAMETER_BBOX_FRAC = 3; | ||
|
|
||
| function isIndicatorConnector(line, svg) { | ||
| for (let node = line; node && node !== svg.parentElement; node = node.parentElement) { | ||
| if (CONNECTOR_INDICATOR_NAME.test(connectorNameFor(node))) return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| function lineScreenEndpoints(svg, line) { | ||
| if (typeof line.getScreenCTM !== "function" || typeof svg.createSVGPoint !== "function") { | ||
| return null; | ||
| } | ||
| const matrix = line.getScreenCTM(); | ||
| if (!matrix) return null; | ||
| const map = (x, y) => { | ||
| const point = svg.createSVGPoint(); | ||
| point.x = x; | ||
| point.y = y; | ||
| const mapped = point.matrixTransform(matrix); | ||
| return { x: mapped.x, y: mapped.y }; | ||
| }; | ||
| return { | ||
| start: map(line.x1.baseVal.value, line.y1.baseVal.value), | ||
| end: map(line.x2.baseVal.value, line.y2.baseVal.value), | ||
| }; | ||
| } | ||
|
|
||
| // Read a near-circular stroke <path> as a ring/hub node box (ring flag from fill), or null if not a resolvable hub; getPointAtLength is guarded so one bad path can't abort the sampler. | ||
| function ringPathBox(path, rootRect) { | ||
| if (typeof path.getTotalLength !== "function" || typeof path.getPointAtLength !== "function") { | ||
| return null; | ||
| } | ||
| let total; | ||
| try { | ||
| total = path.getTotalLength(); | ||
| } catch { | ||
| return null; | ||
| } | ||
| if (!Number.isFinite(total) || total < CONNECTOR_RING_MIN_LEN) return null; | ||
| const points = []; | ||
| try { | ||
| for (let i = 0; i < 16; i++) { | ||
| const local = path.getPointAtLength((total * i) / 16); | ||
| points.push({ x: local.x, y: local.y }); | ||
| } | ||
| } catch { | ||
| return null; | ||
| } | ||
| const fit = fitCirclePoints(points); | ||
| if (!fit || fit.radius < CONNECTOR_RING_MIN_RADIUS) return null; | ||
| if (fit.residual > CONNECTOR_RING_MAX_RESIDUAL_FRAC * fit.radius) return null; | ||
| const rect = toRect(path.getBoundingClientRect()); | ||
| // Reject the shallow-curvature phantom fit: a nearly-straight arc masquerades as a huge ring, so the fitted diameter must stay within a sane factor of the bbox. | ||
| const bboxSpan = Math.max(rect.width, rect.height); | ||
| if (bboxSpan <= 0 || fit.radius * 2 > CONNECTOR_RING_MAX_DIAMETER_BBOX_FRAC * bboxSpan) { | ||
| return null; | ||
| } | ||
| // Known narrow seams left as-is: a sliver partial arc can miss the span gate, and a near-circular connector can read as its own ring node — both rare vs. the false ring this gate closes. | ||
| const area = rectArea(rect); | ||
| if (area < CONNECTOR_NODE_MIN_DOT_AREA || area >= rectArea(rootRect) * 0.5) return null; | ||
| const fill = getComputedStyle(path).fill; | ||
| return { | ||
| selector: selectorFor(path), | ||
| left: rect.left, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 const rect = toRect(path.getBoundingClientRect());
// ...
return {
selector: selectorFor(path),
left: rect.left,
top: rect.top,
right: rect.right,
bottom: rect.bottom,
ring: ...,
};Downstream Consequence: over-anchoring on partial-arc gauges. The real fix is to keep the fit result and use Cheap intermediate: reject |
||
| top: rect.top, | ||
| right: rect.right, | ||
| bottom: rect.bottom, | ||
| ring: fill === "none" || fill === "transparent" || path.getAttribute("fill") === "none", | ||
| }; | ||
| } | ||
|
|
||
| function connectorNodeBoxes(root, rootRect) { | ||
| const boxes = []; | ||
| const rootArea = rectArea(rootRect); | ||
| for (const element of Array.from(root.querySelectorAll("*"))) { | ||
| if (boxes.length >= CONNECTOR_NODE_CAP) break; | ||
| if (element.closest("svg") || !isVisibleElement(element, 0.05)) continue; | ||
| const opaque = | ||
| RASTER_TAGS.has(element.tagName) || hasOpaqueBackground(getComputedStyle(element)); | ||
| if (!opaque && !textContentFor(element)) continue; | ||
| const rect = toRect(element.getBoundingClientRect()); | ||
| const area = rectArea(rect); | ||
| if (area < CONNECTOR_NODE_MIN_AREA || area >= rootArea * 0.5) continue; | ||
| boxes.push({ | ||
| selector: selectorFor(element), | ||
| left: rect.left, | ||
| top: rect.top, | ||
| right: rect.right, | ||
| bottom: rect.bottom, | ||
| ring: false, | ||
| }); | ||
| } | ||
| for (const shape of Array.from(root.querySelectorAll("circle, ellipse, rect"))) { | ||
| if (boxes.length >= CONNECTOR_NODE_CAP) break; | ||
| if (!isVisibleElement(shape, 0.05)) continue; | ||
| const rect = toRect(shape.getBoundingClientRect()); | ||
| const area = rectArea(rect); | ||
| if (area < CONNECTOR_NODE_MIN_DOT_AREA || area >= rootArea * 0.5) continue; | ||
| const fill = getComputedStyle(shape).fill; | ||
| boxes.push({ | ||
| selector: selectorFor(shape), | ||
| left: rect.left, | ||
| top: rect.top, | ||
| right: rect.right, | ||
| bottom: rect.bottom, | ||
| ring: fill === "none" || fill === "transparent" || shape.getAttribute("fill") === "none", | ||
| }); | ||
| } | ||
| for (const path of Array.from(root.querySelectorAll("path"))) { | ||
| if (boxes.length >= CONNECTOR_NODE_CAP) break; | ||
| if (path.closest(CONNECTOR_SKIP_CONTAINERS)) continue; | ||
| if (!isVisibleElement(path, 0.05)) continue; | ||
| const box = ringPathBox(path, rootRect); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 A near-circular const box = ringPathBox(path, rootRect);
if (box) boxes.push(box);
Failure scenario: a near-circular open-arc
Rare shape (most connectors are Fix: track ring-registered paths in a |
||
| if (box) boxes.push(box); | ||
| } | ||
| return boxes; | ||
| } | ||
|
|
||
| window.__hyperframesConnectorSample = function collectConnectorSample() { | ||
| const root = | ||
| document.querySelector("[data-composition-id][data-width][data-height]") || | ||
| document.querySelector("[data-composition-id]") || | ||
| document.body; | ||
| const rootRect = rootRectFor(root); | ||
| const connectors = []; | ||
| for (const svg of Array.from(root.querySelectorAll("svg"))) { | ||
| if (!isVisibleElement(svg, 0.05) || hasAllowOverflowFlag(svg)) continue; | ||
| const svgRect = svg.getBoundingClientRect(); | ||
| if (svgRect.width < CONNECTOR_MIN_SVG_PX || svgRect.height < CONNECTOR_MIN_SVG_PX) continue; | ||
| for (const line of Array.from(svg.querySelectorAll("line, path, polyline"))) { | ||
| if (line.closest(CONNECTOR_SKIP_CONTAINERS)) continue; | ||
| // Stroke-inflated: an axis-aligned line has a zero-height/width client rect, so the plain visibility gate drops it. | ||
| if (!isVisibleElement(line, 0.05)) continue; | ||
| if (isIndicatorConnector(line, svg)) continue; | ||
| if (!isMotionConnectorCandidate(svg, line)) continue; | ||
| const ends = | ||
| line.tagName.toLowerCase() === "line" | ||
| ? lineScreenEndpoints(svg, line) | ||
| : pathScreenEndpoints(svg, line); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 This new call site amplifies a defensive gap in the pre-existing const ends =
line.tagName.toLowerCase() === "line"
? lineScreenEndpoints(svg, line)
: pathScreenEndpoints(svg, line);Look at return {
start: toScreen(path.getPointAtLength(0)),
end: toScreen(path.getPointAtLength(total)),
};In Chrome the pair is usually safe once Failure mode: if either The pre-existing per-sample Fix: wrap the two — Review by Rames D Jusso |
||
| if (!ends) continue; | ||
| const len = Math.hypot(ends.end.x - ends.start.x, ends.end.y - ends.start.y); | ||
| if (len < CONNECTOR_MIN_LEN_PX) continue; | ||
| connectors.push({ | ||
| selector: selectorFor(line), | ||
| ax: round(ends.start.x), | ||
| ay: round(ends.start.y), | ||
| bx: round(ends.end.x), | ||
| by: round(ends.end.y), | ||
| }); | ||
| } | ||
| } | ||
| return { connectors, nodes: connectorNodeBoxes(root, rootRect) }; | ||
| }; | ||
| })(); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 The
ringPathBoxresidual gate is scale-relative and can be vacuous for shallow-curvature paths — a decorative flourish then registers as a ring node and silently suppresses real connector findings that end near it.Kåsa on a shallow-curvature path (say a 100px × 5px slightly-curved decorative stroke) produces a huge phantom-radius fit: as the determinant shrinks toward the degenerate-line case,
uc/vcgrow — the returnedradiuscan be enormous even though the true curvature is small. Meanwhile the absolute residual stays ~1px because the points do lie on SOMETHING circular. Ratioresidual / radius ≈ 0.001easily passes the 0.15 gate.CONNECTOR_RING_MIN_RADIUSdoesn't help because the phantom radius is LARGE.Consequence:
ringPathBoxreturns a node box withring: truefor that decorative flourish. DownstreampointToNodeGaptreats it as a ring perimeter with the flourish's bbox as the outline. Any dangling connector endpoint near the flourish's bbox reads as anchored → the trueconnector_motion_detachedfinding is silently suppressed.Fix: add a bbox-vs-radius sanity gate. A genuine ring's bounding box spans its diameter within a small factor; a shallow-curvature path's bbox is much smaller than the phantom-radius circle it fits to. Something like:
Or require the fit centre
(fit.cx, fit.cy)to sit within the bounding box (a real closed ring's centre is inside its own bbox; a shallow-arc phantom-fit's centre is far away).Background: this is the same lens Magi caught on
#2744's Kåsa fit — different failure mode (n=3 exact-fit there, degenerate-scale phantom-fit here), same underlying issue: residual-normalized-by-radius doesn't degrade gracefully when the fit is bad. Fitting>= CONNECTOR_RING_MIN_LEN-point curves (n = 16 vs 3 params here) makes it overdetermined, but doesn't rule out the shallow-curvature phantom.— Review by Rames D Jusso