Skip to content

fix(web): hovering a commit marker opens its commit card again - #2578

Merged
Makisuo merged 2 commits into
mainfrom
fix/marker-hover-card
Nov 14, 2026
Merged

Makisuo merged 2 commits into
mainfrom
fix/marker-hover-card

fix(web): draw commit markers beside the chart so their card can bind

6e97b75
Select commit
Loading
Failed to load commit list.
Maple Review Bot / Maple / review completed Nov 14, 2026 in 2m 43s

Confidence 8/10 · 1 issue to address

🟢 Confidence 8/10 · likely safe to merge. The marker card is wired the way the existing commit card is, but its own open path and row rendering ship untested.
1 warning −1 · tests partial −1 · risk low

Hovering a deploy marker now opens a card listing the commits first seen in that bucket, wired on the service Overview grid, the API responses chart and the release detail grid. The wiring follows the existing commit-card pattern; the gap is that the card's own behaviour is not pinned by a test.

  • markerCardBody renders a marker group's commits in a shared HoverCard
  • markersOverlay takes a page card and binds label or dash triggers to it
  • markerShas resolves every commit a card lists, capped at MARKER_CARD_COMMITS
  • The marker layer moves beside the chart on the grid, API tab and release grid

Findings

🟠 Warning · F1 · The marker card's open path and its rows have no test

tests · apps/web/src/page/kit/service-metrics/marker-card.ts:70-74

markers.test.ts pins only markerShas. Nothing exercises what this PR adds: markersOverlay handing triggerFor to a label or dash, the groupOf lookup that resolves a trigger to its group, or markerCardBody listing a merged group's commits with the +N more cap. The repository scene-tests hover cards of exactly this shape.

Add a scene test: hover a grid marker label, resolve `WaitHoverDelay` and `MountHoverCard`, then assert the card lists every commit of the merged group and stops at `MARKER_CARD_COMMITS` with `+N more`.
What was checked
  • Marker positions unchanged: the overlay stays absolute inset-0 in the same relative wrapper linkedCursorOverlay already uses (grid.ts:194), and labelGap is a chart-spec gutter
  • markerShas covers every row a merged group shows: the card's first 8 rows lie in the first 8 commits of their own markers (markers.ts:34)
  • toContent resolves a trigger id through groupOf, which is rebuilt with the same trigger list passed as triggerIds (markers-view.ts:162), and the popup is portaled to body so the `pointer-event…

6e97b75 · Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

Annotations

Check warning on line 74 in apps/web/src/page/kit/service-metrics/marker-card.ts

See this annotation in the file changed.

@maple-review-bot maple-review-bot / Maple / review

tests: The marker card's open path and its rows have no test

`markers.test.ts` pins only `markerShas`. Nothing exercises what this PR adds: `markersOverlay` handing `triggerFor` to a label or dash, the `groupOf` lookup that resolves a trigger to its group, or `markerCardBody` listing a merged group's commits with the `+N more` cap. The repository scene-tests hover cards of exactly this shape.