diff --git a/CLAUDE.md b/CLAUDE.md index 8409624..95f23fe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -143,7 +143,8 @@ Every path below exists; keep this list in step with `src/` when adding modules. - `cursors.js` - The pointer's cursors. Feature drags use hollow corner brackets, never the opaque `grab`/`grabbing` hands, so the marker and the gram under the hotspot stay visible; panning keeps the hand - - `svg.js` - SVG text halo styling + - `labelPlate.js` - The white rounded plate every in-gram text label is drawn + on, and the geometry the placement rules leave room for it with (issue #243) - `secureHTML.js` - Guidance-panel rendering without innerHTML - `timeFormatter.js` - Time formatting utilities - `wheelGuidance.js` - Wheel navigation guidance text @@ -263,7 +264,7 @@ 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 — + plated 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 — diff --git a/docs/Adding-Graphical-Features.md b/docs/Adding-Graphical-Features.md index 774c45a..826a97f 100644 --- a/docs/Adding-Graphical-Features.md +++ b/docs/Adding-Graphical-Features.md @@ -13,7 +13,8 @@ When adding a new graphical feature, you will likely need to work with these fil | `src/core/FeatureRenderer.js` | Main render entry point; clears and redraws all features | Adding a new visual element type | | `src/components/table.js` | SVG layout, axis rendering, zoom transforms | Changing axes or SVG structure | | `src/utils/coordinateTransformations.js` | Zoom-aware transforms, `dataToSVG()` | Positioning elements when zoomed | -| `src/utils/svg.js` | SVG element creation helpers (`createSVGLine`, `createSVGText`, `createSVGCircle`) | Creating new SVG shapes | +| `src/rendering/symbols.js` | Marker and pin symbol shapes (`createSymbolMark`) | Drawing a shaped mark | +| `src/utils/labelPlate.js` | The white rounded plate behind on-gram text (`plateLabel`) | Drawing a text label over the gram | | `src/utils/coordinates.js` | Coordinate transforms (screen → SVG → image → data) | Positioning elements on the spectrogram | | `src/core/FeatureRenderer.js` | Cross-mode feature visibility coordinator | Feature needs to persist across mode switches | | `src/core/events.js` | Mouse event handling and coordinate conversion | Feature responds to mouse interactions | @@ -72,7 +73,7 @@ If your feature belongs to an existing mode (e.g., a new annotation type in Anal 1. **Store state** — Add fields to the mode's `static getInitialState()` method 2. **Handle interaction** — Override `handleMouseDown`/`handleMouseMove`/`handleMouseUp` to capture user input 3. **Render** — Add drawing logic to `renderPersistentFeatures()` (for saved features) or `renderCursor()` (for live indicators) -4. **Create SVG elements** using utilities from `src/utils/svg.js`, and append them to `instance.cursorGroup` +4. **Create SVG elements** — `createSymbolMark()` from `src/rendering/symbols.js` for shapes, `plateLabel()` from `src/utils/labelPlate.js` for text — and append them to `instance.cursorGroup` ### Example Pattern (from Doppler mode) diff --git a/docs/Gram-Modes.md b/docs/Gram-Modes.md index fedc7c1..7dc0ff4 100644 --- a/docs/Gram-Modes.md +++ b/docs/Gram-Modes.md @@ -56,8 +56,8 @@ events, broadband pulses or ambient shifts in sonar data. - Each row's **label button** (the tag icon, above the delete ×) opens a dialog for the marker's label. Labels are optional — a marker has none until one is 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, +- A label is drawn on the gram in black on a white rounded plate, 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. 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. diff --git a/docs/Rendering-Troubleshooting.md b/docs/Rendering-Troubleshooting.md index 2e4865c..432cae3 100644 --- a/docs/Rendering-Troubleshooting.md +++ b/docs/Rendering-Troubleshooting.md @@ -12,7 +12,8 @@ Understanding which files handle what is the first step in narrowing down a rend |-----------|------|---------| | Render entry point | `src/core/FeatureRenderer.js` | `renderAllPersistentFeatures()` — clears and redraws all features | | Feature coordination | `src/core/FeatureRenderer.js` | Cross-mode visibility; delegates to each mode's renderer | -| SVG element creation | `src/utils/svg.js` | `createSVGLine`, `createSVGText`, `createSVGCircle` | +| Symbols and labels | `src/rendering/symbols.js`, `src/rendering/labels.js` | `createSymbolMark`, `createMarkerLabel` | +| On-gram text legibility | `src/utils/labelPlate.js` | `plateLabel` — the white rounded plate behind every label | | Coordinate transforms | `src/utils/coordinates.js` | `screenToSVGCoordinates`, `imageToDataCoordinates` | | Zoom-aware conversion | `src/core/events.js` | `screenToDataWithZoom()` — full pipeline with zoom | | SVG layout and axes | `src/components/table.js` | `updateSVGLayout()`, `renderAxes()`, `applyZoomTransform()` | diff --git a/docs/Tech-Architecture.md b/docs/Tech-Architecture.md index d881fe6..1ae41fc 100644 --- a/docs/Tech-Architecture.md +++ b/docs/Tech-Architecture.md @@ -153,11 +153,13 @@ Axes are rendered by `renderAxes(instance)` in `src/components/table.js`: 1. Clears `cursorGroup` 2. Calls `featureRenderer.renderAllPersistentFeatures()` to redraw all saved features -Modes add SVG elements to `cursorGroup` using utilities from `src/utils/svg.js`: +Modes create their own SVG elements and append them to `cursorGroup`, drawing on +the shared rendering helpers: -- `createSVGLine(x1, y1, x2, y2, className)` — Creates `` elements -- `createSVGText(x, y, text, className, anchor)` — Creates `` elements -- `createSVGCircle(cx, cy, r, className)` — Creates `` elements +- `src/rendering/symbols.js` — `createSymbolMark()` for a marker or pin's shape +- `src/rendering/labels.js` — `createMarkerLabel()` for a marker's on-gram label +- `src/utils/labelPlate.js` — `plateLabel()`, which puts any on-gram text on the + white rounded plate that keeps it legible over the gram ### Coordinate Transform Chain diff --git a/src/gramframe.css b/src/gramframe.css index c5f15fd..06315db 100644 --- a/src/gramframe.css +++ b/src/gramframe.css @@ -908,9 +908,10 @@ table.gram-frame-table { } /* - * A marker's on-gram label. Legibility comes from the halo (black glyphs in a - * white outline) set as presentation attributes by applyTextHalo() — see - * src/utils/svg.js. Never a click target: the marker underneath is. + * A marker's on-gram label. Legibility comes from the white rounded plate drawn + * behind it (issue #243) — the geometry and colours are presentation attributes + * set by plateLabel(), see src/utils/labelPlate.js. Never a click target: the + * marker underneath is. */ .gram-frame-marker-label { font-family: Arial, sans-serif; @@ -920,6 +921,16 @@ table.gram-frame-table { user-select: none; } +/* + * The white plate behind any on-gram label, and the group holding the two. Both + * are transparent to the pointer so the plate never intercepts a click meant + * for the feature it annotates, or for the gram beneath it. + */ +.gram-frame-label-plate, +.gram-frame-label-plated { + pointer-events: none; +} + /* Military-style mode selection header */ .gram-frame-mode-header { background: linear-gradient(180deg, #444 0%, #2a2a2a 50%, #1a1a1a 100%); @@ -1276,9 +1287,9 @@ table.gram-frame-table { font-weight: bold; pointer-events: none; /* - * Legibility comes from the halo (black glyphs inside a white outline) set as - * presentation attributes by applyTextHalo() in src/utils/svg.js — see the - * fill/stroke/paint-order there. No drop-shadow: it only blurred the outline. + * Legibility comes from the white rounded plate drawn behind the digits + * (issue #243), set as presentation attributes by plateLabel() in + * src/utils/labelPlate.js. No drop-shadow: it only blurs the plate's edge. */ } diff --git a/src/modes/shared/PinSetMode.js b/src/modes/shared/PinSetMode.js index 448f908..1679ac9 100644 --- a/src/modes/shared/PinSetMode.js +++ b/src/modes/shared/PinSetMode.js @@ -33,7 +33,7 @@ import { BaseDragHandler } from './BaseDragHandler.js' import { getUniformTolerance } from '../../utils/tolerance.js' import { sampledHarmonics } from '../../utils/harmonicSampling.js' import { createSymbolMark, labelSitsBelowSymbol, resolveSymbolScale } from '../../rendering/symbols.js' -import { applyTextHalo } from '../../utils/svg.js' +import { labelPlateExtents, labelPlateRect, measureLabelWidth, plateLabel } from '../../utils/labelPlate.js' /** * Minimum spacing (Hz) any pin set may be dragged or nudged to. @@ -92,22 +92,15 @@ export class PinSetMode extends BaseMode { static MAX_PIN_LINES = 1000 /** - * Font size (px) of a pin's number label; also used as its approximate ascent - * when clamping the label/symbol stack to the image's top edge. + * Font size (px) of a pin's number label. The plate the label sits on is + * sized from it too, so it also fixes how much room the stack leaves above + * and below the text (see `utils/labelPlate.js`). * @type {number} */ static LABEL_FONT_SIZE = 12 /** - * Approximate width of one label character as a fraction of the label font - * size, used to size the label's grab region (bold Arial digits are ~0.6 em - * wide). - * @type {number} - */ - static LABEL_CHAR_WIDTH_RATIO = 0.6 - - /** - * Vertical gap (px) between the pin's number label and its symbol. + * Vertical gap (px) between the edge of the pin label's plate and its symbol. * @type {number} */ static LABEL_GAP = 3 @@ -735,18 +728,19 @@ export class PinSetMode extends BaseMode { labelStackPositions(lineTop, imageTop, set) { const r = this.symbolSize(set) / 2 const gap = PinSetMode.LABEL_GAP - const fontSize = PinSetMode.LABEL_FONT_SIZE + const plate = labelPlateExtents(PinSetMode.LABEL_FONT_SIZE) const below = labelSitsBelowSymbol(set.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. + // Symbol caps the line; the label sits just above the symbol, or — for an + // up-pointing triangle — just below it. The gap is measured from the edge + // of the label's plate rather than from its baseline (issue #243), so the + // white rectangle clears the mark by as much as the bare glyphs used to. let symbolCy = lineTop - r - let labelY = below ? symbolCy + r + gap + fontSize : symbolCy - r - gap + let labelY = below ? symbolCy + r + gap + plate.above : symbolCy - r - gap - plate.below - // 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 + // Keep the top of the stack on-screen: the top of the label's plate when it + // leads the stack, the symbol's top edge when the label hangs below. + const stackTop = below ? symbolCy - r : labelY - plate.above const minTop = imageTop + PinSetMode.STACK_TOP_PAD if (stackTop < minTop) { const shift = minTop - stackTop @@ -782,13 +776,14 @@ export class PinSetMode extends BaseMode { const r = this.symbolSize(set) / 2 const below = labelSitsBelowSymbol(set.symbol) const symbolBottom = symbolCy + r + const plate = labelPlateExtents(PinSetMode.LABEL_FONT_SIZE) return { - // 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), + // The top of the label's plate — unless the label hangs below, in which + // case the symbol leads the stack. + top: below ? symbolCy - r : labelY - plate.above, + // The plate's underside is the bottom of the stack when the label trails. + bottom: Math.max(lineTop, below ? labelY + plate.below : symbolBottom), symbolBottom } } @@ -798,18 +793,25 @@ export class PinSetMode extends BaseMode { * * The wider of the symbol mark and the number label, so both are grabbable: * a `cross` set has no symbol but still shows its label, and a "Large - * symbols" set's mark is wider than its text. Label width is estimated from - * the character count rather than measured, which is ample for a grab region. + * symbols" set's mark is wider than its text. The label's half-width is the + * plate's, measured the same way the renderer sizes it, so the grab region + * covers exactly the white rectangle the analyst is aiming at. * * @param {PinSet} set - Set being hit-tested * @param {number} index - Member index whose label is drawn * @returns {number} Half-width in SVG pixels */ labelStackHalfWidth(set, index) { - const characters = this.labelTextFor(index).length - const labelHalfWidth = characters * PinSetMode.LABEL_FONT_SIZE * PinSetMode.LABEL_CHAR_WIDTH_RATIO / 2 + const fontSize = PinSetMode.LABEL_FONT_SIZE + const plate = labelPlateRect({ + x: 0, + y: 0, + textAnchor: 'middle', + width: measureLabelWidth(this.labelTextFor(index), fontSize), + fontSize + }) - return Math.max(this.symbolSize(set) / 2, labelHalfWidth) + return Math.max(this.symbolSize(set) / 2, plate.width / 2) } // --------------------------------------------------------------------------- @@ -864,7 +866,7 @@ export class PinSetMode extends BaseMode { } /** - * Create the SVG text label for a member. + * Create the plated text label for a member. * * 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 @@ -872,32 +874,34 @@ export class PinSetMode extends BaseMode { * {@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 - * halo reads over both dark and light backgrounds. Set identity is still - * carried by the pin's line and symbol colour. + * The characters are drawn black on a white rounded plate rather than in the + * set's colour: a single colour is only legible over part of a gram, whereas + * the plate reads over both dark and light backgrounds (issue #243). Set + * identity is still carried by the pin's line and symbol colour. * * @param {number} index - Member index * @param {PinSet} set - The set * @param {number} lineX - X position of the pin line (label is centred on it) * @param {number} labelY - Baseline Y position for the label text - * @returns {SVGTextElement} SVG text element + * @returns {SVGGElement} Group holding the plate and its text */ createPinLabel(index, set, lineX, labelY) { const names = this.pinNames - const label = document.createElementNS('http://www.w3.org/2000/svg', 'text') + const label = /** @type {SVGTextElement} */ ( + document.createElementNS('http://www.w3.org/2000/svg', 'text') + ) label.setAttribute('class', names.labelClass) label.setAttribute(names.setIdAttribute, set.id) label.setAttribute(names.indexAttribute, String(index)) label.setAttribute('x', String(lineX)) // centred on the pin line label.setAttribute('y', String(labelY)) // above the symbol label.setAttribute('text-anchor', 'middle') - applyTextHalo(/** @type {SVGTextElement} */ (label)) label.setAttribute('font-size', String(PinSetMode.LABEL_FONT_SIZE)) label.setAttribute('font-weight', 'bold') label.setAttribute('font-family', 'Arial, sans-serif') label.textContent = this.labelTextFor(index) - return label + // Plated last, once the text carries everything the plate is sized from. + return plateLabel(label) } /** diff --git a/src/rendering/labels.js b/src/rendering/labels.js index 0bd12b2..15349d2 100644 --- a/src/rendering/labels.js +++ b/src/rendering/labels.js @@ -1,10 +1,10 @@ /** * In-gram text labels for analysis markers (feature 231). * - * A marker's label is drawn as black glyphs inside a white halo — the same - * treatment the harmonic numbers use — so it reads over both dark and light - * spectrogram pixels. Marker identity is still carried by the crosshair or - * symbol colour; the label text deliberately is not colour-coded. + * A marker's label is drawn as black glyphs on a white rounded plate — the same + * treatment the harmonic numbers use (issue #243) — so it reads over both dark + * and light spectrogram pixels. Marker identity is still carried by the + * crosshair or 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) — including @@ -15,14 +15,15 @@ /// -import { applyTextHalo } from '../utils/svg.js' +import { plateLabel } from '../utils/labelPlate.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' /** - * Build a marker's label as a detached SVG text element. + * Build a marker's label as a detached SVG group: the white plate and the + * text drawn on it. * * Returns `null` when the marker carries no label, so the caller draws nothing * — labels are absent by default. Callers MUST handle a `null` return. @@ -31,7 +32,7 @@ const SVG_NS = 'http://www.w3.org/2000/svg' * @param {number} cx - Marker centre X in SVG overlay space * @param {number} cy - Marker centre Y in SVG overlay space * @param {number} symbolSize - Drawn diameter of the marker's symbol in px - * @returns {SVGTextElement|null} Detached label element, or `null` when unlabelled + * @returns {SVGGElement|null} Detached plate-and-text group, or `null` when unlabelled */ export function createMarkerLabel(marker, cx, cy, symbolSize) { if (!marker.label) { @@ -49,7 +50,7 @@ export function createMarkerLabel(marker, cx, cy, symbolSize) { text.setAttribute('font-size', String(MARKER_LABEL_FONT_SIZE)) text.setAttribute('font-weight', 'bold') text.setAttribute('font-family', 'Arial, sans-serif') - applyTextHalo(text) text.textContent = marker.label - return text + // Plated last, once the text carries everything the plate is sized from. + return plateLabel(text) } diff --git a/src/utils/cursors.js b/src/utils/cursors.js index 755caa1..c7873e1 100644 --- a/src/utils/cursors.js +++ b/src/utils/cursors.js @@ -64,10 +64,11 @@ const DRAG_BRACKETS = [ /** * Build the cursor artwork. * - * Each shape is stroked twice — black underneath, white on top — the halo idiom - * already used for marker text (`applyTextHalo` in `svg.js`). It is what keeps - * the cursor legible over the blue field and over a saturated yellow tonal - * alike, neither of which a single-colour cursor survives. + * Each shape is stroked twice — black underneath, white on top — the halo idiom. + * A cursor cannot carry the plate on-gram text labels use (`labelPlate.js`), so + * it keeps the two-tone outline instead; it is what keeps the cursor legible + * over the blue field and over a saturated yellow tonal alike, neither of which + * a single-colour cursor survives. * @param {string[]} shapes - Path data for the brackets * @param {number} coreWidth - Stroke width of the white core * @param {number} haloWidth - Stroke width of the black halo beneath it diff --git a/src/utils/labelPlate.js b/src/utils/labelPlate.js new file mode 100644 index 0000000..4c27a36 --- /dev/null +++ b/src/utils/labelPlate.js @@ -0,0 +1,257 @@ +/** + * White rounded-rectangle plates behind in-gram text labels (issue #243). + * + * Every label drawn over the spectrogram — a harmonic or sideband pin number, a + * cross-cursor's text label — sits on an opaque white plate with rounded + * corners, and is drawn in black on top of it. This replaces the halo (a white + * outline stroked behind the glyphs) the labels used to carry: the legacy + * spectrogram viewer plates its harmonic numbers, and side by side the plate is + * the easier of the two to read. A halo only whitens the pixels immediately + * around each stroke, so a noisy gram still shows through the counters of the + * digits and between them; a plate clears one contiguous rectangle, so the + * contrast is the same everywhere in the label. + * + * Label identity is still NOT colour-coded: the plate is white and the text + * black whatever colour the feature is, because a coloured label is only legible + * over part of a gram. The pin's line and symbol carry the colour. + * + * The geometry here is pure (numbers in, numbers out) so the placement rules + * that have to leave room for a plate — `utils/markerLabel.js` and + * `PinSetMode.labelStackPositions` — can size their gaps from the same + * constants the renderer draws with, and so the unit lane can cover it without + * a browser. Only {@link plateLabel} and {@link measureLabelWidth} touch the DOM. + */ + +/** SVG namespace for element creation */ +const SVG_NS = 'http://www.w3.org/2000/svg' + +/** + * Class on the plate rectangle drawn behind a label. + * @type {string} + */ +const LABEL_PLATE_CLASS = 'gram-frame-label-plate' + +/** + * Class on the group holding a plate and its text. + * @type {string} + */ +const LABEL_PLATE_GROUP_CLASS = 'gram-frame-label-plated' + +/** + * Plate fill. Opaque white — a translucent plate lets the noise it is there to + * hide back through, which is the halo's weakness all over again. + * @type {string} + */ +const LABEL_PLATE_FILL = '#fff' + +/** + * Text colour drawn on the plate. + * @type {string} + */ +const LABEL_TEXT_FILL = '#000' + +/** + * Horizontal padding (px) between the text's ends and the plate's edges. + * @type {number} + */ +export const LABEL_PLATE_PADDING_X = 3 + +/** + * Corner radius (px) of the plate. + * @type {number} + */ +const LABEL_PLATE_RADIUS = 3 + +/** + * How far the plate rises above the text's baseline, as a fraction of the font + * size. Bold Arial's cap height is ~0.72 em and its ascent — the top of the box + * the browser lays the characters out in — is ~0.92 em, so 0.95 em keeps every + * character a label can hold, digits and accented capitals alike, wholly on the + * plate rather than clipping the tallest of them at the edge. + * @type {number} + */ +const PLATE_ABOVE_RATIO = 0.95 + +/** + * How far the plate drops below the text's baseline, as a fraction of the font + * size — enough for the descenders of `g`, `p` and `y` (~0.21 em) plus a hair + * of padding. Fixed rather than measured per label so every plate in a stack is + * the same height whatever characters it happens to hold. + * @type {number} + */ +const PLATE_BELOW_RATIO = 0.3 + +/** + * Fallback width of one character as a fraction of the font size, used when the + * text cannot be measured (no canvas — the unit lane runs in Node). Bold Arial + * digits are ~0.6 em wide. + * @type {number} + */ +const FALLBACK_CHAR_WIDTH_RATIO = 0.6 + +/** + * How far a plate extends above and below the baseline of the text it carries. + * + * Placement rules use this to leave room for the plate rather than for the bare + * glyphs, so a label still clears the symbol or crosshair it annotates by the + * gap its author intended. + * + * @param {number} fontSize - Label font size in px + * @returns {{above: number, below: number}} Plate extents in px from the baseline + */ +export function labelPlateExtents(fontSize) { + // Rounded to half a pixel: the plate's edges land on tidy coordinates, and + // the placement rules that subtract these from a gap stay free of the + // floating-point dust that `12 * 0.3` would otherwise leave in every label + // position. + return { + above: roundToHalfPixel(fontSize * PLATE_ABOVE_RATIO), + below: roundToHalfPixel(fontSize * PLATE_BELOW_RATIO) + } +} + +/** + * Round a length to the nearest half pixel. + * @param {number} value - Length in px + * @returns {number} The length, snapped to a half pixel + */ +function roundToHalfPixel(value) { + return Math.round(value * 2) / 2 +} + +/** + * The plate rectangle for a label, in the same SVG coordinates as the text. + * + * @param {Object} label - The label being plated + * @param {number} label.x - Text anchor X + * @param {number} label.y - Text baseline Y + * @param {string} label.textAnchor - SVG `text-anchor` of the text (`start`, `middle` or `end`) + * @param {number} label.width - Rendered width of the text in px + * @param {number} label.fontSize - Label font size in px + * @returns {{x: number, y: number, width: number, height: number}} Plate rectangle + */ +export function labelPlateRect({ x, y, textAnchor, width, fontSize }) { + const { above, below } = labelPlateExtents(fontSize) + // Where the text starts, given how it grows from its anchor. + let left = x + if (textAnchor === 'middle') { + left = x - width / 2 + } else if (textAnchor === 'end') { + left = x - width + } + + return { + x: left - LABEL_PLATE_PADDING_X, + y: y - above, + width: width + LABEL_PLATE_PADDING_X * 2, + height: above + below + } +} + +/** + * Canvas 2D context kept for text measurement, or `null` once we know there is + * none to be had. `undefined` means "not looked for yet". + * @type {CanvasRenderingContext2D|null|undefined} + */ +let measurementContext + +/** + * The shared canvas context used to measure label text, if this environment has + * one. + * @returns {CanvasRenderingContext2D|null} Context for measuring, or null + */ +function textMeasurementContext() { + if (measurementContext === undefined) { + try { + measurementContext = document.createElement('canvas').getContext('2d') + } catch { + measurementContext = null + } + } + return measurementContext +} + +/** + * Width in px of a label's text at the font it is drawn in. + * + * Measured with a canvas, which gives the same advance widths the SVG text is + * laid out with, so a plate fits its characters rather than an average of them + * — a label of capitals is wider than one of digits. Falls back to a + * character-count estimate where no canvas exists (the Node unit lane) or the + * measurement comes back empty. + * + * @param {string} content - The label text + * @param {number} fontSize - Font size in px + * @param {Object} [font] - Font overrides + * @param {string} [font.fontFamily] - CSS font family the text is drawn in + * @param {string} [font.fontWeight] - CSS font weight the text is drawn in + * @returns {number} Text width in px + */ +export function measureLabelWidth(content, fontSize, font = {}) { + const { fontFamily = 'Arial, sans-serif', fontWeight = 'bold' } = font + const text = content || '' + const context = textMeasurementContext() + if (context) { + context.font = `${fontWeight} ${fontSize}px ${fontFamily}` + const measured = context.measureText(text).width + if (measured > 0) { + return measured + } + } + return text.length * fontSize * FALLBACK_CHAR_WIDTH_RATIO +} + +/** + * Put an SVG text element on a white rounded plate. + * + * Call this LAST, once the text carries its `x`, `y`, `text-anchor`, + * `font-size` and content: the plate is sized from those, and the text keeps + * every attribute (and its class) so selectors, tests and CSS still find it + * where they did before. The returned group is what the caller appends — + * plate first, text second, so the characters sit on top. + * + * @param {SVGTextElement} text - Fully-attributed text element (mutated: fill and stroke) + * @param {Object} [options] - Plate overrides + * @param {string} [options.fill] - Plate colour + * @param {string} [options.textFill] - Glyph colour + * @returns {SVGGElement} Group holding the plate and the text + */ +export function plateLabel(text, options = {}) { + const { fill = LABEL_PLATE_FILL, textFill = LABEL_TEXT_FILL } = options + + const fontSize = Number(text.getAttribute('font-size')) + const width = measureLabelWidth(text.textContent || '', fontSize, { + fontFamily: text.getAttribute('font-family') || undefined, + fontWeight: text.getAttribute('font-weight') || undefined + }) + const box = labelPlateRect({ + x: Number(text.getAttribute('x')), + y: Number(text.getAttribute('y')), + textAnchor: text.getAttribute('text-anchor') || 'start', + width, + fontSize + }) + + // Black glyphs, and no stroke: the plate behind them is the contrast now, and + // a leftover halo stroke would thicken the digits over it. + text.setAttribute('fill', textFill) + text.removeAttribute('stroke') + text.removeAttribute('stroke-width') + text.removeAttribute('paint-order') + + const plate = document.createElementNS(SVG_NS, 'rect') + plate.setAttribute('class', LABEL_PLATE_CLASS) + plate.setAttribute('x', String(box.x)) + plate.setAttribute('y', String(box.y)) + plate.setAttribute('width', String(box.width)) + plate.setAttribute('height', String(box.height)) + plate.setAttribute('rx', String(LABEL_PLATE_RADIUS)) + plate.setAttribute('ry', String(LABEL_PLATE_RADIUS)) + plate.setAttribute('fill', fill) + + const group = /** @type {SVGGElement} */ (document.createElementNS(SVG_NS, 'g')) + group.setAttribute('class', LABEL_PLATE_GROUP_CLASS) + group.appendChild(plate) + group.appendChild(text) + return group +} diff --git a/src/utils/markerLabel.js b/src/utils/markerLabel.js index 3a28dc2..ee81535 100644 --- a/src/utils/markerLabel.js +++ b/src/utils/markerLabel.js @@ -16,6 +16,7 @@ /// import { labelSitsBelowSymbol, resolveSymbolType } from '../rendering/symbols.js' +import { labelPlateExtents, LABEL_PLATE_PADDING_X } from './labelPlate.js' /** * Longest label accepted. Long enough for a ship name or a contact @@ -88,22 +89,23 @@ export function formatMarkerLabelForTable(label) { /** * Gap in px between a crosshair's arms and the label sitting in the upper-right * quadrant. Clears the crosshair's 3px centre dot without pushing the text away - * from the point it annotates. + * from the point it annotates. Measured to the edge of the label's plate, not + * to the glyphs, so the white rectangle stays out of the crosshair. * @type {number} */ const QUADRANT_GAP = 5 /** - * Gap in px between the top of a shaped symbol and the label's baseline above - * it. Scales with nothing: the symbol's own size is already in the sum. + * Gap in px between a shaped symbol's edge and the nearest edge of the label's + * plate. Scales with nothing: the symbol's own size is already in the sum. * @type {number} */ 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. + * Label font size in px. It also fixes how far the label's plate reaches above + * and below the baseline, which is what the placement gaps below are measured + * to. * * 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. @@ -133,20 +135,27 @@ export const MARKER_LABEL_FONT_SIZE = 12 * @returns {{x: number, y: number, textAnchor: 'start'|'middle'}} Text position and anchor */ export function markerLabelPlacement(symbol, cx, cy, symbolSize) { + // The label sits on a white plate (issue #243), which reaches past the text + // on every side; every gap below is measured to the plate's edge so the + // rectangle clears the mark by as much as the bare glyphs used to. + const plate = labelPlateExtents(MARKER_LABEL_FONT_SIZE) + if (resolveSymbolType(symbol) === 'cross') { // Upper-right quadrant of the crosshair: right of the vertical arm, above // the horizontal one. `start` anchoring grows the text away from the arms. - return { x: cx + QUADRANT_GAP, y: cy - QUADRANT_GAP, textAnchor: 'start' } + return { + x: cx + QUADRANT_GAP + LABEL_PLATE_PADDING_X, + y: cy - QUADRANT_GAP - plate.below, + 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 + // Centred below the symbol, the top of the plate clear of its bottom edge. + const y = cy + symbolSize / 2 + ABOVE_SYMBOL_GAP + plate.above 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' } + // Centred above the symbol, the bottom of the plate clear of its top edge. + return { x: cx, y: cy - symbolSize / 2 - ABOVE_SYMBOL_GAP - plate.below, textAnchor: 'middle' } } diff --git a/src/utils/svg.js b/src/utils/svg.js deleted file mode 100644 index d348315..0000000 --- a/src/utils/svg.js +++ /dev/null @@ -1,56 +0,0 @@ -/** - * SVG text styling utilities. - */ - -/** - * Default halo geometry/colours for in-gram text labels. - * - * A halo (also "text casing" or, in GIS, a label buffer) is a contrasting - * outline drawn behind the glyphs so a label stays legible over an unknown - * background: the white ring carries the digits over dark spectrogram pixels, - * the black core carries them over light ones. This replaces colour-coded label - * text, which is only readable over part of a gram. - * - * `width` is ~25% of the 12px label font — thick enough to survive over noisy - * pixels without closing up the counters of the digits. - * @type {{fill: string, haloColor: string, width: number}} - */ -const TEXT_HALO = { - fill: '#000', - haloColor: '#fff', - width: 3 -} - -/** - * Apply a halo (contrasting outline) to an SVG text element so it reads against - * any background. - * - * The stroke is painted BEHIND the fill via `paint-order`; without that the - * stroke straddles the glyph outline and eats half of each letterform. All - * current browsers support `paint-order` on text (Chrome 35+, Firefox 60+, - * Safari 8+); older engines still show the text, just with thinner-looking - * glyphs, so no duplicate-text-node fallback is drawn. - * - * Set as presentation attributes (not CSS) so the halo travels with the element - * wherever it is rendered, while remaining overridable by a stylesheet. - * - * @param {SVGTextElement} text - Text element to style (mutated in place) - * @param {Object} [options] - Halo overrides - * @param {string} [options.fill] - Glyph colour (the halo's core) - * @param {string} [options.haloColor] - Outline colour drawn behind the glyphs - * @param {number} [options.width] - Outline width in px (total, centred on the glyph outline) - * @returns {SVGTextElement} The same element, for chaining - */ -export function applyTextHalo(text, options = {}) { - const { fill = TEXT_HALO.fill, haloColor = TEXT_HALO.haloColor, width = TEXT_HALO.width } = options - text.setAttribute('fill', fill) - text.setAttribute('stroke', haloColor) - text.setAttribute('stroke-width', String(width)) - // Round joins keep the outline smooth at sharp glyph corners. - text.setAttribute('stroke-linejoin', 'round') - // Paint the halo behind the glyphs so the letterforms stay full-weight. - text.setAttribute('paint-order', 'stroke fill') - return text -} - - diff --git a/tests/harmonic-label-halo.spec.js b/tests/harmonic-label-halo.spec.js deleted file mode 100644 index 93e7445..0000000 --- a/tests/harmonic-label-halo.spec.js +++ /dev/null @@ -1,106 +0,0 @@ -import { test, expect } from './helpers/fixtures.js' - -/** - * @fileoverview E2E tests for the harmonic pin label halo — the number labels - * are drawn as black digits inside a white outline ("halo"/text casing) instead - * of in the harmonic set's colour, so they stay legible over both dark and - * light areas of a gram. - * - * Set identity must still be carried by the pin's line (and symbol) colour, so - * these tests also assert the line keeps the set colour. - * - * Debug config spans freq 0-100 Hz over time 0-60 s, so a 20 Hz set places - * harmonics 1..5 — every pin labelled. - */ - -const BLACK = 'rgb(0, 0, 0)' -const WHITE = 'rgb(255, 255, 255)' - -/** - * Set the colour the next created feature will take (the colour picker's - * selection), via the test-only instance registry. - * @param {import('./helpers/gram-frame-page.js').GramFramePage} gfp - Page helper - * @param {string} color - Hex colour - * @returns {Promise} - */ -async function setSelectedColor(gfp, color) { - await gfp.page.evaluate((c) => { - // @ts-ignore - test-only global - const instance = window.GramFrame.__test__getInstances()[0] - instance.state.selectedColor = c - }, color) -} - -test.describe('Harmonic pin label halo', () => { - test.beforeEach(async ({ gramFramePage }) => { - await gramFramePage.clickMode('Harmonics') - await gramFramePage.waitForImageDimensions() - }) - - test('every pin number label is black digits inside a white halo', async ({ gramFramePage }) => { - const setId = await gramFramePage.addHarmonicSet(30, 20) - - const styles = await gramFramePage.getHarmonicLabelStyles(setId) - expect(styles.length).toBeGreaterThan(0) - - for (const style of styles) { - expect(style.fill).toBe(BLACK) - expect(style.stroke).toBe(WHITE) - // A halo wide enough to read over noise, but not so wide it closes up the digits - expect(parseFloat(style.strokeWidth)).toBeGreaterThan(1) - expect(parseFloat(style.strokeWidth)).toBeLessThanOrEqual(4) - expect(style.strokeLinejoin).toBe('round') - // The halo must be painted BEHIND the fill, or the stroke eats the glyphs - expect(style.paintOrder).toMatch(/^stroke/) - } - }) - - test('label paint is independent of the set colour, which stays on the pin line', async ({ gramFramePage }) => { - // A new set takes the currently selected colour, so pick a different one - // before each add to get two visibly distinct sets. - await setSelectedColor(gramFramePage, '#ff6b6b') - const firstId = await gramFramePage.addHarmonicSet(20, 20) - await setSelectedColor(gramFramePage, '#45b7d1') - const secondId = await gramFramePage.addHarmonicSet(40, 25) - - const state = await gramFramePage.getState() - const sets = state.harmonics.harmonicSets - expect(sets.length).toBe(2) - const colors = sets.map((/** @type {any} */ s) => String(s.color).toLowerCase()) - expect(colors[0]).not.toBe(colors[1]) - - // Both sets' labels are painted identically — colour no longer distinguishes them - for (const setId of [firstId, secondId]) { - const styles = await gramFramePage.getHarmonicLabelStyles(setId) - expect(styles.length).toBeGreaterThan(0) - for (const style of styles) { - expect(style.fill).toBe(BLACK) - expect(style.stroke).toBe(WHITE) - } - } - - // ...because set identity is carried by the pin lines, which keep the colour - const lineColors = await gramFramePage.page.evaluate((ids) => { - return ids.map((id) => { - const line = document.querySelector(`.gram-frame-harmonic-line[data-harmonic-set-id="${id}"]`) - return line ? String(line.getAttribute('stroke')).toLowerCase() : null - }) - }, [firstId, secondId]) - expect(lineColors[0]).toBe(colors[sets.findIndex((/** @type {any} */ s) => s.id === firstId)]) - expect(lineColors[1]).toBe(colors[sets.findIndex((/** @type {any} */ s) => s.id === secondId)]) - }) - - test('labels stay haloed after a zoom re-render', async ({ gramFramePage }) => { - const setId = await gramFramePage.addHarmonicSet(30, 20) - - await gramFramePage.setZoom(2.0, 0.5, 0.5) - - const styles = await gramFramePage.getHarmonicLabelStyles(setId) - expect(styles.length).toBeGreaterThan(0) - for (const style of styles) { - expect(style.fill).toBe(BLACK) - expect(style.stroke).toBe(WHITE) - expect(style.paintOrder).toMatch(/^stroke/) - } - }) -}) diff --git a/tests/harmonic-label-plate.spec.js b/tests/harmonic-label-plate.spec.js new file mode 100644 index 0000000..ae3ee61 --- /dev/null +++ b/tests/harmonic-label-plate.spec.js @@ -0,0 +1,132 @@ +import { test, expect } from './helpers/fixtures.js' + +/** + * @fileoverview E2E tests for the harmonic pin label plate (issue #243) — the + * number labels are drawn as black digits on an opaque white rounded rectangle + * instead of in the harmonic set's colour, so they stay legible over both dark + * and light areas of a gram. The plate replaced a halo (a white outline behind + * the glyphs), which left the gram showing through between and inside the + * digits; these tests assert the plate is there, is opaque white, and actually + * covers the characters it carries. + * + * Set identity must still be carried by the pin's line (and symbol) colour, so + * these tests also assert the line keeps the set colour. + * + * Debug config spans freq 0-100 Hz over time 0-60 s, so a 20 Hz set places + * harmonics 1..5 — every pin labelled. + */ + +const BLACK = 'rgb(0, 0, 0)' +const WHITE = 'rgb(255, 255, 255)' + +/** + * Set the colour the next created feature will take (the colour picker's + * selection), via the test-only instance registry. + * @param {import('./helpers/gram-frame-page.js').GramFramePage} gfp - Page helper + * @param {string} color - Hex colour + * @returns {Promise} + */ +async function setSelectedColor(gfp, color) { + await gfp.page.evaluate((c) => { + // @ts-ignore - test-only global + const instance = window.GramFrame.__test__getInstances()[0] + instance.state.selectedColor = c + }, color) +} + +test.describe('Harmonic pin label plate', () => { + test.beforeEach(async ({ gramFramePage }) => { + await gramFramePage.clickMode('Harmonics') + await gramFramePage.waitForImageDimensions() + }) + + test('every pin number label is black digits on a white rounded plate', async ({ gramFramePage }) => { + const setId = await gramFramePage.addHarmonicSet(30, 20) + + const labels = await gramFramePage.getHarmonicLabelPaint(setId) + expect(labels.length).toBeGreaterThan(0) + + for (const label of labels) { + expect(label.fill).toBe(BLACK) + // No halo left on the glyphs: the plate is the contrast now, and a + // leftover stroke would just thicken the digits over it. + expect(label.stroke).toBe('none') + + expect(label.plate).not.toBeNull() + expect(label.plate?.fill).toBe(WHITE) + // Rounded, as on the legacy viewer — but not so round it turns into a pill + expect(label.plate?.radius).toBeGreaterThan(0) + expect(label.plate?.radius).toBeLessThanOrEqual(6) + } + }) + + test('the plate covers the digits it carries, with room around them', async ({ gramFramePage }) => { + const setId = await gramFramePage.addHarmonicSet(30, 20) + + const labels = await gramFramePage.getHarmonicLabelPaint(setId) + expect(labels.length).toBeGreaterThan(0) + + for (const { plate, textBox } of labels) { + expect(plate).not.toBeNull() + if (!plate) continue + // Every side of the characters is inside the plate — a plate that merely + // overlapped them would leave the same show-through the halo did. + expect(plate.box.left).toBeLessThanOrEqual(textBox.left) + expect(plate.box.right).toBeGreaterThanOrEqual(textBox.right) + expect(plate.box.top).toBeLessThanOrEqual(textBox.top) + expect(plate.box.bottom).toBeGreaterThanOrEqual(textBox.bottom) + // And it is bigger than them, so the contrast reaches past the glyphs + expect(plate.box.right - plate.box.left).toBeGreaterThan(textBox.right - textBox.left) + } + }) + + test('label paint is independent of the set colour, which stays on the pin line', async ({ gramFramePage }) => { + // A new set takes the currently selected colour, so pick a different one + // before each add to get two visibly distinct sets. + await setSelectedColor(gramFramePage, '#ff6b6b') + const firstId = await gramFramePage.addHarmonicSet(20, 20) + await setSelectedColor(gramFramePage, '#45b7d1') + const secondId = await gramFramePage.addHarmonicSet(40, 25) + + const state = await gramFramePage.getState() + const sets = state.harmonics.harmonicSets + expect(sets.length).toBe(2) + const colors = sets.map((/** @type {any} */ s) => String(s.color).toLowerCase()) + expect(colors[0]).not.toBe(colors[1]) + + // Both sets' labels are painted identically — colour no longer distinguishes them + for (const setId of [firstId, secondId]) { + const labels = await gramFramePage.getHarmonicLabelPaint(setId) + expect(labels.length).toBeGreaterThan(0) + for (const label of labels) { + expect(label.fill).toBe(BLACK) + expect(label.plate?.fill).toBe(WHITE) + } + } + + // ...because set identity is carried by the pin lines, which keep the colour + const lineColors = await gramFramePage.page.evaluate((ids) => { + return ids.map((id) => { + const line = document.querySelector(`.gram-frame-harmonic-line[data-harmonic-set-id="${id}"]`) + return line ? String(line.getAttribute('stroke')).toLowerCase() : null + }) + }, [firstId, secondId]) + expect(lineColors[0]).toBe(colors[sets.findIndex((/** @type {any} */ s) => s.id === firstId)]) + expect(lineColors[1]).toBe(colors[sets.findIndex((/** @type {any} */ s) => s.id === secondId)]) + }) + + test('labels stay plated after a zoom re-render', async ({ gramFramePage }) => { + const setId = await gramFramePage.addHarmonicSet(30, 20) + + await gramFramePage.setZoom(2.0, 0.5, 0.5) + + const labels = await gramFramePage.getHarmonicLabelPaint(setId) + expect(labels.length).toBeGreaterThan(0) + for (const label of labels) { + expect(label.fill).toBe(BLACK) + expect(label.plate?.fill).toBe(WHITE) + expect(label.plate?.box.left).toBeLessThanOrEqual(label.textBox.left) + expect(label.plate?.box.right).toBeGreaterThanOrEqual(label.textBox.right) + } + }) +}) diff --git a/tests/helpers/gram-frame-page.js b/tests/helpers/gram-frame-page.js index 93d36ec..89fe571 100644 --- a/tests/helpers/gram-frame-page.js +++ b/tests/helpers/gram-frame-page.js @@ -742,25 +742,43 @@ class GramFramePage { } /** - * Read the resolved paint of every rendered harmonic number label, optionally - * scoped to a single set. Uses computed style (not attributes) so a CSS rule - * overriding the halo would be caught. + * Read how every rendered harmonic number label is painted, optionally scoped + * to a single set: the resolved paint of the digits, and the white plate + * drawn behind them (issue #243). Uses computed style (not attributes) so a + * CSS rule overriding either would be caught, and reports both boxes so a + * test can check the plate actually covers the characters. * @param {string} [setId] - Restrict to one harmonic set - * @returns {Promise>} + * @returns {Promise>} */ - async getHarmonicLabelStyles(setId) { + async getHarmonicLabelPaint(setId) { const selector = setId ? `.gram-frame-harmonic-number[data-harmonic-set-id="${setId}"]` : '.gram-frame-harmonic-number' return this.page.evaluate((sel) => { + /** + * @param {Element} el - Element to box + * @returns {{left: number, right: number, top: number, bottom: number}} Its viewport box + */ + const box = (el) => { + const r = el.getBoundingClientRect() + return { left: r.left, right: r.right, top: r.top, bottom: r.bottom } + } return Array.from(document.querySelectorAll(sel)).map((el) => { const style = window.getComputedStyle(el) + const plate = el.parentElement + ? el.parentElement.querySelector('.gram-frame-label-plate') + : null return { fill: style.fill, stroke: style.stroke, - strokeWidth: style.strokeWidth, - strokeLinejoin: style.strokeLinejoin, - paintOrder: style.paintOrder + plate: plate + ? { + fill: window.getComputedStyle(plate).fill, + radius: Number(plate.getAttribute('rx')), + box: box(plate) + } + : null, + textBox: box(el) } }) }, selector) @@ -936,16 +954,29 @@ class GramFramePage { } /** - * Read a marker's on-gram label element, if it has one. + * Read a marker's on-gram label element, if it has one — including the white + * plate drawn behind it (issue #243) and how the two are boxed on screen, so + * a test can check the plate covers the characters. * @param {string} markerId - Marker to inspect - * @returns {Promise<{text: string, x: number, y: number, textAnchor: string, fill: string, stroke: string, paintOrder: string}|null>} + * @returns {Promise<{text: string, x: number, y: number, textAnchor: string, fill: string, stroke: string, textBox: {left: number, right: number, top: number, bottom: number}, plate: null|{fill: string, radius: number, box: {left: number, right: number, top: number, bottom: number}}}|null>} */ async getMarkerLabelOverlay(markerId) { return this.page.evaluate((id) => { + /** + * @param {Element} node - Element to box + * @returns {{left: number, right: number, top: number, bottom: number}} Its viewport box + */ + const box = (node) => { + const r = node.getBoundingClientRect() + return { left: r.left, right: r.right, top: r.top, bottom: r.bottom } + } const el = document.querySelector( `.gram-frame-analysis-marker[data-marker-id="${id}"] .gram-frame-marker-label` ) if (!el) return null + const plate = el.parentElement + ? el.parentElement.querySelector('.gram-frame-label-plate') + : null return { text: el.textContent || '', x: parseFloat(el.getAttribute('x') || '0'), @@ -953,7 +984,14 @@ class GramFramePage { textAnchor: el.getAttribute('text-anchor') || '', fill: el.getAttribute('fill') || '', stroke: el.getAttribute('stroke') || '', - paintOrder: el.getAttribute('paint-order') || '' + textBox: box(el), + plate: plate + ? { + fill: plate.getAttribute('fill') || '', + radius: Number(plate.getAttribute('rx')), + box: box(plate) + } + : null } }, markerId) } diff --git a/tests/marker-labels.spec.js b/tests/marker-labels.spec.js index 4969625..7da7127 100644 --- a/tests/marker-labels.spec.js +++ b/tests/marker-labels.spec.js @@ -309,14 +309,54 @@ test.describe('Marker labels', () => { expect(overlay.y).toBeGreaterThan(symbol.bottom) }) - test('the label is drawn black inside a white halo, painted behind the glyphs', async ({ gramFramePage }) => { + // Issue #243: the label used to be haloed (a white outline behind the glyphs), + // which let the gram show through between and inside the characters. It now + // sits on an opaque white plate, as the legacy viewer's labels do. + test('the label is drawn black on a white rounded plate', async ({ gramFramePage }) => { const markerId = await placeMarker(gramFramePage, 220, 160) - await gramFramePage.setMarkerLabel(markerId, 'Halo') + await gramFramePage.setMarkerLabel(markerId, 'Plate') const overlay = await gramFramePage.getMarkerLabelOverlay(markerId) expect(overlay.fill).toBe('#000') - expect(overlay.stroke).toBe('#fff') - expect(overlay.paintOrder).toBe('stroke fill') + // No halo stroke left on the glyphs: the plate is the contrast now + expect(overlay.stroke).toBe('') + expect(overlay.plate).not.toBeNull() + expect(overlay.plate.fill).toBe('#fff') + expect(overlay.plate.radius).toBeGreaterThan(0) + }) + + test('the plate covers the label it carries, with room around it', async ({ gramFramePage }) => { + const markerId = await placeMarker(gramFramePage, 220, 160) + await gramFramePage.setMarkerLabel(markerId, 'Contact Alpha') + + const { plate, textBox } = await gramFramePage.getMarkerLabelOverlay(markerId) + expect(plate).not.toBeNull() + expect(plate.box.left).toBeLessThanOrEqual(textBox.left) + expect(plate.box.right).toBeGreaterThanOrEqual(textBox.right) + expect(plate.box.top).toBeLessThanOrEqual(textBox.top) + expect(plate.box.bottom).toBeGreaterThanOrEqual(textBox.bottom) + // Wider than the text, so the contrast reaches past the characters + expect(plate.box.right - plate.box.left).toBeGreaterThan(textBox.right - textBox.left) + }) + + test('the plate stays clear of the crosshair it annotates', async ({ gramFramePage }) => { + const markerId = await placeMarker(gramFramePage, 220, 160) + await gramFramePage.setMarkerLabel(markerId, 'Clear') + + const { plate } = await gramFramePage.getMarkerLabelOverlay(markerId) + const centre = await gramFramePage.page.evaluate((id) => { + const dot = document.querySelector( + `.gram-frame-analysis-marker[data-marker-id="${id}"] circle` + ) + const r = dot.getBoundingClientRect() + return { x: (r.left + r.right) / 2, y: (r.top + r.bottom) / 2, right: r.right, top: r.top } + }, markerId) + + // Upper-right quadrant, with the PLATE — not just the text — outside the + // crosshair's centre dot, so the white rectangle never covers the point the + // marker is on. + expect(plate.box.left).toBeGreaterThanOrEqual(centre.right) + expect(plate.box.bottom).toBeLessThanOrEqual(centre.top) }) test('the label moves with the marker when it is dragged', async ({ gramFramePage }) => { diff --git a/tests/unit/label-plate.test.js b/tests/unit/label-plate.test.js new file mode 100644 index 0000000..4bf3391 --- /dev/null +++ b/tests/unit/label-plate.test.js @@ -0,0 +1,126 @@ +import { describe, it, expect } from 'vitest' +import { + LABEL_PLATE_PADDING_X, + labelPlateExtents, + labelPlateRect, + measureLabelWidth +} from '../../src/utils/labelPlate.js' + +/** + * @fileoverview Unit coverage for the label plate's geometry (issue #243). + * + * The white rounded rectangle behind every on-gram label is sized by pure + * arithmetic, which is what lets both placement rules that must leave room for + * it — `markerLabelPlacement` and `PinSetMode.labelStackPositions` — measure + * their gaps to the plate's edge rather than to the baseline. That arithmetic + * is tested here; the drawn element is covered in tests/harmonic-label-plate.spec.js + * and tests/marker-labels.spec.js. + */ + +const FONT_SIZE = 12 + +describe('labelPlateExtents', () => { + it('reaches further above the baseline than below it', () => { + const { above, below } = labelPlateExtents(FONT_SIZE) + + // Glyphs hang above their baseline; only descenders drop past it. + expect(above).toBeGreaterThan(below) + expect(below).toBeGreaterThan(0) + }) + + it('clears the cap height of the font, and the descenders', () => { + const { above, below } = labelPlateExtents(FONT_SIZE) + + // Bold Arial: cap height ~0.72 em, descenders ~0.21 em. + expect(above).toBeGreaterThan(FONT_SIZE * 0.72) + expect(below).toBeGreaterThan(FONT_SIZE * 0.21) + // ...without the plate growing taller than a line of text needs + expect(above + below).toBeLessThan(FONT_SIZE * 1.5) + }) + + it('scales with the font size', () => { + const small = labelPlateExtents(10) + const large = labelPlateExtents(20) + + expect(large.above).toBeCloseTo(small.above * 2) + expect(large.below).toBeCloseTo(small.below * 2) + }) +}) + +describe('labelPlateRect', () => { + const WIDTH = 20 + const base = { x: 100, y: 80, width: WIDTH, fontSize: FONT_SIZE } + + it('pads the text horizontally on both sides', () => { + const rect = labelPlateRect({ ...base, textAnchor: 'start' }) + + expect(rect.width).toBe(WIDTH + LABEL_PLATE_PADDING_X * 2) + expect(rect.x).toBe(base.x - LABEL_PLATE_PADDING_X) + }) + + it('is as tall as the plate reaches either side of the baseline', () => { + const { above, below } = labelPlateExtents(FONT_SIZE) + const rect = labelPlateRect({ ...base, textAnchor: 'middle' }) + + expect(rect.y).toBe(base.y - above) + expect(rect.height).toBeCloseTo(above + below) + }) + + it('centres on the anchor for middle-anchored text', () => { + const rect = labelPlateRect({ ...base, textAnchor: 'middle' }) + + expect(rect.x + rect.width / 2).toBeCloseTo(base.x) + }) + + it('ends at the anchor for end-anchored text', () => { + const rect = labelPlateRect({ ...base, textAnchor: 'end' }) + + expect(rect.x + rect.width).toBe(base.x + LABEL_PLATE_PADDING_X) + }) + + it('covers the text whatever the anchor, with room to spare', () => { + for (const textAnchor of ['start', 'middle', 'end']) { + const rect = labelPlateRect({ ...base, textAnchor }) + const textLeft = textAnchor === 'start' + ? base.x + : textAnchor === 'middle' ? base.x - WIDTH / 2 : base.x - WIDTH + + expect(rect.x).toBeLessThan(textLeft) + expect(rect.x + rect.width).toBeGreaterThan(textLeft + WIDTH) + } + }) + + it('follows the label when it moves', () => { + const moved = labelPlateRect({ ...base, x: base.x + 25, y: base.y - 10, textAnchor: 'middle' }) + const rect = labelPlateRect({ ...base, textAnchor: 'middle' }) + + expect(moved.x - rect.x).toBe(25) + expect(moved.y - rect.y).toBe(-10) + }) +}) + +describe('measureLabelWidth', () => { + // No canvas in the Node lane, so these exercise the character-count fallback + // the browser path falls back to when measurement is unavailable. + it('grows with the number of characters', () => { + expect(measureLabelWidth('12', FONT_SIZE)).toBeGreaterThan(measureLabelWidth('1', FONT_SIZE)) + }) + + it('grows with the font size', () => { + expect(measureLabelWidth('12', 24)).toBeGreaterThan(measureLabelWidth('12', 12)) + }) + + it('is zero-width for no text, and survives an absent one', () => { + expect(measureLabelWidth('', FONT_SIZE)).toBe(0) + expect(measureLabelWidth(null, FONT_SIZE)).toBe(0) + expect(measureLabelWidth(undefined, FONT_SIZE)).toBe(0) + }) + + it('estimates a plausible width for bold Arial digits', () => { + // Roughly 0.6 em per digit — enough that a plate sized from it holds them. + const width = measureLabelWidth('123', FONT_SIZE) + + expect(width).toBeGreaterThan(FONT_SIZE) + expect(width).toBeLessThan(FONT_SIZE * 3) + }) +})