fix(viewer): stabilize relationship exploration - #225
sunsunsun-java wants to merge 18 commits into
Conversation
Reserve a scrollable Passport height before compact Radar fallback so mobile relationship exploration can recover without overlay collisions. Replace the output-path race fixture's fixed delay with an explicit release handshake.
Move the persistent CDP pointer outside the Passport before every hover scenario, and wait for the user-visible reveal result instead of a pre-animation layout gap.
Declare the browser media capabilities required by the desktop-hover contract so headless CI does not silently exercise the non-hover fallback.
Keep delayed programmatic scroll events inside the semantic camera grace window. Make the hover regression explicitly install a fine-pointer environment before the Viewer initializes.
|
Warning Review limit reachedNext included review available in 38 minutes. View limit detailsLimit details: You’ve used all 10 included reviews 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (10)
📒 Files selected for processing (61)
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 |
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
The relationship-exploration fix is worthwhile: keeping hover camera-neutral while explicit activation reveals the relevant nodes directly addresses #221. The compact/expanded Passport and cancellation coverage are useful. I reviewed head 3cc0b9bbe2e979e5278e1e529956a48d8aa33858 against current dev 24285de8b1b65fe39ee1b3965cbe4d5239c499d5 (the PR merge base is 5de7275). I am requesting changes before integration.
Blocking performance regression — archify/scripts/check-render-output.mjs:628-678. collectDesktopReadability() invokes nearestAncestorDetail(fragment, match.index) for every text element without its own detail attribute. The helper reparses all preceding SVG tags each time, including for primary/boundary labels whose classification does not need that lookup. A valid synthetic SVG containing repeated <g data-detail="context"><text font-size="12" ...>Label</text></g> takes approximately 0.38 / 1.45 / 5.58 seconds for 1,000 / 2,000 / 4,000 labels on this head, versus 35 / 38 / 47 milliseconds on dev. Both outputs report ok: true. These are local Node 22 CLI timings including startup, not end-to-end diagram generation measurements. Resolve inherited detail during one traversal (or reuse an existing parsed ancestry representation), preserving nearest-ancestor precedence. Add bounded large-input regression coverage that establishes the scaling behavior without a machine-sensitive stopwatch assertion.
Integration work needed. Refresh against dev and port the Viewer changes to the canonical viewer/ sources before regenerating the template. Preserve the newer Passport placement fix (#207), visual-check sidecar semantics (#290), shared browser gate (#440), and the removal of tracked Maka output (#383). Reuse existing browser helpers where possible; register the new relationship-explorer suite in the shared browser gate. These are final integration requirements, not a request to redesign the agreed interaction or split it arbitrarily into several PRs.
Independent verification on this head: 136 tests passed, 0 failed, 0 skipped across viewer-relationship-explorer, desktop-reader-browser, render-output-checks, and layout-rules with Node 22 and real Chrome on macOS. This includes the physical pointer/keyboard tests present in that selection, translated controls, responsive disclosure, and rapid hash replacement. I did not rerun the full suite or all of viewer-chrome-layout, and this is not independent perceptual acceptance of the showcase screenshots.
Please retain the readable-label and inherited-detail checks, fix the traversal cost, then provide the current-dev integration and affected generated outputs once. The intended behavior is valuable; the current blocker is the extra checker cost and unfinished integration, not the use of a compact Passport.
2 participants
Problem and value
Closes #221.
Relationship exploration could hide highlighted nodes behind the Semantic Passport, overflow long inbound/outbound lists, and repeatedly reframe the camera while the pointer crossed relationship rows. Large diagrams were especially difficult to inspect.
The Viewer now separates preview from navigation: hover/focus only highlights, while Click, Enter, or Space performs one explicit reveal. Framing uses the remaining safe viewport around visible overlays, and wide showcase diagrams keep relationship labels above the desktop readability floor.
Scope
data-detail="context"from relationship groups with nearest-detail precedence.The architecture schema, semantic inference, URL forms, canonical SVG contract, and non-relationship Viewer workflows remain compatible.
Verification
visual-check: all required viewports passed containment, readability, and Viewer chrome checks.archify.ziprebuild is byte-identical.git diff --check: passed.Generated artifacts
archify.zip, affected Gallery artifacts/manifest, README proof GIF/receipt, MCO public/experiment HTML, Maka HTML plus visual-check sidecars, and the affected public/package examples.e86823096c5bad39e1ca282ec24bc5eb9bec745bfor local visual review; project-specific Pi output remains intentionally untracked.Checklist