fix(ui-ux): give Event Lineage DAG node marks a 24x24px hit target - #554
fix(ui-ux): give Event Lineage DAG node marks a 24x24px hit target#554seonghobae wants to merge 14 commits into
Conversation
The visible node mark in LineageDag.tsx is a 7px-radius (14px) SVG circle with no separate hit area, well under the WCAG 2.2 SC 2.5.8 AA minimum of 24x24 CSS px and this codebase's own --size-control-min token (styles/tokens.css). Add a transparent, pointer-events:all circle (r=12, matching --size-control-min at this DAG's ~1 user-unit-per-px scale) as the first child inside the existing role="button" <g>, ahead of the visible mark, so it enlarges the click/tap area without changing appearance. ROW_H is a 52px row pitch (lineageLayout.ts), so a 24px-diameter hit circle leaves 28px of clearance between adjacent rows -- no overlap. Adds LineageDag.test.tsx asserting the hit circle is present, sized >=24px diameter, transparent, and painted before the visible mark, plus a click-still-works regression check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011jWJzKUd82yy97esEBfJbt
|
Warning Review limit reachedNext included review available in 54 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ll rule The .lineage-dag-node circle rule paints every child circle with --surface-muted plus a border stroke, and author CSS overrides the fill="transparent" presentation attribute, so the enlarged hit target rendered as a second opaque disc. Scope an explicit transparent/no-stroke override to .lineage-dag-hit (devin review thread).
- The unauthenticated Log in handler now calls returnUrlFromLocation() then rememberOidcReturnUrl() before signinRedirect, per ADR 0109, so a shared /?post= link still opens that post after enterprise SSO. The previously unused oidcReturnUrl import is now load-bearing. - AdminPanel renders only when accessToken is a string; the OIDC access token is string | undefined before isAuthenticated narrowing.
The aria-current ring rule (0,2,1) outranked the hit-circle override (0,2,0) and painted a large hollow ring around the current node's mark. Exclude .lineage-dag-hit from the focus/hover/current selectors via :not(), which never matches the hit circle at all, and lift the override to (0,3,0) so future circle-scoped emphasis rules lose by specificity rather than source order (devin review thread).
Equal-specificity source-order was still letting the aria-current ring paint over the hit circle. :not(.lineage-dag-hit) on the emphasis selectors never matches the hit target, closing the ring regression for good.
width="100%" plus a viewBox let the container scale the coordinate system, shrinking the effective 24px hit target below WCAG 2.5.8 on wide containers. Render at the layout's own user-unit width (height already behaves this way) so one SVG unit stays ~1 CSS px and the hit radius keeps its intended size; horizontal overflow is handled by the panel's existing scroll container.
| <circle | ||
| className="lineage-dag-hit" | ||
| r={NODE_HIT_RADIUS} | ||
| fill="transparent" | ||
| style={{ pointerEvents: "all" }} | ||
| /> | ||
| <circle r={7} /> |
There was a problem hiding this comment.
📝 Info: Hit circle stacking and specificity are consistent
The transparent hit circle is rendered first so the visible mark paints over it, and the added .lineage-dag-hit rule plus the :not(.lineage-dag-hit) guards on the hover/focus/current selectors (App.css:564-571) keep the enlarged target from painting as a second disc or ring.
Was this helpful? React with 👍 or 👎 to provide feedback.
| @media (max-width: 768px) { | ||
| .lineage-dag-scroll-hint { | ||
| display: block; | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Scroll hint shown when lineage fits on screen
The .lineage-dag-scroll-hint renders for every figure and shows on any viewport under 768px, regardless of whether the canvas overflows. A single-node or narrow lineage on a phone still tells the buyer to swipe when there is nothing to scroll.
Was this helpful? React with 👍 or 👎 to provide feedback.
| </svg> | ||
| <svg | ||
| viewBox={`0 0 ${group.width} ${group.height}`} | ||
| width={group.width} |
There was a problem hiding this comment.
📝 Info: SVG now renders at intrinsic width on all viewports
Changing the svg from width="100%" to width={group.width} means a wide or deep lineage no longer scales down to fit its container on desktop; it overflows into the horizontal-scroll viewport instead. Both the popup and Ask-answer call sites are affected, not only narrow screens.
Was this helpful? React with 👍 or 👎 to provide feedback.
Gap
Dimension:
touch_interaction(medium severity)Event Lineage DAG node marks (
frontend/src/LineageDag.tsx) are a plain<circle r={7} />— a 14px-diameter visible mark with no separate hitarea — inside a
role="button"<g>that already carries theclick/keydown handlers. That's well under the WCAG 2.2 SC 2.5.8 (AA)
minimum target size of 24x24 CSS px.
Note on the originally-filed evidence: the gap report's code snippet
(a
NodeMarkcomponent withNODE_RADIUS, root/branch shape variants,a
--size-control-min ≈ 44pxconvention) does not match this file'sactual current state — it appears to describe a different revision. I
re-read the file in this worktree before editing per the task's ground-
truth-verification step. The real, current
--size-control-mintoken infrontend/src/styles/tokens.cssis24px, not 44px, and there is noother 44px touch-target convention anywhere in the repo (verified via
repo-wide grep). This fix targets the actual code and the actual token
value.
Fix
Add a transparent,
pointer-events: allcircle as the first childinside the existing
<g role="button">, ahead of the visible 7px mark,sized
r=12(24px diameter) — matching--size-control-minat thisDAG's ~1-SVG-user-unit-per-px scale (
lineageLayout.tsalready treatsROW_H/COL_W/PADas pixel-equivalent units). No CSS/appearancechange; only the effective click/tap area grows.
ROW_His a 52px rowpitch, so a 24px hit circle leaves 28px of clearance between vertically
adjacent nodes — no collision risk.
Tests
Added
frontend/src/LineageDag.test.tsx:is
fill="transparent"withpointer-events: all, and hasr >= 12onSelectPostConfirmed red→green: the new hit-target test fails on the pre-fix
LineageDag.tsx(expected 2 circles, got 1) and passes after the fix.Check output (from
frontend/)Not merging -- leaving for review per team process.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com