diff --git a/CLAUDE.md b/CLAUDE.md index ab2887c..968ff85 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -212,7 +212,7 @@ Every path below exists; keep this list in step with `src/` when adding modules. both at half the size in the label grey. Reads the pointer, or the selected feature when there is one - `AnnotationTables.js` - The three table columns, their headers, count chips - and the "Clear all annotations" footer + and the "Clear all" footer - `icons.js` - The button glyphs, drawn as inline SVG in `currentColor` so they follow the button's states as text does. A mode's glyph rides beside its word; a view control's replaces it, keeping the word in a visually diff --git a/docs/Control-Panel.md b/docs/Control-Panel.md index c2c1edb..0432f89 100644 --- a/docs/Control-Panel.md +++ b/docs/Control-Panel.md @@ -187,7 +187,7 @@ dark-on-white one does, which is the whole point of a plate over a halo. It is a threaded through the renderers, because selection changes far more often than the features do. -The sidebands column carries a footer holding **Clear all annotations**, on +The sidebands column carries a footer holding **Clear all**, on every page. It was trainer-only, on the reasoning that a student's work expires overnight anyway — but "it will be gone tomorrow" is no answer to a student who has mislabelled a gram and wants to start the exercise again today. Clearing is diff --git a/docs/Testing-Strategy.md b/docs/Testing-Strategy.md index 2cf6a5c..8aab8fe 100644 --- a/docs/Testing-Strategy.md +++ b/docs/Testing-Strategy.md @@ -69,7 +69,7 @@ tests drive real mouse and keyboard events against a debug page or a fixture in speed, pan/zoom (including wheel navigation and clamping), and cross-mode integration - **Persistence**: trainer/student context detection, save and restore, the - student 24-hour expiry, "Clear all annotations", and the warning shown when storage + student 24-hour expiry, "Clear all", and the warning shown when storage refuses a write - **Keyboard and focus**: instance focus, Tab navigation, arrow-key handling - **Lifecycle and API hygiene**: the debug-only `__test__*` surface, listener diff --git a/src/components/AnnotationTables.js b/src/components/AnnotationTables.js index 38c79cf..c471c85 100644 --- a/src/components/AnnotationTables.js +++ b/src/components/AnnotationTables.js @@ -118,7 +118,7 @@ function setCount(container, count) { } /** - * Put the "Clear all annotations" button at the foot of the sidebands column. + * Put the "Clear all" button at the foot of the sidebands column. * * On every page, trainer and student alike: a student whose annotations expire * overnight still wants to start today's exercise again today, and clearing is @@ -137,7 +137,7 @@ export function mountClearAllButton(instance, onClear) { const button = document.createElement('button') button.type = 'button' button.className = 'gram-frame-clear-btn' - button.textContent = 'Clear all annotations' + button.textContent = 'Clear all' button.title = 'Remove every cross, harmonic set and sideband set' button.addEventListener('click', event => { event.preventDefault() diff --git a/src/core/annotationCommit.js b/src/core/annotationCommit.js index 60e6b5f..2beac16 100644 --- a/src/core/annotationCommit.js +++ b/src/core/annotationCommit.js @@ -1,4 +1,5 @@ import { dispatch, markAnnotationsChanged } from './state.js' +import { refreshReadoutTarget } from '../components/CursorReadout.js' /* * Its own module rather than a function in `state.js`, because `state.js` sits @@ -14,7 +15,7 @@ import { dispatch, markAnnotationsChanged } from './state.js' * * The cadence every annotation change has to perform -- bump the revision so * the storage listener notices, refresh the owning panel or table, re-render - * the persistent overlay, dispatch -- was copy-pasted at sixteen call sites + * the persistent overlay, re-read the readout column, dispatch -- was copy-pasted at sixteen call sites * (R9-13). Four steps repeated by hand is four chances to forget one, and the * one that goes missing silently is the revision bump: everything still looks * right on screen and nothing is saved. @@ -41,5 +42,10 @@ export function commitAnnotationChange(instance, refreshPanel = null, dispatchOp instance.featureRenderer.renderAllPersistentFeatures() } + // The readout column reads the selected feature, so it goes stale with it: + // without this a dragged harmonic set showed the spacing it had at the press + // until it was deselected (issue #344). + refreshReadoutTarget(instance) + dispatch(instance, dispatchOptions) } diff --git a/src/core/selectionTarget.js b/src/core/selectionTarget.js index 0552290..87dde08 100644 --- a/src/core/selectionTarget.js +++ b/src/core/selectionTarget.js @@ -25,16 +25,17 @@ import { normalizeMarkerLabel } from '../utils/markerLabel.js' * @typedef {Object} SelectionDescription * @property {string} label - What to call it: a marker's own label, or its family and position * @property {number} time - Where it sits on the time axis, seconds - * @property {number} freq - The frequency it is about: a marker's own, a - * sideband set's fundamental, a harmonic set's spacing + * @property {number} freq - The frequency it is about: a marker's own, or a + * pin set's spacing */ /** * Describe the selected feature, or null when nothing is selected. * * A marker has a time and a frequency outright. A pin set has an anchor time - * and, for its frequency, the number that set is *about*: the fundamental an - * analyst placed for a sideband set, the spacing that defines a harmonic one. + * and, for its frequency, its spacing: the number a drag of any member but a + * sideband set's origin changes, and the one an analyst is measuring + * (issue #344). * @param {GramFrame} instance - GramFrame instance * @returns {SelectionDescription|null} The selection, or null */ @@ -63,5 +64,5 @@ export function describeSelection(instance) { } const set = (sidebands ? sidebands.sidebandSets : []).find(candidate => candidate.id === selection.selectedId) - return set ? { label: `Sidebands ${ordinal}`, time: set.anchorTime, freq: set.fundamentalFreq } : null + return set ? { label: `Sidebands ${ordinal}`, time: set.anchorTime, freq: set.spacing } : null } diff --git a/tests/harmonics-mode.spec.js b/tests/harmonics-mode.spec.js index 669fa20..6b81da5 100644 --- a/tests/harmonics-mode.spec.js +++ b/tests/harmonics-mode.spec.js @@ -164,6 +164,24 @@ test.describe('Harmonics Mode - creating and adjusting sets by drag', () => { expect(spacings[2]).toBeGreaterThan(spacings[1]) }) + test('the frequency readout shows the live spacing mid-drag, not the spacing at the press (issue #344)', async ({ gramFramePage }) => { + await gramFramePage.addHarmonicSet(30, 20) + const line = await gramFramePage.page.locator('.gram-frame-harmonic-line').first().boundingBox() + if (!line) throw new Error('no harmonic line was rendered to drag') + const readout = gramFramePage.freqLED.locator('.gram-frame-led-value') + + await gramFramePage.page.mouse.move(line.x + line.width / 2, line.y + line.height / 2) + await gramFramePage.page.mouse.down() + await gramFramePage.page.mouse.move(line.x + line.width / 2 + 80, line.y + line.height / 2, { steps: 4 }) + + // Read while the button is still down: the release is not what refreshes it. + const spacing = (await gramFramePage.getState()).harmonics.harmonicSets[0].spacing + expect(spacing, 'the drag changed the spacing').not.toBeCloseTo(20, 1) + await expect(readout).toHaveText(spacing.toFixed(2)) + + await gramFramePage.page.mouse.up() + }) + test('dragging an existing set re-spaces it', async ({ gramFramePage }) => { // Created through the helper, so the starting spacing is exact and the // change below is measured against a known number. diff --git a/tests/sideband-mode.spec.js b/tests/sideband-mode.spec.js index 52bd9e7..40e82cc 100644 --- a/tests/sideband-mode.spec.js +++ b/tests/sideband-mode.spec.js @@ -133,6 +133,23 @@ test.describe('Sidebands mode', () => { expect(updated.fundamentalFreq).toBeCloseTo(50, 5) }) + test('the frequency readout shows the live spacing mid-drag, not the fundamental (issue #344)', async ({ gramFramePage }) => { + await gramFramePage.addSidebandSet(30, 50, 10) + const readout = gramFramePage.freqLED.locator('.gram-frame-led-value') + + // Mid-gesture: no onSetDragEnd, so nothing but the drag itself refreshes it. + await gramFramePage.page.evaluate(() => { + // @ts-ignore - test-only global + const instance = window.GramFrame.__test__getInstances()[0] + const sideband = /** @type {any} */ (instance.modes['sideband']) + const target = sideband.findSetTarget({ freq: 60, time: 30 }) + sideband.onSetDragStart(target) + sideband.onSetDragUpdate(target, { freq: 70, time: 30 }, { freq: 60, time: 30 }) + }) + + await expect(readout).toHaveText('20.00') + }) + test('dragging the fundamental moves the origin and leaves the spacing alone', async ({ gramFramePage }) => { const setId = await gramFramePage.addSidebandSet(30, 50, 10)