[kswenson]

CLUE-720

The Timeline tile now has an ⓘ button beside its title that opens a "Zooming and Moving" modal. The modal explains how to get around the graph: the zoom and pan buttons, click and shift+click to zoom, click+hold+drag to pan, and the Full Timeline. The text and button states follow the Zeplin spec.

The Timeline tile is also reachable by keyboard for the first time. Select it and press Enter, and Tab moves through the title, the info button, Prev, Next, the Full Timeline's scrollbar thumb, the toolbar, the drag handle and the resize handle, skipping any that are disabled; Shift+Tab walks the same cycle backwards. The thumb already pans the view with the arrow keys, Page Up/Down, Home and End (CLUE-719); this makes it reachable.

• The modal opens with focus on OK, so Enter or Space closes it, as do the close button and Escape. Focus then returns to the info button.
• Screen readers announce the modal's text on open: it is the dialog's accessible description.
• The info button's colors are CLUE's teal palette, sampled from Zeplin: teal-light-5 at rest, light-4 on hover and keyboard focus, light-3 while pressed, plus the standard focus ring.
• When Next reaches the last event (or Prev the first) and disables itself, focus moves to the other one rather than dropping out of the tile.
• While the title is being edited, the info button moves clear of the edit box, which is wider than a short title.

Departures from the spec

• OK is white at rest, as drawn. That means it isn't the modal's default button, which custom-modal.scss always fills teal.
• The modal is centered in the window, like every other CLUE modal, not over the tile as Zeplin draws it.
• Because focus starts on OK, OK opens in its focus state (teal with a ring), even after a mouse click. The shared modal styles use :focus rather than :focus-visible.

Implementation

• New TimelineInfoButton component, built on useCustomModal.
• BasicEditableTileTitle renders children inside the title area, so a tile can add a control beside its title. The button is positioned absolutely, so the title stays centered.
• useCustomModal takes an opt-in describeContent prop that points aria-describedby at the modal content. It's opt-in because a form dialog's controls are announced as they're reached.
• The Timeline tile registers with the tile focus trap through ClueTileAccessibilityBridge, as dataflow does:
  • the editable title is the title slot;
  • the info button is the topbar slot;
  • .timeline-container is the content slot, holding Prev, Next and the scrollbar thumb. It is reported only while one of them is enabled, since an empty slot would make Tab stop and go nowhere.
  • A focusContent enters the slot at its first (or, going backwards, last) enabled control. Left to itself, the trap enters at the slot's first tabindex="0" element, which is the thumb, and skips Prev and Next.
• In an editable tile, the graph's event-number labels and pinned time-marker label get tabIndex={-1}, so they stay out of the Tab cycle until the graph has a keyboard design; Prev and Next select events by keyboard. A read-only tile has no focus trap, so they stay in its natural Tab order.
• DynamicScrollbar stops the keys it handles from propagating. Otherwise ArrowUp on the thumb would also trigger the tile's ArrowUp exit and send focus out of the tile.
• The tile toolbar handles Tab itself, and its route never visited the topbar slot, so Shift+Tab from the toolbar skipped the info button. It now visits the topbar in both directions, matching the trap's cycle order.
• info-icon.svg has a class on its inner fill so CSS can set the button-state colors.
• useCustomModal builds its title and content ids with useId, so each modal's aria-labelledby and aria-describedby point at its own elements. They were built from dataTestId or className, so every modal of a class shared them.

Not in this PR

• The graph and the Full Timeline strip aren't reachable by keyboard yet. They need a design for what the keys do there; zoom and pan already work from the toolbar and the scrollbar thumb. That's CLUE-724.
• Focus on a disabled toolbar button is faded. The toolbar's disabled style fades the whole button, focus outline included, so on a Timeline with no data the toolbar's tab stop is easy to miss. That's toolbar-wide and filed as CLUE-723.
• Found in self-review and left for later:
  • Prev and Next don't announce the newly selected event to screen readers; the graph click and the scrollbar do. A live region on the event label would fix it.
  • The help text doesn't mention the scrollbar thumb's keyboard controls, and the button also shows on read-only tiles, where the graph isn't interactive.
  • The modal's Close button (shared by every CLUE modal) shows focus only as a background change of about 1.15:1, below the 3:1 a focus indicator needs.
  • A very long title or a narrow tile can push the info button past the tile's right edge.
  • The pressed-state fill (teal-light-3, from Zeplin) puts the ⓘ glyph under 3:1 for the moment it's held.

Testing

• Jest:
  • TimelineInfoButton: opening, the modal text and its accessible description, initial focus on OK, closing with OK, the close button and Escape, and focus returning to the info button.
  • Timeline tile: the info button in the title area; a real FocusTrapController walking title → info → Prev → Next → thumb and back; focus moving to the other button when Prev or Next disables itself, and left alone when an unfocused Next is clicked; no registration when read-only.
  • DynamicScrollbar keeps the keys it handles from its container and passes the rest on; the graph's labels are out of the Tab order in an editable tile and in it in a read-only one.
  • useCustomModal: aria-describedby absent by default, and pointing at the content with describeContent; two open modals of the same class each labelled and described by their own elements.
  • Tile toolbar: Tab and Shift+Tab reach the topbar slot, Tab prefers the title over it, and a topbar of several controls is entered at the right end.
  • I ran the timeline, toolbar, tile-component, dataflow, ui, hooks and modal suites; CI runs the rest.
• Manually, in /editor/ with a one-tile Timeline document without data: the button's position and states, the modal against Zeplin, and focus returning to the button on close.
• In headless Chrome (a throwaway Cypress spec, not committed) with a Timeline that has three events and a seismogram, using real key presses:
  • Tab: title → info → Prev → Next → thumb → toolbar → drag handle → resize handle → title, and the reverse with Shift+Tab;
  • Enter on Next at the last event leaves focus on Prev;
  • while the title is being edited, the button sits 4px right of the edit box, and Tab out of the box lands on it.
• Branch preview: https://collaborative-learning.concord.org/branch/timeline-info-modal/?appMode=qa&fakeClass=673&fakeUser=student:1&qaGroup=1&unit=seismic&problem=1.1
  1. Add a Wave Runner tile (second toolbar button), pick a station and the Compact Model, set a short date range, and click Timeline It!.
  2. Click the ⓘ beside the Timeline's title.
  3. Click the graph to zoom in, then select the tile, press Enter, and Tab and Shift+Tab around it. On the scrollbar thumb, the arrow keys (ArrowUp included) should pan the view without leaving the tile.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.17%. Comparing base (32268e2) to head (e82529f).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3030      +/-   ##
==========================================
+ Coverage   87.14%   87.17%   +0.02%     
==========================================
  Files        1063     1064       +1     
  Lines       60939    61003      +64     
  Branches    16272    16294      +22     
==========================================
+ Hits        53103    53177      +74     
+ Misses       7812     7802      -10     
  Partials       24       24              

Flag	Coverage Δ
cypress	?
cypress-regression	70.15% <15.00%> (-0.11%)	⬇️
cypress-smoke	40.40% <11.66%> (-0.04%)	⬇️
jest	62.03% <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 #20644

Run Properties:   Passed #20644  •  e82529f3fc: Address review of the Timeline info button [CLUE-720]

Project	collaborative-learning
Branch Review	CLUE-720-timeline-info-modal
Run status	Passed #20644
Run duration	03m 44s
Commit	e82529f3fc: Address review of the Timeline info button [CLUE-720]
Committer	Kirk Swenson
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 ↗︎

[Copilot]

🟢 Approval recommended

The accessibility behavior is coherent, localized, and supported by comprehensive targeted tests.

0 open findings

What changed in this PR

Adds Timeline help content and keyboard navigation while extending shared modal and focus-trap infrastructure.

Changes:

• Adds the “Zooming and Moving” information modal.
• Integrates Timeline controls into tile keyboard navigation.
• Improves modal descriptions and scrollbar key handling.

File	Description
src/​plugins/​timeline/​components/​timeline-tile.tsx	Registers Timeline focus slots and info button.
src/​plugins/​timeline/​components/​timeline-tile.test.tsx	Tests focus-slot registration.
src/​plugins/​timeline/​components/​timeline-info-button.tsx	Implements the information modal trigger.
src/​plugins/​timeline/​components/​timeline-info-button.test.tsx	Tests modal behavior and accessibility.
src/​plugins/​timeline/​components/​timeline-info-button.scss	Styles the button and modal.
src/​plugins/​timeline/​components/​time-marker-overlay.tsx	Excludes marker controls from Tab order.
src/​plugins/​timeline/​components/​time-marker-overlay.test.tsx	Verifies marker tab behavior.
src/​plugins/​timeline/​components/​event-overlay.tsx	Excludes graph labels from Tab order.
src/​plugins/​timeline/​components/​event-overlay.test.tsx	Verifies event-label tab behavior.
src/​plugins/​timeline/​assets/​info-icon.svg	Adds the styleable information icon.
src/​hooks/​use-custom-modal.tsx	Adds opt-in dialog content descriptions.
src/​hooks/​use-custom-modal.test.tsx	Tests aria-describedby behavior.
src/​components/​ui/​dynamic-scrollbar.tsx	Stops handled keyboard events from bubbling.
src/​components/​ui/​dynamic-scrollbar.test.tsx	Tests scrollbar event propagation.
src/​components/​toolbar/​tile-toolbar.tsx	Routes Tab navigation through topbar controls.
src/​components/​toolbar/​tile-toolbar.test.tsx	Tests topbar focus routing.
src/​components/​tiles/​basic-editable-tile-title.tsx	Supports controls beside tile titles.

🧠 Review effort: Balanced

---

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

[emcelroy]

Looks good 👍 A Claude-generated review raised the issues below to consider, none of which are major.

---

1. The hadFocus check has no test

In handleSelectEvent (timeline-tile.tsx:48), the refocus runs only if the clicked button had focus. I removed that check and all tests still passed.

The test is cheap to write. In jsdom (the fake browser Jest uses), a click does not move focus. So a test can click Prev or Next without focusing it first, and then check that focus did not move.

2. Does the "before the trap is entered" test describe a real state?

The test "keeps the contents in the focus cycle before the trap is entered" works like this:

1. It turns the trap off, which sets the tile's controls to tabindex="-1".
2. It asks the tile for its slots.
3. It calls focusContent directly.

I checked how the library enters the trap. On Enter, on Tab on the tile, and when focus arrives from outside, the trap first restores the controls' tabindex. Only then does it ask for the slots and call focusContent. So I found no path where the tile is asked for its slots while its controls are still at -1. If no such path exists, two things follow:

• The test checks a state the app never reaches.
• This line in the PR description has no real case behind it: "judged from the controls' disabled state, not their tabindex, which the trap holds at -1 until it is entered".

Limit: TileComponent also reads the slots in other places (for example, at tile-component.tsx:532, when it builds the trap). I did not trace whether any of those reads happen while the controls are at -1.

If there is a real path where the tile reports its slots while its controls are at -1, add a comment to the test that names it. If not, rename or remove the test, and update the PR description line.

Optional

Commit the full keyboard-walk test

This is worth doing only if this PR is meant to give end-to-end keyboard coverage. The Jest Tab walk covers the Timeline's own slots in both directions. It has no toolbar, drag handle or resize handle, and the toolbar tests use stand-in elements. The PR says the full walk was checked in a Cypress spec that was not committed. I confirmed most of that walk in the browser.

Remove the unreachable fallback to the info button

At timeline-tile.tsx:43-44, the ?? .timeline-info-button fallback cannot run:

• When Prev or Next disables itself, another content control is still enabled.
• With two or more events, the other button is enabled.
• With a seismogram, the thumb is enabled. In an editable tile it is always enabled, because full-timeline.tsx passes disabled={readOnly}.

Removing the fallback has two other effects:

• The .timeline-info-button selector no longer appears twice in the file (it is also on line 116).
• The comment above the effect ("the first control still enabled") becomes exact.

Wording in tests and comments

• "puts the scrollbar thumb in the focus cycle whenever there's seismogram data" checks only that the container is reported and holds the thumb. It does not Tab to the thumb. Also, "whenever" is too strong, because a read-only tile has no focus cycle.
• "A focus trap wired to the tile's API as TileComponent wires it" — the helper copies only the title, topbar, content and focusContent parts. "As TileComponent wires these slots" would be exact.
• The tryFocusTopbar comment in tile-toolbar.tsx says "or the slot element itself when it is the only control". The code falls back to the slot element when the slot has no focusable children. "When it has no focusable children" would be exact.

PR description

• "info-icon.svg gains a class on its inner fill" suggests that the file already existed. It is new on this branch (added in 74a4aacd9). Suggested wording: "info-icon.svg has a class on its inner fill so CSS can set the button-state colors."

Follow-up (separate ticket)

Modal IDs are not unique

use-custom-modal.tsx:107 builds contentId from dataTestId or className. That is the same way the existing titleId is built. So every Timeline's modal gets the same ids.

• react-modal-hook can render more than one modal. If two ever render together, aria-describedby and aria-labelledby can point to the wrong dialog.
• In normal use the open modal covers the page, so a second Timeline modal cannot be opened.
• The id scheme existed before this branch.

An id from React's useId would fix both ids.

Automatic findings that I checked and dismissed

Finding	Result
Shift+Tab from the toolbar lands on an event label when the thumb is disabled	Cannot happen. The thumb is disabled only in a read-only tile, and a read-only tile has no focus trap. In the browser, Shift+Tab went to the thumb.
Focus is lost when Prev/Next disables itself and no fallback exists	Cannot happen. Another control is always enabled.
Focus drops to body for a moment before the refocus	Not seen in Chrome. Focus went straight from Next to Prev.
tryFocusTopbar breaks Dataflow's roving tabindex	Wrong premise. Dataflow's topbar has no roving tabindex. Dataflow also has a title and a palette, which the toolbar's Tab code tries before the topbar. (Checked in the code only. The Dataflow demo document did not load in /editor/.)
The labels read read-only from context, but the tile uses its readOnly input	Existing pattern. full-timeline.tsx and timeline-plot.tsx already read read-only from the context. Canvas is the only provider in the app.
The thumb blocks arrow, Page and Home/End keys even with Cmd, Ctrl or Alt held	No effect today. No shortcut uses those key combinations.

[kswenson]

@emcelroy Addressed in e82529f:

1. hadFocus test: added. It clicks Next without focusing it and checks focus stays put. Removing the check now fails it.
2. "Before the trap is entered" test: you were right. Every way into the trap restores the controls' tabindex before it reads the slots. The one read made while they're at -1 is in setChildrenNonTabbable, and it only reads the slots that have tabHandlers, which the Timeline has none of. I removed the test, the tabindex clause in the comment, and the matching line in the description.

Optional:

• Unreachable fallback: removed.
• Wording: fixed the test name, the helper comment and the tryFocusTopbar comment.
• PR description: fixed the info-icon.svg line.
• Modal ids: fixed here rather than in a follow-up. useCustomModal now builds both ids with useId. A new test opens two modals of the same class and checks that each one's aria-labelledby and aria-describedby point inside itself. Nothing referenced the old ids.
• Cypress keyboard walk: left out. The Jest walk covers the Timeline's own slots, and the toolbar route has its own unit tests, so a committed spec would add regression time for little gain.