From 192a2626b843f538a4154a2d278496d1015b6fea Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 13:38:45 +0000 Subject: [PATCH] Draw on-gram labels on a white plate instead of a halo (#243) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harmonic and sideband pin numbers, and analysis marker labels, were drawn as black glyphs inside a white halo — a stroke painted behind the letterforms. A halo only whitens the pixels immediately around each stroke, so a noisy gram still shows through the counters of the digits and the space between them. The legacy spectrogram viewer plates its harmonic numbers instead: an opaque white rounded rectangle containing the characters, which clears one contiguous area and reads the same everywhere in the label. Every in-gram label now sits on such a plate: - New `src/utils/labelPlate.js` owns the plate. `plateLabel()` wraps a fully-attributed `` in a group with the rectangle behind it, sized from the text measured at the font it is drawn in (canvas measurement, with a character-count fallback for the Node unit lane). The geometry is pure so the placement rules can size their gaps from it. - The two placement rules — `markerLabelPlacement` and `PinSetMode.labelStackPositions` — now measure their gaps to the edge of the plate rather than to the baseline, so the white rectangle clears the symbol, the crosshair and the image's top edge by as much as the bare glyphs used to. A pin's grab region tracks the plate too. - The text keeps its class and attributes, so selectors, CSS and tests still find the label where they did. Labels stay black-on-white whatever colour the feature is; identity is still carried by the pin's line and symbol. Closes #243 --- CLAUDE.md | 5 +- docs/Adding-Graphical-Features.md | 5 +- docs/Gram-Modes.md | 4 +- docs/Rendering-Troubleshooting.md | 3 +- docs/Tech-Architecture.md | 10 +- src/gramframe.css | 23 ++- src/modes/shared/PinSetMode.js | 82 ++++----- src/rendering/labels.js | 19 ++- src/utils/cursors.js | 9 +- src/utils/labelPlate.js | 257 +++++++++++++++++++++++++++++ src/utils/markerLabel.js | 35 ++-- src/utils/svg.js | 56 ------- tests/harmonic-label-halo.spec.js | 106 ------------ tests/harmonic-label-plate.spec.js | 132 +++++++++++++++ tests/helpers/gram-frame-page.js | 60 +++++-- tests/marker-labels.spec.js | 48 +++++- tests/unit/label-plate.test.js | 126 ++++++++++++++ 17 files changed, 721 insertions(+), 259 deletions(-) create mode 100644 src/utils/labelPlate.js delete mode 100644 src/utils/svg.js delete mode 100644 tests/harmonic-label-halo.spec.js create mode 100644 tests/harmonic-label-plate.spec.js create mode 100644 tests/unit/label-plate.test.js 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) + }) +})