diff --git a/CLAUDE.md b/CLAUDE.md index 448ccf6..8409624 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -263,7 +263,8 @@ There is no visual/screenshot regression testing — see ### Mode-Specific Features - **Pan Mode**: The default mode; drag to pan when zoomed in, so a first click never places anything - **Analysis Mode**: Persistent draggable markers with cross-mode visibility and optional - haloed text labels (upper-right of a crosshair, centred above a shaped symbol) + haloed text labels (upper-right of a crosshair, centred above a shaped symbol — + below an upward-pointing triangle, whose apex points at the data above it) - **Harmonics Mode**: Real-time harmonic calculation and display - **Sidebands Mode**: A pin set with a user-placed origin — the fundamental — with members spread each side of it and labelled by signed offset diff --git a/docs/Gram-Modes.md b/docs/Gram-Modes.md index 99883e3..fedc7c1 100644 --- a/docs/Gram-Modes.md +++ b/docs/Gram-Modes.md @@ -58,7 +58,9 @@ events, broadband pulses or ambient shifts in sonar data. entered — and clearing the field removes the label again. - A label is drawn on the gram in black inside a white halo, so it reads over both dark and light pixels: in the upper-right quadrant of a crosshair marker, - or centred above a marker that carries a shaped symbol. + or centred above a marker that carries a shaped symbol. The one exception is + the upward-pointing triangle, which is aimed at the gram above it — its label + is centred *below* the symbol so the data being marked stays visible. - The table's **Label** column shows labels of five characters or fewer in full, and abbreviates anything longer to its first three characters plus `..`. The full text stays on the gram and in the dialog. @@ -85,6 +87,10 @@ multiples of a spacing. are drawn within the visible span, at a regular "nice" interval, so a small spacing over a wide span stays legible. The pins themselves are not thinned, so the unlabelled harmonics still show against the data. +- Each harmonic number sits above its symbol — except for a set drawn with the + upward-pointing triangle, whose apex points at the gram above it. There the + number hangs below the symbol instead, leaving that data clear; the grab region + follows the digits, so the set is still grabbable wherever it is drawn. - Sets are listed in the harmonics panel; selecting a row enables arrow-key adjustment and in-place restyling, as for markers. diff --git a/src/modes/shared/PinSetMode.js b/src/modes/shared/PinSetMode.js index 0db73cf..448f908 100644 --- a/src/modes/shared/PinSetMode.js +++ b/src/modes/shared/PinSetMode.js @@ -32,7 +32,7 @@ import { import { BaseDragHandler } from './BaseDragHandler.js' import { getUniformTolerance } from '../../utils/tolerance.js' import { sampledHarmonics } from '../../utils/harmonicSampling.js' -import { createSymbolMark, resolveSymbolScale } from '../../rendering/symbols.js' +import { createSymbolMark, labelSitsBelowSymbol, resolveSymbolScale } from '../../rendering/symbols.js' import { applyTextHalo } from '../../utils/svg.js' /** @@ -592,9 +592,10 @@ export class PinSetMode extends BaseMode { // Only the thinned subset is labelled, so only those pins carry a stack. const labelled = this.labelledIndices(minIndex, maxIndex) // A hidden pin draws mini-pins instead of full lines, so its line grab - // region is the mini-pin stub hanging from the stack's underside. + // region is the mini-pin stub hanging from the symbol's underside — which + // is where the renderer hangs it, label placement notwithstanding. const pinDrawn = set.showPin !== false - const lineFrom = pinDrawn ? lineTop : stack.bottom + const lineFrom = pinDrawn ? lineTop : stack.symbolBottom const lineTo = lineFrom + (pinDrawn ? lineHeight : PinSetMode.MINI_PIN_HEIGHT) const tolerance = getUniformTolerance(this.getViewport(), this.instance.ui.spectrogramImage) @@ -720,6 +721,12 @@ export class PinSetMode extends BaseMode { * (label + symbol) is nudged down by the overflow so it stays legible * (spec 159, FR-011). * + * An upward-pointing triangle inverts the label (issue #242): its apex points + * at the gram above the pin, so a number stacked over it hides exactly the + * data the set was placed against. That label drops to the symbol's underside + * instead, over the pin line's top — ink the set already spends there. The + * symbol keeps capping the line either way, so the pin's anchor never moves. + * * @param {number} lineTop - Top Y position of the pin lines (SVG coords) * @param {number} imageTop - Top edge of the spectrogram image in SVG coords * @param {PinSet} set - Set being laid out (its symbol size drives the stack) @@ -729,16 +736,20 @@ export class PinSetMode extends BaseMode { const r = this.symbolSize(set) / 2 const gap = PinSetMode.LABEL_GAP const fontSize = PinSetMode.LABEL_FONT_SIZE + const below = labelSitsBelowSymbol(set.symbol) - // Symbol caps the line; label baseline sits just above the symbol. + // Symbol caps the line; the label baseline sits just above the symbol, or — + // for an up-pointing triangle — a whole line of text below it, so the + // glyphs (which hang above their baseline) clear the mark. let symbolCy = lineTop - r - let labelY = symbolCy - r - gap + let labelY = below ? symbolCy + r + gap + fontSize : symbolCy - r - gap - // Keep the top of the label (approx one ascent above its baseline) on-screen. - const labelTop = labelY - fontSize + // Keep the top of the stack on-screen: the label's approximate ascent when + // it leads the stack, the symbol's top edge when the label hangs below. + const stackTop = below ? symbolCy - r : labelY - fontSize const minTop = imageTop + PinSetMode.STACK_TOP_PAD - if (labelTop < minTop) { - const shift = minTop - labelTop + if (stackTop < minTop) { + const shift = minTop - stackTop symbolCy += shift labelY += shift } @@ -751,23 +762,34 @@ export class PinSetMode extends BaseMode { * * Derived from the same {@link PinSetMode#labelStackPositions} layout the * renderer uses, so the grab region tracks the drawn stack — including the - * downward nudge applied near the image's top edge. The bottom is clamped to - * the pin line's top so the stack region and the line region always meet with - * no dead gap between them. + * downward nudge applied near the image's top edge, and the label's drop to + * the underside of an up-pointing triangle (issue #242): move the text and + * the hotspot moves with it. The bottom is clamped to the pin line's top so + * the stack region and the line region always meet with no dead gap between + * them. + * + * `symbolBottom` is reported separately because it, not the region's bottom, + * is where a mini-pin hangs from — a label drawn below the symbol pushes the + * region past the stub it would otherwise anchor. * * @param {number} lineTop - Top Y position of the pin lines (SVG coords) * @param {PinSet} set - Set being hit-tested - * @returns {{top: number, bottom: number}} Top and bottom Y of the stack region + * @returns {{top: number, bottom: number, symbolBottom: number}} Stack region and the symbol's underside */ labelStackBounds(lineTop, set) { const imageTop = getImageBounds(this.getViewport(), this.instance.ui.spectrogramImage).top const { symbolCy, labelY } = this.labelStackPositions(lineTop, imageTop, set) const r = this.symbolSize(set) / 2 + const below = labelSitsBelowSymbol(set.symbol) + const symbolBottom = symbolCy + r return { - // One ascent above the label's baseline is the top of the characters. - top: labelY - PinSetMode.LABEL_FONT_SIZE, - bottom: Math.max(lineTop, symbolCy + r) + // One ascent above the label's baseline is the top of the characters — + // unless the label hangs below, in which case the symbol leads the stack. + top: below ? symbolCy - r : labelY - PinSetMode.LABEL_FONT_SIZE, + // The baseline is the underside of the characters when they trail. + bottom: Math.max(lineTop, below ? labelY : symbolBottom), + symbolBottom } } @@ -847,6 +869,8 @@ export class PinSetMode extends BaseMode { * Centred horizontally on the pin's line (`text-anchor: middle` at `lineX`) and * positioned above the pin's symbol (baseline at `labelY`), so the vertical * stack over a pin reads label -> symbol -> line (spec 159, FR-009/FR-010). + * {@link PinSetMode#labelStackPositions} owns that baseline, so a set whose + * symbol carries its label underneath needs nothing special here. * * The characters are drawn black inside a white halo rather than in the set's * colour: a single colour is only legible over part of a gram, whereas the diff --git a/src/rendering/labels.js b/src/rendering/labels.js index a604e6a..0bd12b2 100644 --- a/src/rendering/labels.js +++ b/src/rendering/labels.js @@ -7,25 +7,20 @@ * symbol colour; the label text deliberately is not colour-coded. * * Where the label goes is `markerLabelPlacement`'s decision (see - * `utils/markerLabel.js`, where it stays pure and unit-testable). This module - * only builds the element. + * `utils/markerLabel.js`, where it stays pure and unit-testable) — including + * the drop below an upward-pointing triangle, whose apex points at the data + * above it (issue #242). This module only builds the element, at the font size + * that rule sizes its gaps from. */ /// import { applyTextHalo } from '../utils/svg.js' -import { markerLabelPlacement } from '../utils/markerLabel.js' +import { MARKER_LABEL_FONT_SIZE, markerLabelPlacement } from '../utils/markerLabel.js' /** SVG namespace for element creation */ const SVG_NS = 'http://www.w3.org/2000/svg' -/** - * Label font size in px. Matches the harmonic-number labels so all in-gram text - * reads as one family. - * @type {number} - */ -const MARKER_LABEL_FONT_SIZE = 12 - /** * Build a marker's label as a detached SVG text element. * diff --git a/src/rendering/symbols.js b/src/rendering/symbols.js index d3d0534..f12acc0 100644 --- a/src/rendering/symbols.js +++ b/src/rendering/symbols.js @@ -92,6 +92,30 @@ export function resolveSymbolType(symbolType) { : DEFAULT_SYMBOL } +/** + * Whether a symbol's text label belongs BELOW the mark rather than above it. + * + * Every other symbol is symmetric about its centre, or points downwards, so the + * space above it is free and the label goes there. An upward-pointing triangle + * is the exception (issue #242): it is drawn to point AT something above it, so + * a label stacked over its apex sits exactly on the data the analyst placed it + * against. For that one symbol the label drops to the underside, where the + * triangle's own base — and, on a pin, the line hanging from it — already + * covers the gram. + * + * Callers that lay out a label MUST also move the label's grab region to match, + * or the hotspot parts company with the drawn text. + * + * Pure: no DOM, no state. Unknown/absent values resolve through + * {@link resolveSymbolType} first, so they follow the default (`cross`, above). + * + * @param {SymbolType|string|null|undefined} symbolType - Candidate symbol id + * @returns {boolean} True when the label is drawn beneath the symbol + */ +export function labelSitsBelowSymbol(symbolType) { + return resolveSymbolType(symbolType) === 'triangle' +} + /** * Build a `points` attribute string from an array of [x, y] pairs. * @param {Array<[number, number]>} pts - Point pairs diff --git a/src/utils/markerLabel.js b/src/utils/markerLabel.js index f07e729..3a28dc2 100644 --- a/src/utils/markerLabel.js +++ b/src/utils/markerLabel.js @@ -15,7 +15,7 @@ /// -import { resolveSymbolType } from '../rendering/symbols.js' +import { labelSitsBelowSymbol, resolveSymbolType } from '../rendering/symbols.js' /** * Longest label accepted. Long enough for a ship name or a contact @@ -100,6 +100,17 @@ const QUADRANT_GAP = 5 */ const ABOVE_SYMBOL_GAP = 4 +/** + * Label font size in px. Doubles as the approximate ascent when the label hangs + * BELOW a symbol, where the baseline has to clear the symbol by a whole line of + * text rather than sit just above it. + * + * Lives here rather than in `rendering/labels.js` so the placement rule and the + * element that obeys it read the same number; the renderer imports it back. + * @type {number} + */ +export const MARKER_LABEL_FONT_SIZE = 12 + /** * Where a marker's label goes, given what the marker draws. * @@ -108,7 +119,10 @@ const ABOVE_SYMBOL_GAP = 4 * empty quadrants — the label goes in the upper-right one, clear of both * arms; * - a marker with a shaped symbol has no free quadrant, so the label is - * centred above the symbol. + * centred above the symbol; + * - except an upward-pointing triangle, which is drawn to point at whatever + * sits above it (issue #242). A label there covers the very data the + * analyst aimed the apex at, so it is centred BELOW the symbol instead. * * Pure: takes numbers, returns numbers. * @@ -125,6 +139,14 @@ export function markerLabelPlacement(symbol, cx, cy, symbolSize) { return { x: cx + QUADRANT_GAP, y: cy - QUADRANT_GAP, textAnchor: 'start' } } + if (labelSitsBelowSymbol(symbol)) { + // Centred below the symbol. The baseline drops a whole line of text past + // the symbol's bottom edge, so the glyphs — which hang above their baseline + // — start clear of it rather than overlapping the mark. + const y = cy + symbolSize / 2 + ABOVE_SYMBOL_GAP + MARKER_LABEL_FONT_SIZE + return { x: cx, y, textAnchor: 'middle' } + } + // Centred above the symbol, baseline clear of its top edge. return { x: cx, y: cy - symbolSize / 2 - ABOVE_SYMBOL_GAP, textAnchor: 'middle' } } diff --git a/tests/harmonic-hotspot.spec.js b/tests/harmonic-hotspot.spec.js index 7f6fc1a..cdb146d 100644 --- a/tests/harmonic-hotspot.spec.js +++ b/tests/harmonic-hotspot.spec.js @@ -165,4 +165,45 @@ test.describe('Harmonic set hotspot', () => { expect(await probeAt(gramFramePage, labelSelector(setId, CENTRE_HARMONIC), -60)).toBeNull() }) + + // Issue #242: an up-pointing triangle carries its label underneath, so the + // hotspot has to follow the digits rather than stay where they used to be. + test.describe('up-pointing triangle', () => { + const symbolSelector = (setId) => `.gram-frame-harmonic-symbol[data-harmonic-set-id="${setId}"]` + + test('the label under the triangle still grabs its set', async ({ gramFramePage }) => { + const setId = await gramFramePage.addHarmonicSet(ANCHOR_TIME, SPACING) + await updateSet(gramFramePage, setId, { symbol: 'triangle' }) + + expect(await probeAt(gramFramePage, labelSelector(setId, CENTRE_HARMONIC))).toBe(setId) + }) + + test('the triangle itself still grabs its set', async ({ gramFramePage }) => { + const setId = await gramFramePage.addHarmonicSet(ANCHOR_TIME, SPACING) + await updateSet(gramFramePage, setId, { symbol: 'triangle' }) + + expect(await probeAt(gramFramePage, symbolSelector(setId))).toBe(setId) + }) + + test('the space above the apex, which the label vacated, grabs nothing', async ({ gramFramePage }) => { + const setId = await gramFramePage.addHarmonicSet(ANCHOR_TIME, SPACING) + + // Just above the symbol is live while the label sits there... + await updateSet(gramFramePage, setId, { symbol: 'circle' }) + expect(await probeAt(gramFramePage, symbolSelector(setId), -10)).toBe(setId) + + // ...and goes dead once the label drops below an up-pointing triangle, + // because nothing is drawn there any more. + await updateSet(gramFramePage, setId, { symbol: 'triangle' }) + expect(await probeAt(gramFramePage, symbolSelector(setId), -10)).toBeNull() + }) + + test('with the pin hidden, the mini-pin still hangs from the symbol and grabs the set', async ({ gramFramePage }) => { + const setId = await gramFramePage.addHarmonicSet(ANCHOR_TIME, SPACING) + await updateSet(gramFramePage, setId, { symbol: 'triangle', showPin: false }) + + const miniPin = `.gram-frame-harmonic-mini-pin[data-harmonic-set-id="${setId}"][data-harmonic-number="${CENTRE_HARMONIC}"]` + expect(await probeAt(gramFramePage, miniPin)).toBe(setId) + }) + }) }) diff --git a/tests/harmonic-labels.spec.js b/tests/harmonic-labels.spec.js index 6e973dc..ed15d72 100644 --- a/tests/harmonic-labels.spec.js +++ b/tests/harmonic-labels.spec.js @@ -200,6 +200,55 @@ test.describe('Harmonic Pin Labels (feature 159)', () => { } }) + // ──────────────────────────────────────────────────────────── + // Issue #242 — an up-pointing triangle carries its label underneath + // ──────────────────────────────────────────────────────────── + test('an up-triangle set stacks symbol -> label, keeping the gram above it clear', async ({ gramFramePage }) => { + await gramFramePage.selectSymbol('triangle') + const setId = await gramFramePage.addHarmonicSet(30, 20) + + const { byNum } = await readStackGeometry(gramFramePage, setId) + const entries = Object.values(byNum) + expect(entries.length).toBeGreaterThan(0) + for (const g of entries) { + // Still centred on its pin, and the symbol still caps the line — only + // the label has changed sides. + expect(Math.abs(g.labelCx - g.lineCx)).toBeLessThanOrEqual(3) + expect(g.symbolBottom).toBeLessThanOrEqual(g.lineTop + 2) + // The label now hangs under the symbol, over the pin line's own ink, + // leaving the gram above the apex unobscured. + expect(g.labelTop).toBeGreaterThanOrEqual(g.symbolBottom - 2) + } + }) + + test('a down-pointing triangle keeps its label above the symbol', async ({ gramFramePage }) => { + // Only the UP triangle points at the space a label would occupy. + await gramFramePage.selectSymbol('triangle-down') + const setId = await gramFramePage.addHarmonicSet(30, 20) + + const entries = Object.values((await readStackGeometry(gramFramePage, setId)).byNum) + expect(entries.length).toBeGreaterThan(0) + for (const g of entries) { + expect(g.labelBottom).toBeLessThanOrEqual(g.symbolTop + 2) + } + }) + + test('an up-triangle stack near the top edge stays within the image', async ({ gramFramePage }) => { + // The clamp now measures from the symbol, which leads the stack when the + // label hangs below it (FR-011 still holds). + await gramFramePage.selectSymbol('triangle') + for (const anchorTime of [0, 60]) { + const setId = await gramFramePage.addHarmonicSet(anchorTime, 20) + + const { image, byNum } = await readStackGeometry(gramFramePage, setId) + const entries = Object.values(byNum) + expect(entries.length).toBeGreaterThan(0) + for (const g of entries) { + expect(g.symbolTop).toBeGreaterThanOrEqual(image.top - 1) + } + } + }) + test('a label/symbol stack near the top edge stays within the image', async ({ gramFramePage }) => { // Place the pin at each time extreme; whichever puts the pin near the top // must still keep the label within the image's top edge (FR-011). diff --git a/tests/marker-labels.spec.js b/tests/marker-labels.spec.js index 13775f2..4969625 100644 --- a/tests/marker-labels.spec.js +++ b/tests/marker-labels.spec.js @@ -287,6 +287,28 @@ test.describe('Marker labels', () => { expect(overlay.y).toBeLessThan(symbol.top) }) + // Issue #242: an up-pointing triangle is aimed at the gram above it, so its + // label goes underneath rather than over the data being marked. + test('an up-triangle marker draws its label centred BELOW the symbol', async ({ gramFramePage }) => { + await gramFramePage.selectSymbol('triangle') + const markerId = await placeMarker(gramFramePage, 220, 160) + await gramFramePage.setMarkerLabel(markerId, 'Below') + + const symbol = await gramFramePage.page.evaluate((id) => { + const el = document.querySelector(`.gram-frame-marker-symbol[data-marker-id="${id}"]`) + if (!el) return null + const box = /** @type {SVGGraphicsElement} */ (el).getBBox() + return { centreX: box.x + box.width / 2, bottom: box.y + box.height } + }, markerId) + + const overlay = await gramFramePage.getMarkerLabelOverlay(markerId) + expect(symbol).not.toBeNull() + expect(overlay.textAnchor).toBe('middle') + expect(overlay.x).toBeCloseTo(symbol.centreX, 1) + // The baseline clears the symbol's underside, so the glyphs sit below it. + expect(overlay.y).toBeGreaterThan(symbol.bottom) + }) + test('the label is drawn black inside a white halo, painted behind the glyphs', async ({ gramFramePage }) => { const markerId = await placeMarker(gramFramePage, 220, 160) await gramFramePage.setMarkerLabel(markerId, 'Halo') diff --git a/tests/unit/marker-label.test.js b/tests/unit/marker-label.test.js index 8566b9b..80de1a6 100644 --- a/tests/unit/marker-label.test.js +++ b/tests/unit/marker-label.test.js @@ -5,6 +5,7 @@ import { formatMarkerLabelForTable, markerLabelPlacement } from '../../src/utils/markerLabel.js' +import { labelSitsBelowSymbol, SYMBOL_CATALOG } from '../../src/rendering/symbols.js' /** * @fileoverview Unit coverage for feature 231 — cross-cursor labels. @@ -98,8 +99,8 @@ describe('markerLabelPlacement', () => { expect(placement.textAnchor).toBe('middle') }) - it('places every shaped symbol the same way', () => { - const shaped = ['circle', 'square', 'diamond', 'triangle', 'triangle-down', 'star'] + it('places every shaped symbol above, bar the up-pointing triangle', () => { + const shaped = ['circle', 'square', 'diamond', 'triangle-down', 'star'] const expected = markerLabelPlacement('circle', CX, CY, SYMBOL_SIZE) for (const symbol of shaped) { @@ -107,6 +108,33 @@ describe('markerLabelPlacement', () => { } }) + // Issue #242: an up-pointing triangle is aimed at the gram above it, so a + // label there covers the data the analyst was marking. + it('centres an up-triangle marker\'s label BELOW the symbol', () => { + const placement = markerLabelPlacement('triangle', CX, CY, SYMBOL_SIZE) + + expect(placement.x).toBe(CX) + expect(placement.textAnchor).toBe('middle') + // A whole line of text below the symbol's bottom edge, so the glyphs — + // which hang above their baseline — start clear of the mark. + expect(placement.y).toBeGreaterThan(CY + SYMBOL_SIZE / 2 + 12) + }) + + it('leaves the down-pointing triangle\'s label above it', () => { + // Only the UP triangle points at the space a label would occupy. + const down = markerLabelPlacement('triangle-down', CX, CY, SYMBOL_SIZE) + + expect(down.y).toBeLessThan(CY - SYMBOL_SIZE / 2) + }) + + it('drops the up-triangle\'s label further for a larger symbol', () => { + const small = markerLabelPlacement('triangle', CX, CY, 14) + const large = markerLabelPlacement('triangle', CX, CY, 28) + + expect(large.y).toBeGreaterThan(small.y) + expect(large.y - small.y).toBe((28 - 14) / 2) + }) + it('lifts the label further for a larger symbol', () => { const small = markerLabelPlacement('square', CX, CY, 14) const large = markerLabelPlacement('square', CX, CY, 28) @@ -136,3 +164,19 @@ describe('markerLabelPlacement', () => { expect(moved.y - base.y).toBe(-10) }) }) + +describe('labelSitsBelowSymbol', () => { + it('is true for the up-pointing triangle alone', () => { + // The rule the pin stack and the marker label share (issue #242): one + // symbol points at the space above it, so only that one gives it up. + const below = SYMBOL_CATALOG.filter(labelSitsBelowSymbol) + + expect(below).toEqual(['triangle']) + }) + + it('treats an unknown, null or absent symbol as the default (above)', () => { + expect(labelSitsBelowSymbol('pentagon')).toBe(false) + expect(labelSitsBelowSymbol(null)).toBe(false) + expect(labelSitsBelowSymbol(undefined)).toBe(false) + }) +})