feat: directional guidance towards svg element with mouse and touchscreen

[soundaryakp1999]

Pull Request

Description

When the user moves the mouse, auditory queues point them towards the nearest point in the curve.

Related Issues

Changes Made

Earlier behavior: the sonified (x,y) position is played if the mouse curser or touchscreen pointer touches any of the elements present in the svg. Nearby points provide no indication of proximity to the curve.
Current behavior: The relative direction to the nearest point from curser is represented with auditory notification. High pitch indicates that the point is above the curser. stereo panning to the right indicates that the point is to the right side. This is designed to guide the user towards the point, which will help the user physically draw the curve on the mouse, thus reinforcing a realistic shape of the curve.

Screenshots (if applicable)

Checklist

• I have read the Contributor Guidelines.
• I have performed a self-review of my own code and ensured it follows the project's coding standards.
• I have tested the changes locally following ManualTestingProcess.md, and all tests related to this pull request pass.
• I have commented my code, particularly in hard-to-understand areas.
• I have updated the documentation, if applicable.
• I have added appropriate unit tests, if applicable.

Additional Notes

[claude]

Code Review

Thanks for the contribution! The feature concept is well thought-out — using pitch and stereo pan to guide users toward the nearest curve point is a meaningful accessibility improvement. Here are some issues to address before this is ready to merge.

---

Critical: DOM Access in the Model Layer

src/model/abstract.ts — getTouchGuidance

const bbox = nearest.element.getBoundingClientRect();
const centerX = bbox.x + bbox.width / 2;
const centerY = bbox.y + bbox.height / 2;

getBoundingClientRect() is a DOM/layout API and must not live in the Model layer. Per the MVVC architecture, the Model contains data and navigation only — no UI concerns. This should be moved to a Service that already has legitimate DOM access (e.g. HighlightService or a dedicated TouchGuidanceService).

---

Architecture: Service Calling Service Directly

src/service/keybinding.ts

const guidance = this.commandContext.context.getTouchGuidance(x, y);
this.commandContext.audioService.playTouchGuidance(guidance);

Mousebindingservice now calls AudioService directly, bypassing the Command → Observer chain that the rest of the codebase uses. User input (mouse/pointer) should translate into a Command, which then triggers model changes, which then notify Observers (AudioService included). Direct cross-service calls make the flow harder to follow and test, and this sets a precedent that undermines the architecture.

Suggested approach: compute guidance in the mouse listener, then route it through the existing command infrastructure rather than calling audioService directly.

---

Bug: Incomplete Resource Cleanup in stopAll

src/service/audio.ts — playTouchGuidanceBeep

this.activeAudioIds.set(audioId, oscillator); // only the oscillator is tracked

gainNode and stereoPanner are not stored in activeAudioIds. When stopAll() is called (e.g. on dispose()), only oscillator.disconnect() and oscillator.stop() run; the gain and panner nodes remain connected to the compressor, leaking Web Audio graph nodes until GC.

Either track all three nodes (the map already supports OscillatorNode[]), or disconnect the gain/panner inside the existing setTimeout callback before removing from the map.

---

Minor: Volume Floor Ignores Mute

const guidanceVolume = Math.max(this.volume * TOUCH_GUIDANCE_VOLUME, 0.05);

When this.volume is 0 (user has muted audio), guidance beeps still play at 0.05. If this is intentional (always-audible guidance), it should be documented explicitly; if not, the floor should be removed or conditional on intent.

---

Minor: Missing Blank Line in Interface

src/model/plot.ts — missing blank line between getTouchGuidance JSDoc and the next method (getExtremaTargets), inconsistent with the rest of the interface formatting.

---

No Tests

The checklist notes "I have added appropriate unit tests, if applicable" is unchecked. Given that getTouchGuidance has directional logic (above/below, left/right, on-curve detection) and playTouchGuidance has rate-limiting and interpolation logic, unit tests would be valuable and straightforward to write here.

---

Summary

Severity	Issue
Critical	DOM access (getBoundingClientRect) in Model layer
High	Mousebindingservice calling AudioService directly (architecture bypass)
Medium	gainNode/stereoPanner not tracked → not disconnected in stopAll()
Low	Volume floor of 0.05 when muted — clarify intent
Low	Missing blank line in plot.ts interface
Low	No unit tests for new directional logic

The directional guidance idea is solid and the audio implementation (frequency mapping, rate-limiting by distance, stereo pan) is well-structured. Addressing the architecture and DOM-access concerns will make this mergeable.

[soundaryakp1999]

I have addressed the feedback of claude bot