Commit 301109b
feat: add auditory directional guidance towards svg element with mouse (#587)
* feat: guidance
* chore: address feedback
* chore: address follow-up review feedback
- Avoid double findNearestPoint per pointer event by merging
moveToPoint and getTouchGuidance into moveToPointAndGetPointerGuidance
- Move volume guard before Web Audio node creation in
playPointerGuidanceBeep to prevent orphan node allocation
- Rename TouchGuidance -> PointerGuidance for consistency (the feature
serves mouse and touch input)
- Document BarTrace.findNearestPoint hit-test asymmetry vs other traces
* chore: address PR review round 3
- Fix click hover mode regression: split into pointermove (guidance) and
click (navigate-only) listeners so clicks no longer emit guidance beeps;
added executeNavigateOnly on PointerGuidanceCommand
- Remove now-dead moveToPoint method from Trace interface, AbstractPlot,
AbstractTrace, Figure, and Subplot (no remaining callers since the
pointer flow uses moveToPointAndGetPointerGuidance)
- Dedupe clamp/interpolate: added to MathUtil, used in audio.ts and
pointerGuidance.ts
- Document why PointerGuidanceCommand is instantiated directly rather
than through CommandFactory
- Reword the "inverted pan" comment in pointerGuidance.ts to describe
intent ("pan toward the curve") rather than implementation
- Add unit tests for PointerGuidanceCommand.execute and
executeNavigateOnly covering with/without event and coordinates
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* chore: address PR review round 4
- Rename PointerGuidanceState fields to cursorVerticalPosition /
cursorHorizontalPosition; the new names anchor the reference frame
to the cursor so call sites read unambiguously
- Add PointerGuidanceCommand.reset() and route pointerleave + hover-mode
changes through it instead of execute() with no arg
- Restore full parameter list on Box/ViolinBox moveToNearest overrides
so the override matches the base contract at a glance
- Document AudioContext.currentTime behaviour when the context is
suspended (before first user gesture)
- Add unit tests for AudioService.playPointerGuidance covering
AudioMode.OFF skip, on-curve bypass, off-curve playback,
throttle window, throttle release after time advance, and reset
on null guidance
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* chore: address PR review round 5
- Decouple throttle reset from AudioMode in playPointerGuidance: null /
on-curve guidance still resets nextPointerGuidanceBeepAt, but
AudioMode.OFF no longer does, so re-enabling audio mid-hover does not
fire an immediate beep
- Add Scope.TRACE guard to PointerGuidanceCommand so guidance does not
fire while modals or other navigation scopes are active; pointer-leave
reset still runs regardless of scope to keep throttle state clean
- Document the 2x multiplier on the beep cleanup timeout (lets the
exponential ramp tail finish before disconnecting)
- Strengthen PointerGuidanceState docs to make the cursor-relative frame
explicit and harder to invert at call sites
- Add contract test for the moveToNearest hook used by BoxTrace and
ViolinBoxTrace, plus a new AudioService test verifying that
AudioMode.OFF preserves the throttle, plus scope-guard tests on the
command
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* chore: address review feedback (typing, contracts, comments)
- Tighten executeNavigateOnly parameter to PointerEvent | MouseEvent
so the call site signals the supported event shapes explicitly
- Make moveToNearest onCurve parameter non-optional and tighten
NearestPoint to non-null so subclass overrides must accept the
contract instead of silently recomputing or ignoring it
- Drop unused jest import + jest.fn() linter workaround in
moveToNearestOverride test
- Document tie-break behavior at the exact center for cursor
position relations
- Clarify that pointer guidance is intentionally mode-agnostic
across non-OFF audio modes
* chore: address PR review round 5
- Add StereoPannerNode feature detection in playPointerGuidanceBeep
so guidance degrades to mono on Safari < 14.5 instead of throwing
- Document the intentional asymmetric pointerleave listener
registration (only added for pointermove mode, not click mode)
- Add scatter-style "side effect before delegating" contract test
to lock down ScatterTrace.moveToNearest mode-switch semantics
- Drop redundant public modifier on PointerGuidanceCommand
constructor
- Replace `globalThis as any` cast in audio.pointerGuidance test
with a typed accessor
- Standardize on it() within describe() for new pointer-guidance
tests to match the lint rule
* chore: address PR review round 6
- Stop zeroing the guidance throttle on out-of-range pointer events
(resolvePointerGuidanceBeep returning null), which would otherwise
unblock a beep on every crossing of maxDistancePx
- Note in PointerGuidanceCommand that isInTraceScope duplicates
CommandExecutor.isValidForScope and must stay in sync
- Trim the over-long class/method JSDoc in PointerGuidanceCommand
- Add unit tests for MathUtil.clamp and MathUtil.interpolate
- Add coverage for the out-of-range throttle path
- Clarify the volume=100 settings mock in audio.pointerGuidance
* chore: address PR review round 7 — blockers
Resolves the two review items flagged as merge-blockers.
#3 Naming inversion (cursor frame → curve frame)
The field formerly named `cursorVerticalPosition` described where the
*cursor* sat, but the audio mapping inverts the frame: cursor below
the curve plays a *high* pitch because the point is above. A
contributor reading `cursorVerticalPosition: 'below'` would naturally
expect a low sound — the opposite of intended.
Renamed to describe the curve point's position relative to the cursor,
which lines up 1:1 with the audio output:
- `curveVertical: 'above'` → high pitch (point is up there)
- `curveVertical: 'below'` → low pitch (point is down there)
- `curveHorizontal: 'left'` → pan left (point is to the left)
- `curveHorizontal: 'right'` → pan right (point is to the right)
While here, also converted `PointerGuidanceState` to a discriminated
union — `distancePx` and the direction fields no longer exist on the
on-curve branch, so the previous "callers should check onCurve before
using this" JSDoc smell is now enforced by the type checker.
#2 Histogram / Segmented / Smooth `findNearestPoint` return type
Verified: none of `Histogram`, `SegmentedTrace`, `SmoothTrace`, or
`SmoothTraceSvgXY` override `findNearestPoint`. They inherit from
`AbstractBarPlot` / `LineTrace`, both of which return the updated
`NearestPoint` shape with `centerX` / `centerY`. Build passes as
independent confirmation.
* chore: address PR review round 8
#1 Throttle consumed at volume=0 (blocker)
playPointerGuidance advanced nextPointerGuidanceBeepAt unconditionally,
but playPointerGuidanceBeep returned early when volume*POINTER_GUIDANCE_VOLUME
was 0 — the throttle slot was burnt without a beep firing, so turning
volume back up left the user with one silent throttle window. Changed
playPointerGuidanceBeep to return a boolean and only bump the throttle
when a beep actually emitted. New test simulates volume=0 → 100 mid-flow
to lock the contract down.
#2 Scope mismatch leaves stale throttle armed
execute() now calls reset() on out-of-scope events too (not just on
missing event / missing coordinates), so a hover that started in
Scope.TRACE and continued through a scope change does not delay the
first beep after the user returns to trace scope. Updated the relevant
unit test.
#4 ARIA announcement on hover (verified, no change)
The hover-driven moveToIndex → observer notification flow predates this
PR — old AbstractTrace.moveToPoint already called moveToIndex when the
cursor was in bounds, with identical observer fanout. The refactor into
moveToNearest preserves the behavior 1:1, so screen-reader semantics are
unchanged.
#6 / #7 polish
Trimmed restate-the-type JSDoc on PointerGuidanceBeep / PointerGuidanceConfig
and noted in resolvePointerGuidanceBeep that "above" follows screen-space y.
* chore: clarify pointer-guidance scope-check rationale
PR review round 9.
Action item: extract the duplicated scope-check or document the link
to its peer.
There is no peer. The prior JSDoc cited a non-existent
`CommandExecutor.isValidForScope`; in this codebase keyboard commands
rely on hotkeys-js scope routing (only the active scope receives the
event, so commands don't self-validate), while DOM pointer listeners
receive every event regardless of scope. PointerGuidanceCommand is
the only path that needs a self-check, so there is nothing to keep
in sync. Rewrote the comment to describe the actual architecture
instead of inventing a sync hazard.
Action item: confirm type-check passes for all trace types.
`npm run build` (which runs vite's TS check + vite-plugin-dts rollup
across every trace) is green. `Histogram` and `SegmentedTrace`
inherit `findNearestPoint` from `AbstractBarPlot`; `SmoothTrace` /
`SmoothTraceSvgXY` inherit from `LineTrace` — no overrides in those
files, so the updated `NearestPoint` shape is picked up automatically.
Type formatting (leading `|` style): kept as-is. The convention matches
`FigureState`, `SubplotState`, and `TraceState` defined immediately
below in the same file.
* test(pointer-guidance): import jest globals explicitly
Future-proofs the test against a Jest config switch to
`injectGlobals: false`, and matches every other PR test file
which already imports describe/expect/it explicitly.
* chore: address PR review round 9
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* chore: address PR review round 10
- audio.ts (playPointerGuidanceBeep): scale exponential gain ramp target
with guidanceVolume so the fade-out stays below the start value at any
user volume (fixed 0.001 inverted the ramp at low volumes)
- audio.ts (playPointerGuidanceBeep): early-return when AudioContext state
is not 'running' to avoid scheduling start(0)/stop(0.06) on a suspended
context, which would fire an unexpected beep on resume
- audio.ts: use MathUtil.clamp directly at the new guidance pan callsite
- audio.ts: rewrite pointer-guidance JSDoc to use curve-relative framing
matching PointerGuidanceState.curveVertical/curveHorizontal semantics
- scatter.ts (moveToNearest): guard NavMode.COL mutation behind onCurve
so hover guidance probes no longer override a ROW mode set by keyboard
- scatter.ts: rewrite stale JSDoc that referenced the deleted moveToPoint
* chore: address PR review round 11
- audio.ts: stop resetting guidance throttle on onCurve. The
isPointInBounds boundary oscillates on/off at frame rate; resetting
on each on-curve frame let every following off-curve frame bypass the
rate limit, producing a 60 Hz buzz instead of discrete beeps. Only
null guidance (pointer left the trace) clears the throttle now.
- state.ts / abstract.ts / pointerGuidance.ts: add 'center' variant to
curveHorizontal for the x === centerX tie. Resolver returns pan = 0
for 'center' instead of falling through to 'left' and panning audio
away from a curve the cursor is already horizontally aligned with
(heatmap-center case).
- audio.ts: inline MathUtil.clamp/interpolate at all 7 call sites and
drop the private Range-object wrappers + Range interface.
- audio.ts / command/pointerGuidance.ts: drop JSDoc that restated what
the code does (the 'Behavior:' bullet list on playPointerGuidance and
the mechanical summary on PointerGuidanceCommand.execute) per
CLAUDE.md guidance against what-not-why comments.
- audio.ts / pointerGuidance.ts: drop the redundant
DEFAULT_POINTER_GUIDANCE_CONFIG argument at the only production
resolver call site; the default parameter already covers it.
- abstract.ts: reword 'puts the point below the cursor' to
'puts the curve center below the cursor' to remove ambiguity between
the curve point and the cursor itself.
- Tests: cover the throttle non-reset on onCurve, the curveHorizontal
'center' emission from abstract.ts, and the pan = 0 resolver result.
* chore: address PR review round 12
- pointerGuidance.ts: drop the redundant MathUtil.clamp in
resolvePointerGuidanceBeep. The early-return on line 38 guarantees
distancePx <= maxDistancePx and Math.hypot is non-negative, so the
ratio is already in [0, 1].
- bar.ts / box.ts / candlestick.ts / heatmap.ts / line.ts / scatter.ts /
violin.ts / violinBox.ts: use the named NearestPoint interface for
findNearestPoint return types. The inline structural types were
identical today but would silently drift if the contract grows a
required field; the named alias surfaces such changes at the override
signature instead of only at the return statement.
- audio.ts: cast stopAudioNode's argument to AudioScheduledSourceNode
(the common parent of OscillatorNode / AudioBufferSourceNode /
ConstantSourceNode) rather than OscillatorNode. Today only oscillators
reach this path, but the narrower cast would mislead a future
contributor adding a sample-based palette entry.
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>1 parent 7ce2b25 commit 301109b
23 files changed
Lines changed: 1446 additions & 163 deletions
File tree
- src
- command
- model
- service
- type
- util
- test
- command
- model
- service
- util
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
99 | 99 | | |
100 | 100 | | |
101 | 101 | | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
102 | 107 | | |
103 | 108 | | |
104 | 109 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| 13 | + | |
13 | 14 | | |
14 | 15 | | |
15 | 16 | | |
| |||
53 | 54 | | |
54 | 55 | | |
55 | 56 | | |
| 57 | + | |
| 58 | + | |
56 | 59 | | |
57 | 60 | | |
58 | 61 | | |
| |||
281 | 284 | | |
282 | 285 | | |
283 | 286 | | |
284 | | - | |
285 | | - | |
286 | | - | |
287 | | - | |
288 | | - | |
289 | | - | |
290 | | - | |
291 | | - | |
292 | | - | |
293 | | - | |
294 | | - | |
295 | | - | |
296 | | - | |
297 | | - | |
298 | 287 | | |
299 | 288 | | |
300 | 289 | | |
| |||
311 | 300 | | |
312 | 301 | | |
313 | 302 | | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
314 | 322 | | |
315 | 323 | | |
316 | 324 | | |
| |||
795 | 803 | | |
796 | 804 | | |
797 | 805 | | |
798 | | - | |
799 | | - | |
800 | | - | |
| 806 | + | |
| 807 | + | |
| 808 | + | |
| 809 | + | |
| 810 | + | |
| 811 | + | |
| 812 | + | |
| 813 | + | |
| 814 | + | |
| 815 | + | |
801 | 816 | | |
802 | | - | |
| 817 | + | |
| 818 | + | |
| 819 | + | |
| 820 | + | |
803 | 821 | | |
804 | | - | |
805 | | - | |
806 | | - | |
807 | | - | |
808 | | - | |
809 | | - | |
810 | | - | |
811 | | - | |
| 822 | + | |
| 823 | + | |
| 824 | + | |
| 825 | + | |
| 826 | + | |
| 827 | + | |
| 828 | + | |
| 829 | + | |
| 830 | + | |
| 831 | + | |
| 832 | + | |
| 833 | + | |
| 834 | + | |
| 835 | + | |
| 836 | + | |
| 837 | + | |
| 838 | + | |
| 839 | + | |
| 840 | + | |
| 841 | + | |
| 842 | + | |
| 843 | + | |
| 844 | + | |
| 845 | + | |
| 846 | + | |
| 847 | + | |
| 848 | + | |
| 849 | + | |
| 850 | + | |
| 851 | + | |
| 852 | + | |
| 853 | + | |
| 854 | + | |
| 855 | + | |
| 856 | + | |
| 857 | + | |
| 858 | + | |
| 859 | + | |
| 860 | + | |
| 861 | + | |
| 862 | + | |
| 863 | + | |
| 864 | + | |
| 865 | + | |
| 866 | + | |
| 867 | + | |
| 868 | + | |
| 869 | + | |
| 870 | + | |
| 871 | + | |
| 872 | + | |
| 873 | + | |
| 874 | + | |
| 875 | + | |
812 | 876 | | |
| 877 | + | |
813 | 878 | | |
814 | 879 | | |
815 | 880 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
| 5 | + | |
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| |||
239 | 239 | | |
240 | 240 | | |
241 | 241 | | |
242 | | - | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
243 | 252 | | |
244 | 253 | | |
245 | 254 | | |
246 | 255 | | |
247 | 256 | | |
248 | 257 | | |
249 | 258 | | |
250 | | - | |
| 259 | + | |
251 | 260 | | |
252 | 261 | | |
253 | 262 | | |
| |||
284 | 293 | | |
285 | 294 | | |
286 | 295 | | |
287 | | - | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
288 | 303 | | |
289 | 304 | | |
290 | 305 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
4 | | - | |
| 4 | + | |
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| |||
496 | 496 | | |
497 | 497 | | |
498 | 498 | | |
499 | | - | |
| 499 | + | |
500 | 500 | | |
501 | 501 | | |
502 | 502 | | |
| |||
521 | 521 | | |
522 | 522 | | |
523 | 523 | | |
| 524 | + | |
| 525 | + | |
524 | 526 | | |
525 | 527 | | |
526 | 528 | | |
527 | 529 | | |
528 | | - | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
529 | 536 | | |
530 | | - | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
531 | 543 | | |
532 | 544 | | |
533 | 545 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| |||
1046 | 1046 | | |
1047 | 1047 | | |
1048 | 1048 | | |
1049 | | - | |
| 1049 | + | |
1050 | 1050 | | |
1051 | 1051 | | |
1052 | 1052 | | |
| |||
1072 | 1072 | | |
1073 | 1073 | | |
1074 | 1074 | | |
| 1075 | + | |
| 1076 | + | |
1075 | 1077 | | |
1076 | 1078 | | |
1077 | 1079 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
420 | 420 | | |
421 | 421 | | |
422 | 422 | | |
423 | | - | |
| 423 | + | |
| 424 | + | |
424 | 425 | | |
425 | | - | |
426 | | - | |
427 | | - | |
428 | | - | |
429 | | - | |
430 | | - | |
431 | | - | |
432 | | - | |
433 | | - | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
434 | 429 | | |
435 | | - | |
436 | | - | |
| 430 | + | |
| 431 | + | |
437 | 432 | | |
438 | 433 | | |
439 | 434 | | |
| |||
0 commit comments