fix: connector_detached now skips fully dash-hidden connector shafts - #3912
Open
miga-heygen wants to merge 2 commits into
Open
miga-heygen wants to merge 2 commits into
miga-heygen wants to merge 2 commits into
Conversation
connector_detached fired on SVG connector paths that were 100% hidden via stroke-dashoffset/stroke-dasharray draw-on entrances, evaluating rendered/user-space geometry unconditionally. The sibling connector_orphan check already gates on shaftDashHidden(path); add the same gate here, in the same position. Co-Authored-By: Miguel Angel <miguel.sierra@heygen.com>
…wner for connector enumeration `shaftDashHidden` read only the first `stroke-dasharray` entry, so any pattern starting with 0 (`0 4` dotted) or a long first dash counted as hidden and suppressed both connector findings on a visible stroke. It now resolves the full computed list (comma/space separated, units, percentages against the SVG viewport diagonal, odd lists repeated; `none`, all-zero and negative lists render solid) and treats the shaft as hidden only when the window `[dashoffset, dashoffset + length]` sits in a single gap, with up to 10% of the length overlapping a neighbouring dash. Zero-length dashes paint only under `stroke-linecap: round | square`; under the default `butt` they render nothing, which also covers the `0px, 999999px` state a finished draw-off tween leaves behind. `connector_detached` and `connector_orphan` each walked SVG paths with their own copy of the candidate/gate logic, which had already drifted. A single `connectorShafts(root)` generator now owns enumeration, the dash gate and endpoint resolution, and yields the `painted` verdict; both findings consume it. The path length is queried once per shaft (`pathUserEndpoints` returns it) instead of twice, and the attach threshold formula has one owner. Tests pin `0 4` (butt and round), `0, 4`, `4 0`, `0`, `none`, px units, negative offsets, a draw-off end state, and both sides of the 10% boundary. Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com>
Contributor
Author
|
Addressed in 58aefbb. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
connector_detached(the layout-audit finding for SVG connector paths whose rendered endpoints don't attach to any anchor) evaluated a path's geometry unconditionally — it had no gate forstroke-dashoffset/stroke-dasharraydraw-on visibility. A connector fully hidden behind an un-advanced "draw the line in" dash animation still fired the finding as if it were rendered and visible. The sibling check,connector_orphan, already skipped fully dash-hidden shafts viashaftDashHidden(path).Two things changed on top of simply adding that gate:
One owner for connector enumeration.
connector_detachedandconnector_orphaneach walkedsvg pathwith their own copy of the candidate/gate logic (visible svg, skipdefs/marker/clipPath/mask/symbol/pattern, connector intent, endpoint resolution, dash gate) and the copies had drifted. A singleconnectorShafts(root)generator now owns that walk and yields{ svg, path, user, rendered, chord, painted }; both findings consume it.connector_orphanstill requirespainted(display/visibility/opacity);connector_detachedstill does not gate on it — that is unchanged behaviour and documented at the owner. The path length is queried once per shaft (pathUserEndpointsnow returnstotal) instead of twice, and the attach-threshold formula has one owner.shaftDashHiddenreads the whole pattern. It previously read only the firststroke-dasharrayentry, so a visible dotted pattern like0 4— or any pattern with a long first dash — was treated as hidden and suppressed both findings. It now resolves the full computed list (comma/space separated, units, percentages against the SVG viewport's normalised diagonal, odd lists repeated;none, all-zero and negative lists render solid) and treats the shaft as hidden only when the visible window[dashoffset, dashoffset + length]sits inside a single gap, allowing up to 10% of the length to overlap a neighbouring dash (the tail of a nearly finished tween — same boundary the old>= 0.9 * lengthrule had). A zero-length dash paints only as around/squareline cap; under the defaultbuttit renders nothing, which also covers the0px, 999999pxstate a finished draw-off tween leaves behind.Test plan
strokeDasharray/strokeDashoffsetno longer firesconnector_detached, on the same fixture as the adjacent positive control (sharedforeignFrameDom/foreignFrameRects/foreignFrameStyles, mirroring this file'sorphanDomconvention).0 4with butt cap → hidden;0 4round and0, 4square → painted;0px, 999999px@-99.999px→ hidden;0 400round @ 0 → painted (dot inside the window), @ 1 → hidden;4 0,0,none→ painted;100px@100pxand100@-100→ hidden;50 100@ 50 and @ 40 (exactly 10% painted) → hidden, @ 30 → painted.connector_orphantests (including its dash-hidden case) pass unchanged against the shared enumerator.layout-audit.browser.test.tsdoes not collect under this sandbox's vitest (No such built-in module: node:— pre-existing, also on an unmodified checkout). Run instead underbun testwith a happy-dom preload: 126 pass, 9 fail, where the 9 arecontrast-auditcases that fail identically before this change.bunx tsc --noEmit -p packages/cli,oxlint,oxfmt --checkclean; pre-commit lint/format/fallow/typecheck hooks pass.%dash lengths (resolved againstviewBox, else the SVG's layout box) andcalc()values (parse toNaN→ treated as solid, i.e. the safe direction).