CLUE-709: Timeline Tile — update how a marker is created, placed and deleted

[lbondaryk]

Closes CLUE-709.

Placing a time marker becomes an explicit tool rather than something the plot does all the time, and the marker it leaves behind is the student's own work: it persists in the document, can be moved, and is deleted deliberately.

CLUE-719 had already replaced click-to-pin with zoom, and CLUE-673 removed the follow line, so this is built as a new mode rather than a change to the old pinning behaviour.

What a student does

• Add Time Marker in the toolbar arms placement mode. A dashed line previews where the marker will land as the pointer moves.
• Clicking the plot places the marker. Placement mode ends immediately and the plot goes back to zooming — placing is one gesture, not a mode to turn off. Escape cancels.
• The marker persists in the document, and shows on the full timeline strip as well as the graph, so it can still be found once it has scrolled out of the zoomed view.
• Dragging the stem or the label moves it. The model is written once on pointer-up, so a drag is a single undo step.
• The x in the label deletes it. Only one marker at a time; the button re-enables once it is gone.

Notable implementation points

• pinnedTime is renamed markerTime throughout and moved from volatile into .props() so it is saved.
• viewPctToTime clamps to the loaded data range, not the visible view, so a drag past the edge of the plot cannot land the marker where there is no data.
• The icon is assembled from existing house icons: the frame from show-navigator-icon.svg (hence the bolder bottom bar) and the plus from add-data-graph-icon.svg, mirrored to the bottom-right. The gap around the plus is unpainted rather than filled, because toolbar.scss sets fill on every path — a halo drawn as a path would recolour to charcoal and disappear.
• The marker button moved ahead of the zoom group and is titled "Add Time Marker", per the toolbar design.

Also in here, strictly adjacent

The per-event shapes were removed from the full timeline strip. They sat at top: -9px, in the same band as the "Full Timeline" header and the marker label that hangs down from the plot. They are still drawn on the graph above. The header also now outranks the marker label in z-order.

Testing

162 tests in src/plugins/timeline; full suite 5439 passing. Each task's assertions were mutation-checked.

One gap worth flagging for reviewers: jsdom ignores pointer-events when dispatching, so a click test passes against a control that is unusable in a browser. That bit — the delete button was dead because the placed label never opted back in to pointer events. time-marker-overlay.styles.test.ts guards that contract by reading the stylesheet, since no rendering test can.

Not included

"Add Event" appears in the toolbar design but has no button or config entry; it is separate work.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Read-only mutation, drag lifecycle, touch, and keyboard accessibility issues remain unresolved.

6 open findings

• Disable marker interactions in read-only views · New
• Apply touch-action policy to the interactive grip · New
• Make marker movement keyboard-operable · New
• Handle lost pointer capture during marker dragging · New
• Keep drag target mounted when dragged out of view · New
• Add keyboard controls for marker placement · New

What changed in this PR

Adds a persistent, movable timeline marker with explicit placement, deletion, preview, and full-timeline display.

Changes:

• Persists marker state and adds placement/dragging behavior.
• Adds toolbar controls, marker visuals, and accessibility announcements.
• Expands model and component test coverage.

File	Description
src/​plugins/​timeline/​timeline-toolbar.tsx	Adds the marker toolbar button.
src/​plugins/​timeline/​timeline-toolbar.test.tsx	Tests marker button states.
src/​plugins/​timeline/​models/​timeline-content.ts	Persists marker state and placement mode.
src/​plugins/​timeline/​models/​timeline-content.test.ts	Tests persistence and time conversion.
src/​plugins/​timeline/​components/​timeline-plot.tsx	Implements marker placement and cancellation.
src/​plugins/​timeline/​components/​timeline-plot.test.tsx	Tests placement behavior.
src/​plugins/​timeline/​components/​timeline-plot.scss	Adds the placement cursor.
src/​plugins/​timeline/​components/​time-marker-overlay.tsx	Adds marker dragging and deletion.
src/​plugins/​timeline/​components/​time-marker-overlay.test.tsx	Tests marker interactions.
src/​plugins/​timeline/​components/​time-marker-overlay.styles.test.ts	Verifies pointer-event styling.
src/​plugins/​timeline/​components/​time-marker-overlay.scss	Styles marker states and controls.
src/​plugins/​timeline/​components/​full-timeline.tsx	Displays markers on the overview strip.
src/​plugins/​timeline/​components/​full-timeline.test.tsx	Tests overview marker rendering.
src/​plugins/​timeline/​components/​full-timeline.scss	Styles overview markers and layering.
src/​plugins/​timeline/​assets/​toolbar/​add-marker-icon.svg	Provides the toolbar icon.
src/​clue/​app-config.json	Enables the marker toolbar control.
docs/​superpowers/​specs/​2026-10-09-clue-709-timeline-marker-design.md	Documents the feature design.
docs/​superpowers/​plans/​2026-10-09-clue-709-timeline-marker.md	Records the implementation plan.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.22%. Comparing base (74f49f6) to head (a9e9401).
⚠️ Report is 68 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #3032    +/-   ##
========================================
  Coverage   87.21%   87.22%            
========================================
  Files        1064     1065     +1     
  Lines       61064    61182   +118     
  Branches    16325    16372    +47     
========================================
+ Hits        53256    53363   +107     
- Misses       7785     7796    +11     
  Partials       23       23            

Flag	Coverage Δ
cypress	?
cypress-regression	69.92% <3.84%> (-0.19%)	⬇️
cypress-smoke	40.27% <3.84%> (-0.11%)	⬇️
jest	62.15% <100.00%> (+0.07%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cypress]

collaborative-learning    Run #20664

Run Properties:   Passed #20664  •  a9e9401299: Merge master into CLUE-709 and address the Copilot review

Project	collaborative-learning
Branch Review	CLUE-709-timeline-tile-update-how-a-marker-is-created-placed-deleted
Run status	Passed #20664
Run duration	03m 45s
Commit	a9e9401299: Merge master into CLUE-709 and address the Copilot review
Committer	Leslie Bondaryk
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	0
Skipped	0
Passing	4
View all changes introduced in this branch ↗︎

[kswenson]

PR Review Summary

Changes: 22 files, +2185 / -123 lines (about 1170 of the added lines are the spec and plan under docs/superpowers/)

What it does

The Timeline tile gets an Add Time Marker toolbar button. The button arms a one-shot placement mode: a dashed preview follows the pointer, and the next click on the plot places the marker instead of zooming. The marker is now part of the document: pinnedTime (volatile) becomes the markerTimeISO prop. It can be dragged by its stem or its label, nudged from the keyboard (the stem is a role="slider"), and deleted with an × or Delete/Backspace. It is also drawn on the Full Timeline strip. The PR also removes the per-event shapes from the strip, which crowded the header band.

The approach

• Model (timeline-content.ts): adds markerTimeISO as a prop and isPlacingMarker as volatile state. setMarkerTime also ends placement and clears the hover preview. The new viewPctToTime is the inverse of timeToViewPct and clamps to the loaded data range.
• Plot (timeline-plot.tsx): a short click places the marker while placing; otherwise it zooms. Pointer-move feeds the preview, and a window-level Escape listener cancels.
• Overlay (time-marker-overlay.tsx): owns the drag. It keeps the mid-drag time in local state and writes the model once on pointer-up. It handles pointer capture, including lost capture, and holds the marker at the edge mid-drag so the captured target isn't unmounted. Keyboard nudges are restricted to the visible view. The overlay is pointer-events: none and the grips opt back in, which time-marker-overlay.styles.test.ts guards by reading the stylesheet.
• Strip (full-timeline.tsx): the strip marker is an indicator positioned against the whole data range.

Assessment

The core design is sound and carefully done:

• one model write per drag;
• pointer capture handled across up, cancel and lost capture;
• the edge hold during a drag;
• a real slider with time aria-valuetext;
• focus restored when the marker is deleted;
• a tested Tab order.

Lint and types are clean, and src/plugins/timeline passes 202 tests.

The gaps cluster around the marker's lifecycle outside the happy path:

• A marker can be left where it can neither be seen nor deleted, which permanently disables the Add button.
• On a real tile, the stem's ArrowUp nudge throws focus out of the marker.
• Placement mode has no pointer way out and outlives the tile's selection.

None of this is hard to fix.

Issues

Major

1. A marker outside the loaded data can never be removed, and Add Time Marker stays disabled for good.

   • Where: timeline-content.ts (markerTime), timeline-toolbar.tsx:101, time-marker-overlay.tsx (the isPctInView render guard)
   • Cause: changing the date range or station in Wave Runner calls sharedSeismogram.setTimeRange. The Timeline's effect (timeline.tsx:26-43) clamps the view to the new data, but nothing touches markerTimeISO.
   • Failure:
     • A student places a marker on 10/3, then moves Wave Runner's start date to 10/5.
     • The marker can't come back into view, because the view can't leave the data, so neither the × nor the slider's Delete key is reachable.
     • disabled={!!markerTime || …} keeps Add disabled.
     • After a reload, once the undo history is gone, only deleting the tile clears it.
   • The spec's "Known edges" accepts an out-of-view marker on the grounds that the student can pan back. That doesn't hold once the marker is outside the data.
   • Fix options: clear or clamp the marker when the data stops covering it; or base the button's disabled state on whether the marker is within the data, letting a new placement replace a stranded one.
   • Verified.

2. The strip pins an out-of-range marker to its edge (full-timeline.tsx FullTimelineMarker).

   • Cause: it reuses toStripPct, which clamps to 0–100. Clamping is right for event bars, which overlap the range, but wrong for a point.
   • Failure: in case 1, the strip shows a solid marker at its far end that reads as "marker at the end of the data", while the graph shows nothing even after View All.
   • Fix: render nothing when the time is outside [totalStart, totalEnd].
   • Verified.

3. ArrowUp on the stem nudges once and then moves focus to the tile container (time-marker-overlay.tsx handleKeyDown).

   • Cause: the handler calls preventDefault but not stopPropagation. TileComponent.handleKeyDown (tile-component.tsx:737-749) treats ArrowUp from any non-editable element inside the tile as a soft exit.
   • The Full Timeline thumb, the same kind of slider, guards against exactly this (dynamic-scrollbar.tsx:89-90: "Keep the keys the slider uses from also acting on a container, such as a tile's ArrowUp exit").
   • The overlay tests render the overlay without a tile, so they can't catch it.
   • Fix: add e.stopPropagation() for the handled keys, plus a tile-level test.
   • Verified.

Minor

4. A drag past the edge of the plot commits a time outside the view, and the marker vanishes on release (time-marker-overlay.tsx handleDragMove).

   • Cause: viewPctToTime clamps to the data, not the view. The stem is held at the edge while dragging, but its label shows the running time.
   • Failure: on release, isPctInView is false and the marker disappears until the student pans to it. The keyboard path clamps to the view (clampPct), so the two paths disagree.
   • Consider clamping the drag to the view as well. The PR's "must not land where there is no data" is then satisfied for free.

5. The dashed placement preview freezes when the pointer leaves the plot (timeline-plot.tsx).

   • Cause: there is no onPointerLeave, and clearHoverTime now has no caller outside tests.
   • Failure: arm placement, move across the plot, then move to the toolbar. A dashed line and time label stay put on both the graph and the strip, looking like a marker.
   • Related: hoverTime is only ever set while placing, so the overlay's "hover" class branch and the .hover styles are now unreachable.

6. Placement mode has no pointer way out and outlives its context.

   • The button reports aria-pressed (via selected), but clicking it again calls startPlacingMarker again, so it doesn't toggle off. Escape is the only exit, and nothing tells the user about it.
   • The volatile flag survives the tile being deselected. Its toolbar disappears, and a later click on that plot places a marker instead of zooming.
   • Suggest: make the button toggle, and stop placing on deselect.

7. The Escape listener is window-wide (timeline-plot.tsx:156-163).

   • Escape pressed anywhere cancels placement, without consuming the key.
   • The tile's own FocusTrapController also handles Escape (it exits the trap and deselects), so with focus in the tile one press does both.
   • The listener is registered before the readOnly early return, so read-only panes of the same model each add one.
   • Low impact, but a keydown on the tile (or a check that the focus is in this tile) would be tighter.

8. No keyboard way to place a marker.

   • The spec scopes placement as pointer-only, pending a keyboard design for the graph.
   • The result is a button a keyboard user can press ("pressed") that then does nothing they can reach. Under WCAG 2.1.1 (Keyboard) that is a blocker.
   • A cheap bridge: when activated from the keyboard, drop the marker at the view centre and focus the stem slider, which already supports fine positioning.
   • Fine to split out into its own story if that's agreed with design/PM.

9. Accessibility details:

   • The × is $charcoal-light-1 (#949494). That is 3.03:1 on white and 2.61:1 on the #eee hover background, below 4.5:1 for a 14px glyph. $charcoal-dark-2 or the icon grey #545454 would pass.
   • The × hit area is about 8×14px (padding: 0) and sits on a drag grip, so a near-miss starts a drag (2.5.8).
   • AddMarkerButton sets no disabledMessage, so activating it while disabled announces the default "Select something to enable this action". It should say "Delete the existing marker first" or "No data loaded".
   • Entering or cancelling placement and deleting a marker aren't announced; only placing is.

10. endDrag commits dragTime read from the render closure (time-marker-overlay.tsx:60-65).

    • Cause: pointermove is a continuous-priority update under React 18's automatic batching.
    • Failure: a pointerup that arrives before that render commits the previous position, or nothing if no render happened since pointerdown.
    • TimelinePlot keeps its drag in a ref for this reason. A ref alongside the state would close it (CLAUDE.md's valueRef pattern).
    • Plausible; not reproduced.

11. A second pointer hijacks an active drag (handleDragStart has no in-progress guard).

    • Failure: on touch, a second finger on the label takes over the drag.
    • Fix: if (dragPointerRef.current !== undefined) return.

12. The info modal still describes only zooming and moving (timeline-info-button.tsx).

    • The marker tool, dragging, × and Escape aren't described anywhere a student can see.
    • Possibly a design call, but the modal shipped one PR ago and is the natural home.

13. Test gaps worth closing (tests pass; these are mutations that would survive):

    • Backspace delete is untested.
    • The selected state of the Add button is untested.
    • The "Marker placed at …" announcement is untested.
    • Nothing tests that the Escape listener is removed after placement ends.
    • Nothing tests that the strip renders nothing for an out-of-range marker.
    • The time-marker-overlay.test.tsx test titled "does not start a drag from the delete control" only clicks. It never fires pointerdown/move, so handleDeletePointerDown is not exercised.
    • There's no Cypress coverage for the pointer-capture / pointer-events behaviour the PR itself says jsdom can't see. Nothing existing breaks: there are no Timeline specs at all.

Nit

14. Docs and process:

    • The PR commits a 1021-line implementation plan and a design spec under docs/superpowers/. Several parts of the plan are already stale: the "Add Marker" button name, a placeholder icon, and add-marker placed after view-all. The plan also contains Jira keys and git commit -m "CLUE-709: …" lines.
    • Suggest dropping both before merge; their durable reasoning is already in the code comments.
    • The new Timeline section in docs/unit-configuration.md lists the default buttons starting at add-marker. It omits the three data-set-view buttons and the separator that precede it in app-config.json.

15. A press-and-release on the stem or label with no movement still calls setMarkerTime. This only matters for a stored ISO not already in toUTC().toISO() form (for example, curriculum-authored), where it rewrites the string as a no-op undo entry. Commit only if the time changed.

16. Unchecked: if the seismic unit's content.json overrides settings.timeline.tools, the button won't appear there until that unit is updated. Worth checking before testing on unit=seismic.

Comments and docs

• Jira refs:
  • timeline-plot.test.tsx:406: "CLUE-673 removed the follow line deliberately" — drop the key.
  • The plan and spec files carry many more (see 14).
• Inaccurate:
  • timeline-content-controls.ts:2-3: "never the first or last of these". With 0–1 events, Prev and Next are both disabled, so the stem and the × come before the first enabled control, the thumb. Trap entry then skips them. The comment also leaves out the delete button. (verified)
  • time-marker-overlay.tsx:13-14: "The Full Timeline's scrollbar steps its view by the same fractions". The scrollbar steps by fractions of the whole data range (dynamic-scrollbar.tsx:63), not of the view. (verified)
  • time-marker-overlay.test.tsx:200: the title "does not start a drag from the delete control" doesn't match what the test does (see 13). (verified)
  • full-timeline.tsx:167: "the graph's timeToViewPct". It's a model method.
• Backward-looking:
  • time-marker-overlay.test.tsx:88: "the two can no longer coexist".
  • timeline-content.test.ts:733: "the marker used to be a transient pin…".
  • full-timeline.test.tsx:128-129: "On the strip they crowded the Full Timeline header".
  • Describe what's true now instead.
• Repeated explanations: in each case the source comment is canonical and the test comment restates it; trim the test copies or point to the source.
  • Grips opt back into pointer events: time-marker-overlay.scss:6 / styles.test.ts:4-5
  • The overlay needs its own touch-action: scss:9-11 / styles.test.ts:30
  • Hold at the edge mid-drag: time-marker-overlay.tsx:131-133 / test.tsx:206-208
  • A keyboard nudge stays in the view: tsx:94-96 / test.tsx:256
  • Lost capture without a cancel: tsx:67-68 / test.tsx:223
  • Dashed means unsettled: time-marker-overlay.scss:38, full-timeline.scss:101 and full-timeline.test.tsx:162
• Two names for one thing:
  • The waveform area is "graph" in the full-timeline files and "plot" in the overlay/model.
  • The × is "the x", "delete control" and "delete button".
• I left out a few wording-only nits.