diff --git a/CLAUDE.md b/CLAUDE.md index 71038014..ac199eca 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,6 +59,7 @@ curl -d "status here" ntfy.sh/iancc2025 - **Main Class**: `GramFrame` in `src/main.js` - Central component managing all functionality - **Entry Point**: `src/index.js` - Main module export and global registration - **State Management**: `src/core/state.js` - Centralized state with listener pattern +- **Control Row**: five columns above the gram — the mode rail, the armed mode's guidance, the cursor readouts, the style panel and the three annotation tables. Assembled by `src/components/MainUI.js`; see [Control-Panel.md](docs/Control-Panel.md) - **Mode System**: Modular architecture with five modes — Pan (default), Analysis, Harmonics, Sidebands and Doppler. `analysis` is the code and storage name; the button an analyst sees reads **Cross Cursor** (`modeRoster.js`, and see [Gram-Modes.md](docs/Gram-Modes.md)) - **Spectrograph Player**: `src/audio/` (decode and analyse a WAV into the gram image) and `src/player/` (the transport and the scrolling view) make an audio-sourced instance; the modes measure it through the unchanged coordinate pipeline (spec 168, ADR-019) - **Feature Rendering**: `src/core/FeatureRenderer.js` - Cross-mode feature coordination @@ -110,6 +111,10 @@ Every path below exists; keep this list in step with `src/` when adding modules. gesture pairs a pan with a pause and a resume time. Also owns what a *click* (a press that never moved) means, both ways: pause, and — for Pan mode only — resume + - `bookmarks.js` - Time bookmarks: playback chrome, in-memory, never saved + with the gram's annotations + - `transportKeys.js` - The transport's keyboard shortcuts (Space/K, J/L, Home, + M, B) - `playerView.js` - The waterfall geometry: `viewTop`, its clamp, the follow loop, the reveal rule, and the time read off a click on the time axis - `gramRepaint.js` - The painted 8-bit levels, kept per instance in a @@ -117,6 +122,14 @@ Every path below exists; keep this list in step with `src/` when adding modules. of colour map is a repaint rather than a second analysis - `src/core/` - Core system modules: - `state.js` - State management and listeners + - `featureStyle.js` - What the style controls act on — the defaults for the + next feature, or the selected one — and the four restyle operations. One + gate (`getSelectedFeature`) decides, so the tab an analyst can see and what + a colour click does cannot disagree + - `panelRefresh.js` - "The tables are stale, redraw them", asked from the + selection layer, the restyle layer and the control row + - `preferences.js` - The two chrome preferences: the pin style (per session) + and the guidance collapse (per user) - `annotationCommit.js` - `commitAnnotationChange`: the one cadence every annotation mutation performs — mark it changed, refresh the panel showing it, re-render the overlay, dispatch (R9-13). A leaf over `state.js`; nothing in `state.js` imports back @@ -131,7 +144,15 @@ Every path below exists; keep this list in step with `src/` when adding modules. read-merge-write, so two tabs on one gram are additive rather than last-writer-wins; deletions travel as tombstones, because a union cannot otherwise tell "never had it" from "deleted it" (issue #269) - - `keyboardControl.js` - Arrow-key control, selection and restyling + - `keyboardControl.js` - Arrow-key control and restyling + - `selection.js` - Which feature is selected: setting it, clearing it, and the + five things that follow (the tables re-diff, the readout retargets, the + style panel re-arms, and on a player the view scrolls to it). Split from + `keyboardControl.js`, which is about arrow keys — selection is what they act + *on*, not part of how they work + - `selectionTarget.js` - What is selected, described: its name, its time, the + frequency it is about. A leaf, so the readout column and the player can both + ask without a cycle back through `selection.js` - `wheelPan.js` - The middle-button pan, as a drag on the shared engine. Split from `events.js`, which owns *which* gesture happens rather than how each one behaves @@ -156,17 +177,17 @@ Every path below exists; keep this list in step with `src/` when adding modules. pulling in every mode class (issue #263) - `analysis/AnalysisMode.js` - Analysis mode with marker persistence - `harmonics/HarmonicsMode.js` - Harmonics calculation mode (a `PinSetMode`) - - `harmonics/ManualHarmonicModal.js` - Manual harmonic-spacing dialog. Like - `MarkerLabelModal`, built with `createElement` and class-scoped selectors — - no page-global ids escape into the host document; Escape is bound on the - document so it works wherever the focus is; and closing hands focus back to - the button that opened it (issue #260) + - `harmonics/ManualHarmonicModal.js` - Manual harmonic-spacing dialog. Built + with `createElement` and class-scoped selectors — no page-global ids escape + into the host document; Escape is bound on the document so it works wherever + the focus is; and closing hands focus back to the button that opened it + (issue #260). The symbol popup follows the same three rules - `sideband/SidebandMode.js` - Sidebands mode: a pin set whose origin the analyst places (a `PinSetMode`) - `doppler/DopplerMode.js` - Doppler speed calculation mode - `pan/PanMode.js` - Pan mode (the default mode) - `capabilities.js` - Duck-typed mode capabilities (`PersistentFeatureProvider`, - `PanelOwner`, `PinSetOwner`) and their predicates. How `FeatureRenderer` and `MainUI` find + `PanelOwner`, `PinSetOwner`, `MarkerOwner`) and their predicates. How `FeatureRenderer` and `MainUI` find what a mode can do without naming it (ADR-017) - `shared/BaseDragHandler.js` - The shared drag engine: every pointer drag (move, create, place, pan) and the single `state.drag` projection - `shared/PinSetMode.js` - The shared pin-set mode: pin geometry, the @@ -174,25 +195,42 @@ Every path below exists; keep this list in step with `src/` when adding modules. add/update/remove for Harmonics and Sidebands. A subclass supplies only where its sets live and what frequency a member index maps to - `src/components/` - UI component modules: - - `UIComponents.js` - LED displays, colour picker and layout helpers - - `MainUI.js` - Unified layout and persistent panels - - `ModeButtons.js` - Mode switching interface. A group is the mode button - followed by that mode's commands - - `icons.js` - The button glyphs (the pan hand, the fit frame), drawn as - inline SVG in `currentColor` so they follow the button's states as text - does, and always paired with a visually hidden word + - `UIComponents.js` - What is left of the shared UI helpers: the LED re-export + - `MainUI.js` - The control row's five columns, assembled from the modules below + - `ModeButtons.js` - The mode rail: five stacked tools, then the view controls + (zoom out, zoom in, fit) in a footer beneath them + - `GuidancePanel.js` - The armed mode's guidance column, and the 40px rail it + collapses to. Collapse is remembered per user; with no stored choice the + column decides by the panel's own width + - `CursorReadout.js` - The instrument face: frequency as the one large, lit + reading, with time immediately beneath it and doppler speed at the foot, + both at half the size in the label grey. Reads the pointer, or the selected + feature when there is one + - `AnnotationTables.js` - The three table columns, their headers, count chips + and the "Clear all annotations" footer + - `icons.js` - The button glyphs, drawn as inline SVG in `currentColor` so + they follow the button's states as text does. A mode's glyph rides beside + its word; a view control's replaces it, keeping the word in a visually + hidden span - `HarmonicPanel.js` - Harmonics display panel - `SidebandPanel.js` - Sidebands display panel (its own column beside the harmonics panel; both are always visible) - - `DiffingTable.js` - Shared row-diffing table behind the markers table and harmonics panel - - `ColorPicker.js` - Colour selection component - - `SymbolPicker.js` - Symbol selection component - - `MarkerLabelModal.js` - Add/edit/remove a marker's label - - `PinToggle.js` - Harmonic-pin visibility toggle + - `DiffingTable.js` - Shared row-diffing table behind all three tables + - `tableScroll.js` - Keeping a newly added or newly selected row in view + - `StylePanel.js` - The style panel and its twin target tabs + - `styleTarget.js` - What the panel is pointed at, and the four things it can + do to it (arm, rename, delete, describe) + - `ColorPicker.js` - The colour slider: the one colour control in the panel + - `SymbolPicker.js` - The symbol button and its popup, plus the size trial + - `Segmented.js` - The shared two-option segmented control + - `PinToggle.js` - Tall pins or mini, for the next created pin set or the + selected one - `ExpandToggle.js` - Expand/collapse the image to fill the space - `StorageWarning.js` - Non-blocking banner when a save fails - `TransportBar.js` - The playback controls under an audio-sourced gram, the - visible time span, and the polite live region a screen reader hears + scrub track, the visible time span, and the polite live region a screen + reader hears + - `TransportBookmarks.js` - The bookmark flags on that track and the saved list - `DisplayRangeControls.js` - The contrast floor and ceiling, on that bar - `ColourMapChoice.js` - The colour-map radio row on that bar — Colour, Grey and the four uniform maps — as `player.analysis.colourMap`. Radios rather @@ -218,6 +256,10 @@ Every path below exists; keep this list in step with `src/` when adding modules. - `displayFilter.js` - The contrast controls' `feComponentTransfer`: built into the SVG's defs on first use, removed entirely at the resting positions so the default is the image as it loaded + - `selectionHalo.js` - What a selected feature looks like on the gram: its own + geometry redrawn beneath itself, wider and in translucent white, plus the + inverted label plate its table row also gets. A pass over the finished + overlay, so selection costs a few elements rather than a re-render - `markerGlyph.js` - What an analysis marker is drawn as: the crosshair, or the shaped symbol that replaces it. `drawsCrosshair` is the one answer to "does this marker have arms?", so the hit test asks it rather than re-deriving the @@ -249,8 +291,12 @@ Every path below exists; keep this list in step with `src/` when adding modules. gram under the hotspot stay visible; panning keeps the hand - `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 + - `secureHTML.js` - Guidance-panel rendering without innerHTML. Builds DOM and + nothing else; the deciding is `guidanceContent.js`'s - `timeFormatter.js` - Time formatting utilities + - `guidanceContent.js` - What a mode's guidance says, as data: which sections + it has and whether each line carries a trigger. Pure, so the unit lane + covers the branches `secureHTML.js` cannot be loaded to exercise - `navigationGuidance.js` - The cross-mode navigation guidance text (wheel zoom and pan, the wheel-button drag, Shift + drag region zoom) - `version.js` - Version constant (injected at build time) @@ -487,12 +533,23 @@ There is no visual/screenshot regression testing — see analyst is looking. The `+`/`−` buttons have no pointer on the gram to follow, so they hold the middle of the view instead - **Pan Mode**: The default mode; drag to pan when zoomed in, so a first click never places anything -- **The control row**: each mode's group is `[mode button] [its commands]`, and - Pan's is the only group with commands — zoom out, zoom in, fit — so it is the - only row where four controls share the column. Pan and fit therefore show - glyphs rather than words (`components/icons.js`), each keeping its word in a - visually hidden span so the accessible name, and every test selector, is - still "Pan" and "Fit" (issue #310) +- **The control row**: five columns — the mode rail, the armed mode's guidance, + the cursor readouts, the style panel and the three annotation tables — + separated by hairlines rather than boxed, so the readouts are the first read. + The mode rail stacks the five tools with a glyph beside each word, and puts + the view controls (zoom out, zoom in, fit) in a footer beneath them, where + they stay whichever mode is armed: they act on the view, not on the tool. Fit + shows a glyph in place of its word, keeping the word in a visually hidden span + so the accessible name, and every test selector, is still "Fit" (issue #310). + See [Control-Panel.md](docs/Control-Panel.md) +- **The style panel's twin tabs**: "New features" or "Selected: " — the + panel states what it is about to change, and the analyst can arm either + without giving up their selection (`state.styleTarget`). One gate, + `getSelectedFeature` in `core/featureStyle.js`, decides what a colour click + actually does, so the tab and the behaviour cannot disagree +- **Marker labels** are edited in the style panel, in a field beside the same + marker's colour and symbol. The per-row dialog is gone; clearing the field + removes the label - **Analysis Mode**: Persistent draggable markers whose grab region follows exactly what is drawn — a symbol marker has no crosshair arms to grab (issue #273) — with cross-mode visibility and optional @@ -522,6 +579,7 @@ There is no visual/screenshot regression testing — see - Unchanged — Web Storage (`localStorage` trainer / `sessionStorage` student). No persisted-shape change in this phase. (167-structural-refactor) ## Recent Changes +- 172-control-panel: The upper control panel rebuilt to the design handoff — five hairline-separated columns, a mode rail with the view controls in its foot, a permanent collapsible guidance column, instrument-styled readouts, a twin-tab style panel that states its target, inverted selected rows, and a transport bar with time bookmarks - Site structure: one landing page instead of two competing ones, the demo pages committed at the paths they are published at, and one script assembling the Pages tree for both the deploy and the PR preview. `yarn check:site` fails a PR diff --git a/docs/Control-Panel.md b/docs/Control-Panel.md new file mode 100644 index 00000000..c2c1edbc --- /dev/null +++ b/docs/Control-Panel.md @@ -0,0 +1,253 @@ +# The control panel + +The row of controls above the gram. This describes what it is, why it is +arranged the way it is, and which module owns each part. + +The plot area below it is unchanged and out of scope: the gram, the axes and +every overlay are drawn exactly as before. + +## The problem it solves + +The old panel was five bordered boxes of equal visual weight. Nothing in it was +the first thing to read, so the eye had no entry point; per-mode guidance had +nowhere permanent to live; and it was never clear whether the colour slider was +setting the style of the *next* feature or restyling the one just clicked. + +The redesign is about rank, not content. Almost everything that was in the panel +is still in it. + +## The five columns + +One 250px row, `display: flex`, on the panel surface. Columns are separated by a +single hairline rather than boxed. + +| # | Column | Width | Module | +|---|--------|-------|--------| +| 1 | Mode rail | 154px | `components/ModeButtons.js` | +| 2 | Guidance | 264px, or a 40px rail | `components/GuidancePanel.js` | +| 3 | Cursor readouts | 210px | `components/CursorReadout.js` | +| 4 | Style | 222px | `components/StylePanel.js` | +| 5 | Annotation tables | the rest | `components/AnnotationTables.js` | + +`components/MainUI.js` assembles them and builds none of them. + +The first four are fixed: the instrument face and the tool list are what the +panel is for, so a narrow host squeezes the tables first and then collapses the +guidance to its rail. That decision is made by a **container query** on the +component, not a viewport media query — the same page can carry a full-width +gram and a half-width one. + +### 1. The mode rail + +Five modes stacked in roster order, each showing its glyph beside its word. The +armed mode takes an accent left border and a tinted ground; the border is +transparent rather than absent at rest, so arming changes no geometry and rows +do not shift under the pointer. + +Beneath them, fenced off by a rule, are the **view controls**: zoom out, zoom +in, fit. They are declared by Pan — the mode that owns the viewport — but they +are drawn here in every mode, because zooming and fitting are things an analyst +does *while* measuring rather than instead of it. They used to sit inside Pan's +own row, which made one row of five four controls wide. + +Clear is deliberately not here. It removes annotations, not view state; see +column 5. + +### 2. Guidance + +Immediately beside the rail, so the armed tool and the gestures that drive it +read as one block. Its space is dedicated: collapsing it hands the width to the +annotation tables and nothing else ever moves in. + +Each line is split into a **trigger** in a fixed 76px track and an **outcome** +beside it. That split is the whole point: the four gestures of a mode compare +down one column instead of having to be read out of four sentences. The shape +is `{trigger, outcome}` objects returned from each mode's `getGuidanceText()` +and rendered by `utils/secureHTML.js`, which remains the only path guidance +takes to the DOM. + +Beneath a mode's own lines, every mode carries an **In every mode** section: +the wheel zoom and pan, the wheel-button drag, and Shift + drag region zoom. +They are resolved centrally, ahead of mode delegation, so they have always +worked everywhere — but they were listed in Pan's guidance alone, because Pan +is the initial mode and the old panel had room for them nowhere else. An +analyst who armed Cross Cursor first therefore never learnt that Shift + drag +zooms. `utils/guidanceContent.js`'s `withNavigationGuidance` appends the +section, so no mode has to remember to. + +Collapse has three states, not two: + +- **collapsed** — the analyst pressed Hide. +- **open** — the analyst pressed the reveal. +- **automatic** — they have never touched it, and the column shows itself where + there is room and collapses where there is not. + +The stored preference distinguishes all three (`true` / `false` / absent). Were +"never chosen" folded into "open", the automatic behaviour could never fire. + +### 3. The cursor readouts + +The panel's anchor, and the only part styled as an instrument: recessed ground, +monospaced tabular numerals, and a faint bloom on the one reading that is lit. + +That reading is the **frequency**. In use it is by far the most important +number in the panel, and the first pass set time at the same size directly +above it, so the two competed for the eye and neither was the first read. Now +frequency is the one large reading, and **time** sits immediately beneath it at +half the size, captioned, in the same grey as the panel's labels rather than +in ink. Doppler speed is fenced off at the foot between two rules at that same +second rank, because it is a derived quantity rather than a coordinate. + +The column has two targets, named by its kicker: + +- **CURSOR** — it follows the pointer over the gram. +- **SELECTED <name>** — a feature is selected, and it shows that feature's + own values, so a marker's numbers can be read without hovering it and losing + them again on the way to the panel. + +`updateUniversalCursorReadouts` stands aside while something is selected, which +is what stops the two targets fighting over the same two numbers. + +What is selected — its name, its time, its frequency — is answered once, by +`core/selectionTarget.js`, because the readout is not the only thing that asks: +on a player, selecting a feature also **scrolls the view to it**. A recording is +one tall gram read through a window a few tens of seconds high, so a feature +placed ten minutes in is usually not on screen at all, and selecting its row +used to light up a row and write two readouts for something the analyst could +not see. `player/playerView.js`'s `revealTime` moves the view only when the time +is off screen — scrolling a visible feature into the middle would move the gram +under the eye on every row click — and only while paused, since the follow loop +owns the view during playback and would undo it on the next frame. It moves the +view, never the playhead: pressing play afterwards resumes from where the audio +actually is. Dragging the gram is the gesture that means "play from here". + +There is no intensity or colourmap legend. One was drawn and removed: without +calibrated units it tells the operator nothing they cannot see. + +### 4. The style panel + +Twin tabs across the top state what the controls are about to change: + +- **New features** — colour, symbol and pin style for everything added from now + on, in any mode. +- **Selected: <name>** — the same controls pointed at that feature, plus + the two things that only make sense for one that already exists: its label, + edited in place, and a nudge pair mirroring the arrow keys. Its footer offers + Delete. + +`state.styleTarget` records which is armed. Selecting a feature arms the second; +clicking the first arms the defaults **without** giving up the selection, so an +analyst can change what comes next while still nudging the row they are on. +`core/featureStyle.js`'s `getSelectedFeature` is the single gate: `getActiveStyle` +and all four `apply*ToSelectedFeature` functions read the answer from it. + +There is exactly one colour control in the panel. The second row of fixed +swatches is gone: two ways to set one property meant neither was the one that +said what the colour currently *was*. + +The symbol control is a button opening a popup of all seven shipped symbols, +drawn at the size and in the colour they will actually take. It also houses the +temporary large-symbol trial, which was a permanent control in the main panel +for a temporary question. + +**Marker labels** are edited here rather than in a dialog opened from the row. +Labels are optional; clearing the field removes the label. + +### 5. The annotation tables + +Markers, harmonics and sidebands, three equal columns, all visible at all times +whatever mode is armed. Each has a header with a count chip, hidden at zero, and +an instructional empty state rather than a blank rectangle. + +The **selected row is reversed out whole** — light ground, dark ink. It used to +be an accent border, which collided with the feature colours the rows themselves +carry: a green-bordered row holding a green marker said two things at once. +Inversion survives any feature colour. + +The gram says the same thing, which it previously did not: the panel could read +"Selected: Marker 3" while three identical crosses sat on the plot with nothing +to tell them apart. `rendering/selectionHalo.js` draws the selected feature's +own geometry a second time underneath itself, wider and in translucent white — +a **halo**. It was chosen over the alternatives because it is the only one that +generalises: a crosshair, a shaped symbol and a stack of forty pin lines all +take the same cue from the same code, and it is independent of the feature's +colour, which the tables learnt the hard way. Corner brackets read well on a +marker and badly on a tall pin set; dimming every other feature is unmissable +but destroys the comparison between features, which is usually why several +exist; a pulse has faded by the time the eye arrives. + +Labels are the exception, deliberately: a white glow behind a white plate says +nothing, so a selected feature's plate is **inverted** instead — the same +reversal its table row performs. That applies to a pin set's number labels as +much as to a marker's single one. Inverting a dozen of them at once is a heavier +change, but it is the same change, it is reversed the moment the set is +deselected, and issue #243's guarantee is about contrast rather than about white +specifically: light text on a dark plate clears the same contiguous rectangle a +dark-on-white one does, which is the whole point of a plate over a halo. It is a pass of its own rather than a flag +threaded through the renderers, because selection changes far more often than +the features do. + +The sidebands column carries a footer holding **Clear all annotations**, on +every page. It was trainer-only, on the reasoning that a student's work expires +overnight anyway — but "it will be gone tomorrow" is no answer to a student who +has mislabelled a gram and wants to start the exercise again today. Clearing is +the same operation either way; only where the annotations were stored differs. + +Sideband sets are rare, so the foot of that column is the space the panel has +going spare — and a destructive control among three view controls in the rail +would be one slip from an afternoon's work. + +## The transport bar + +Only present when the gram is audio-sourced, and mounted **below** the gram: +that is where the scrub track and its bookmark flags line up with the time axis +they refer to, and where the panel's height never changes when a recording +loads. + +The view scrolls back to the very start of a recording, not to one window above +it. The top edge of the plot area *is* the playhead, and drag-seek resumes from +whatever time that edge shows, so a lower bound of one window made the opening +seconds of every recording impossible to put under the playhead — visible at the +bottom of the view, but unreachable by dragging. Reaching zero means the window +below the start is blank, which is the same blankness the opening seconds +already showed while the first window played. + +**Time bookmarks** are the one new affordance. Bookmark (or the `B` key) flags +the playhead; each bookmark draws a numbered flag on the track that can be +clicked to jump, and the "*n* saved" button lists them with a way to remove each. +They are playback chrome, not annotation — a marker says what was measured and +belongs to the gram, a bookmark says where to listen again and belongs to the +sitting — so they live in memory for the life of the page and never reach the +annotation store. + +## Tokens + +Declared once on `.gram-frame-container` in `src/gramframe.css` and used through +the variables, so a change lands everywhere at once. + +- Ground `--gf-bg` #0d0e18 · Surface `--gf-surface` #171926 · Text `--gf-text` + #f7f7fa. The first palette was a shade lighter in the ground and a shade + darker in the ink; body text on the surface now reads at ~16:1 rather than + ~13:1, which is what a panel read beside a bright gram in a lit room needs +- Accent `--gf-accent` #9184d9 with a ramp: `-900` for tinted fills, + `-800`/`-700`/`-600` for borders, `-400` for hover, `-200`/`-100` for text on + those tints +- Muted ink is one value, `rgba(247,247,250,.70)` (~8.5:1 on the surface). + Alphas below .60 do not reach 4.5:1 on this ground and are not used for text +- Non-text tier (borders, hover tints, hairlines): `rgba(247,247,250,.05)`–`.18`, + and `rgba(0,0,0,.22)`–`.36` for recesses +- Danger is a hover-only affordance: border `#8d5a5a`, text `#e2b3b3`. Nothing + in the panel is red at rest +- Keyboard focus is one treatment everywhere: `outline: 2px solid var(--gf-accent)` + at `outline-offset: 2px` + +## What was considered and rejected + +- **New-user onboarding** (first-run cards, a guided mode). The interface should + be self-explanatory instead, which is what the permanent guidance column is + for. +- **Docking the transport into the panel.** It keeps all the chrome in one + block, but puts the playhead ~150px from the axis it refers to, so scrubbing + means watching two places at once. +- **An accent tint for the selected row**, rather than inverting it. It collides + with feature colours; see column 5. diff --git a/docs/Gram-Modes.md b/docs/Gram-Modes.md index 92a443e6..25f1a26b 100644 --- a/docs/Gram-Modes.md +++ b/docs/Gram-Modes.md @@ -51,10 +51,13 @@ mode, so a first stray click never leaves an annotation behind. and is disabled when it is already showing. On an audio-sourced gram "the whole gram" is the configured `window-seconds` window, not the entire recording. -- Pan's own button shows a hand rather than a word. Its row is the only one - carrying command buttons, and four controls do not fit across the column with - a word among them; the hand is also what the cursor becomes over a gram you - can drag. Every other mode is still named in full. +- Pan's own button shows a hand beside its word, as every mode's button shows a + glyph beside its word: in a stacked rail the shape is what the eye lands on + and the word is what settles it. The hand is also what the cursor becomes over + a gram you can drag. +- The `+` / `−` / fit controls are drawn in the mode rail's own footer, beneath + every mode rather than inside Pan's row, and stay there whichever mode is + armed: they act on the view, not on the armed tool. Pan still declares them. - On an audio-sourced gram the drag keeps working when the pointer leaves the image. Scrolling back to the start of a recording *means* putting blank space on screen — the top edge is the playhead, so the first second only reaches it @@ -85,11 +88,12 @@ events, broadband pulses or ambient shifts in sonar data. - Right-click a marker to delete it. - Markers are listed in the markers table above the gram; clicking a row selects that marker, after which the arrow keys nudge it (Shift for larger steps). -- A marker's colour and symbol come from the style controls; with a marker - selected, those controls restyle it in place. -- 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 marker's colour and symbol come from the style panel; with a marker selected + and the panel's **Selected** tab armed, those controls restyle it in place. +- The style panel's **Label** field edits the selected marker's label where the + rest of its style is set, rather than in a dialog opened from the row. 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 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 @@ -97,7 +101,7 @@ events, broadband pulses or ambient shifts in sonar data. is centred *below* the symbol so the data being marked stays visible. - The table's **Label** column shows labels of five characters or fewer in full, and abbreviates anything longer to its first three characters plus `..`. The - full text stays on the gram and in the dialog. + full text stays on the gram and in the style panel's field. --- diff --git a/docs/Testing-Strategy.md b/docs/Testing-Strategy.md index e8e76c59..2cf6a5c8 100644 --- a/docs/Testing-Strategy.md +++ b/docs/Testing-Strategy.md @@ -69,7 +69,7 @@ tests drive real mouse and keyboard events against a debug page or a fixture in speed, pan/zoom (including wheel navigation and clamping), and cross-mode integration - **Persistence**: trainer/student context detection, save and restore, the - student 24-hour expiry, "Clear gram", and the warning shown when storage + student 24-hour expiry, "Clear all annotations", and the warning shown when storage refuses a write - **Keyboard and focus**: instance focus, Tab navigation, arrow-key handling - **Lifecycle and API hygiene**: the debug-only `__test__*` surface, listener diff --git a/hygiene-baseline.json b/hygiene-baseline.json index 5736dc6a..eac8edc5 100644 --- a/hygiene-baseline.json +++ b/hygiene-baseline.json @@ -11,30 +11,34 @@ "_comment_moduleLines_state_levelscope": "src/core/state.js: 441 -> 442 and src/types.js: 779 -> 780, for the `level-scope` painting control (`levelScope` on `player.analysis`): whether the display percentiles are measured over the whole file or over each painted row, the per-line gain a legacy display applies and the reason its quiet passages stay sharply bounded. One field line in state.js, one @property line in types.js, for the same reason every painting control raised both: the initial state is the one place a core slice's shape is stated. Caps otherwise only fall; these are raised deliberately, as the earlier painting controls raised them.", "_comment_moduleLines_state_levelspan": "src/core/state.js: 442 -> 444 and src/types.js: 780 -> 782, for `level-span` and `normalisation-window` (`levelSpan` and `normalisationWindow` on `player.analysis`): the fixed dB span above the floor that is the legacy LOFAR painting, and the split window's reach stated in hertz rather than a bin count that means a different width at every FFT size. One field line each in state.js, one @property line each in types.js, as every painting control before them. Caps otherwise only fall; these are raised deliberately.", "moduleLineDefault": 350, + "_comment_moduleLines_state_redesign": "src/core/state.js: 432 -> 447. The control-row redesign adds three core keys: `guidanceCollapsed` (whether the guidance column is a rail), `bookmarks` (the transport's flagged times) and `styleTarget` (which of the style panel's twin tabs the style controls act on). The initial state is the one place a core slice's shape is stated, so there is nowhere else for them to live -- the same reason spec 171 raised this cap, and this is the same kind of deliberate raise.", + "_comment_moduleLines_types_redesign": "src/types.js: 774 -> 802. Declarations for the same redesign: the `TimeBookmark` and `StyleTarget` typedefs, the new `GramFrameState` and `instance.ui` members behind the guidance column and the instrument readouts, and the two selection helpers on the interaction seam. types.js is capped rather than exempt so it ratchets like the rest; it grows when the component's surface does.", + "_comment_moduleLines_merge_326": "src/core/state.js: 447 -> 459 and src/types.js: 802 -> 810, on merging main into the control-row redesign (#326). Neither side grew past the other's cap on its own: main took state.js 432 -> 444 and types.js 774 -> 782 for the painting controls (`_comment_moduleLines_state_painting` through `_state_levelspan`), and the redesign took them 432 -> 447 and 774 -> 802 (`_comment_moduleLines_state_redesign`, `_types_redesign`), each with its reason recorded above. The merged file carries both sets of fields, and the caps are the sum of the two deliberate raises -- no line was added by the merge itself.", + "_comment_moduleLines_fell_redesign": "Caps that FELL with this change, and why, so the ratchet records the split rather than just the growth: keyboardControl.js 698 -> 524 (the transport keys moved to player/transportKeys.js and the style routing to core/featureStyle.js); storage.js 1025 -> 988 (the two chrome preferences moved to core/preferences.js); main.js 966 -> 951 (the clear-all button builds in components/AnnotationTables.js, beside the footer it sits in); AnalysisMode.js 725 -> 636 and HarmonicsMode.js 369 -> 368 (the per-row label dialog is gone -- labels are edited in the style panel). events.js 445 -> 422: the image-bounds gate on pointer delivery moved to core/pointerScope.js, which is where the one mode that opts out of it is explained. keyboardControl.js 524 -> 415: selection itself moved to core/selection.js, since selection is what the arrow keys act on rather than part of how they work, and every table that reports a clicked row had to import a keyboard module to say so.", "moduleLineCaps": { "src/api/GramFrameAPI.js": 415, "src/core/events.js": 423, - "src/core/keyboardControl.js": 698, - "src/core/state.js": 444, - "src/core/storage.js": 1025, - "src/main.js": 966, - "src/modes/analysis/AnalysisMode.js": 725, + "src/core/keyboardControl.js": 415, + "src/core/state.js": 459, + "src/core/storage.js": 988, + "src/main.js": 951, + "src/modes/analysis/AnalysisMode.js": 636, "src/modes/doppler/DopplerMode.js": 729, - "src/modes/harmonics/HarmonicsMode.js": 369, + "src/modes/harmonics/HarmonicsMode.js": 368, "src/modes/shared/BaseDragHandler.js": 357, "src/modes/shared/PinSetMode.js": 1046, "src/rendering/axes.js": 403, - "src/types.js": 782 + "src/types.js": 810 }, - "_comment_uncoveredUnitLines": "R9-11 (issue #266). Lines in the unit lane's scope - the module list in vitest.config.js - that no unit test executes. A count, so it falls like every other ratchet here, rather than a percentage that would need an inverted comparison and could rise while the untested code grew. Regenerate with `yarn coverage`; `yarn hygiene` skips the check when no summary is present.", - "uncoveredUnitLines": 411, + "_comment_uncoveredUnitLines": "R9-11 (issue #266). Lines in the unit lane's scope - the module list in vitest.config.js - that no unit test executes. A count, so it falls like every other ratchet here, rather than a percentage that would need an inverted comparison and could rise while the untested code grew. Regenerate with `yarn coverage`; `yarn hygiene` skips the check when no summary is present. Fell 411 -> 409 with the control-row redesign: the guidance column's content rules moved out of the DOM-building utils/secureHTML.js (which the unit lane cannot load at all) into the pure utils/guidanceContent.js, where tests/unit/guidance-content.test.js covers them.", + "uncoveredUnitLines": 409, "circularDependencies": 0, "unusedExportModules": 4, - "_comment_specTypeErrors": "R9-10. `yarn typecheck` (tsconfig.json) covers src/, tests/helpers, tests/unit and scripts/ and must stay at zero. The Playwright spec files carry the rest -- mostly unguarded nulls from `boundingBox()` and `querySelector` inside `page.evaluate` -- and are counted here through tsconfig.specs.json so the debt is visible and falling rather than invisible. At zero, fold tests/**/*.js into tsconfig.json and delete tsconfig.specs.json.", - "specTypeErrors": 509, + "_comment_specTypeErrors": "R9-10. `yarn typecheck` (tsconfig.json) covers src/, tests/helpers, tests/unit and scripts/ and must stay at zero. The Playwright spec files carry the rest -- mostly unguarded nulls from `boundingBox()` and `querySelector` inside `page.evaluate` -- and are counted here through tsconfig.specs.json so the debt is visible and falling rather than invisible. At zero, fold tests/**/*.js into tsconfig.json and delete tsconfig.specs.json. Fell 509 -> 503 with the control-row redesign: the marker-label dialog and the six unguarded locators its specs used are gone.", + "specTypeErrors": 503, "waitForTimeoutOccurrences": 1, "_comment_instanceSurface": "specs/167-structural-refactor Story 5. instanceStateReachIns counts lines containing `instance.state` under src/; instanceFields counts class-field declarations between `export class GramFrame` and its constructor in src/main.js.", - "instanceStateReachIns": 143, + "instanceStateReachIns": 141, "_comment_instanceFields_168": "Raised 11 -> 12 by spec 168: `player` is a fifth grouped sub-object (the audio element, its controller and follow loop share one lifetime), not a flat field returning to the surface.", "instanceFields": 12 } diff --git a/src/components/AnnotationTables.js b/src/components/AnnotationTables.js new file mode 100644 index 00000000..38c79cfd --- /dev/null +++ b/src/components/AnnotationTables.js @@ -0,0 +1,154 @@ +/** + * The three annotation tables — markers, harmonic sets, sideband sets — and + * the headers above them. + * + * All three are visible at all times, whatever mode is armed: a set is managed + * from its own table wherever the analyst happens to be (issue #241). What + * lives here is the furniture they share — the header, the count chip, the + * empty-state line, and the slot each mode fills with its own table. What the + * columns contain is each mode's business, in `AnalysisMode`, `HarmonicPanel` + * and `SidebandPanel`. + * + * The sideband column carries one extra thing: the footer holding "Clear all + * annotations". Sideband sets are rare, so the foot of that column is the space + * the panel has going spare — and clearing is not a view control, so it does + * not belong in the mode rail beside zoom and fit. + */ + +/// + +/** + * The containers the layout mounts and the modes fill. + * @typedef {Object} AnnotationTableElements + * @property {HTMLDivElement} tables - The three columns' shared container + * @property {HTMLDivElement} markersContainer - Markers column + * @property {HTMLDivElement} harmonicsContainer - Harmonics column + * @property {HTMLDivElement} sidebandsContainer - Sidebands column + */ + +/** + * Build one table column: a header, and the space beneath it a mode fills. + * @param {string} className - The column's own class + * @param {string} title - Heading text + * @returns {HTMLDivElement} The column + */ +function createTableColumn(className, title) { + const column = document.createElement('div') + column.className = `gram-frame-table-column ${className}` + + const header = document.createElement('div') + header.className = 'gram-frame-panel-header' + + const heading = document.createElement('h4') + heading.textContent = title + header.appendChild(heading) + + // How many of this kind there are, hidden while there are none: a chip + // reading "0" is noise beside an empty state that already says so. + const count = document.createElement('span') + count.className = 'gram-frame-count-chip' + count.hidden = true + header.appendChild(count) + + column.appendChild(header) + return column +} + +/** + * Build the three annotation columns. + * @returns {AnnotationTableElements} The columns, ready to mount and fill + */ +export function createAnnotationTables() { + const tables = document.createElement('div') + tables.className = 'gram-frame-tables' + + const markersContainer = createTableColumn('gram-frame-markers-persistent-container', 'Markers') + + const harmonicsContainer = createTableColumn('gram-frame-harmonics-persistent-container', 'Harmonics') + // The slot HarmonicsMode mounts its "+ Manual" button into. It is a command + // for the harmonics table specifically, so it rides that table's header + // rather than the mode rail, where it would be one mode's command among four + // view controls. + const harmonicsButtons = document.createElement('div') + harmonicsButtons.className = 'gram-frame-harmonics-button-container' + const harmonicsHeader = harmonicsContainer.querySelector('.gram-frame-panel-header') + if (harmonicsHeader) { + harmonicsHeader.classList.add('gram-frame-harmonics-header') + harmonicsHeader.appendChild(harmonicsButtons) + } + + const sidebandsContainer = createTableColumn('gram-frame-sidebands-persistent-container', 'Sidebands') + + tables.appendChild(markersContainer) + tables.appendChild(harmonicsContainer) + tables.appendChild(sidebandsContainer) + + return { tables, markersContainer, harmonicsContainer, sidebandsContainer } +} + +/** + * Show how many of each kind of annotation there are. + * + * Called from the same place the tables themselves are refreshed, so the chip + * and the rows it counts can never disagree. + * @param {GramFrame} instance - GramFrame instance + * @returns {void} + */ +export function refreshTableCounts(instance) { + const { analysis, harmonics, sidebands } = instance.state + const { markersContainer, harmonicsContainer, sidebandsContainer } = instance.ui + setCount(markersContainer, analysis ? analysis.markers.length : 0) + setCount(harmonicsContainer, harmonics ? harmonics.harmonicSets.length : 0) + setCount(sidebandsContainer, sidebands ? sidebands.sidebandSets.length : 0) +} + +/** + * Write one column's count chip, hiding it at zero. + * @param {HTMLElement|null} container - The column + * @param {number} count - How many rows it holds + * @returns {void} + */ +function setCount(container, count) { + const chip = container ? container.querySelector('.gram-frame-count-chip') : null + if (!(chip instanceof HTMLElement)) { + return + } + chip.textContent = String(count) + chip.hidden = count === 0 +} + +/** + * Put the "Clear all annotations" button at the foot of the sidebands column. + * + * On every page, trainer and student alike: a student whose annotations expire + * overnight still wants to start today's exercise again today, and clearing is + * the same operation either way — only where the annotations were stored + * differs. + * + * It is here rather than in the mode rail beside zoom and fit because it + * removes annotations, not view state, and a destructive control among three + * view controls is one slip from an afternoon's work. Sideband sets are rare, + * so the bottom of that column is the space the panel has spare. + * @param {GramFrame} instance - GramFrame instance + * @param {function(): void} onClear - What the button does + * @returns {void} + */ +export function mountClearAllButton(instance, onClear) { + const button = document.createElement('button') + button.type = 'button' + button.className = 'gram-frame-clear-btn' + button.textContent = 'Clear all annotations' + button.title = 'Remove every cross, harmonic set and sideband set' + button.addEventListener('click', event => { + event.preventDefault() + onClear() + }) + + const footer = document.createElement('div') + footer.className = 'gram-frame-tables-footer' + footer.appendChild(button) + + if (instance.ui.sidebandsContainer) { + instance.ui.sidebandsContainer.appendChild(footer) + } +} diff --git a/src/components/ColorPicker.js b/src/components/ColorPicker.js index bdbcb5a0..9b74acf3 100644 --- a/src/components/ColorPicker.js +++ b/src/components/ColorPicker.js @@ -1,25 +1,23 @@ /** - * Style panel for GramFrame overlays. + * The colour slider: the one colour control in the panel. * - * One panel, three bands, grouped by what each control actually affects: + * A full-hue gradient with a bar thumb, and the arithmetic that turns a + * position into a colour and back. It is the widest-reaching control in the + * style panel — it colours analysis markers, harmonic sets, sideband sets AND + * doppler curves — and, since the redesign, the only one: the second row of + * fixed swatches is gone, because two ways to set one property meant neither + * was the one that said what the colour currently was. * - * - **Colour** — the gradient slider, uncaptioned. The widest-reaching control - * there is: it styles analysis markers, harmonic sets AND doppler curves. - * - **Symbol** — the symbol drop-down and the (temporary) large-symbol toggle. - * Both apply to markers and harmonic sets; neither applies to doppler. - * - **Harmonics** — the pin toggle, fenced off below a rule because it is the - * one control here that harmonic sets alone understand. + * The strip is a canvas rather than a CSS gradient so the position-to-colour + * mapping is read from the same pixels the analyst clicked, rather than from a + * second description of the ramp that could drift from the drawn one. * - * The panel used to be headed "Symbol" with all four controls stacked - * undifferentiated, which named the narrowest scope in the panel and left the - * rest to guesswork. + * The panel around it — the target tabs, the symbol row, the pin row — is + * `StylePanel.js`. */ /// -import { createSymbolSelect, createLargeSymbolToggle } from './SymbolPicker.js' -import { createPinToggle } from './PinToggle.js' -import { getActiveStyle } from '../core/keyboardControl.js' import { dispatch } from '../core/state.js' /** @@ -42,117 +40,43 @@ const COLOR_PALETTE = [ ] /** - * Create a band caption ("Symbol", "Harmonics") for the style panel. - * @param {string} text - Caption text - * @returns {HTMLDivElement} The caption element + * The slider's handle. + * @typedef {Object} ColorControl + * @property {function(string): void} setValue - Move the thumb to this colour, without changing state */ -function createGroupLabel(text) { - const label = document.createElement('div') - label.className = 'gram-frame-style-group-label' - label.textContent = text - return label -} /** - * Create the "Style" panel: a colour band, a symbol band and a harmonics band. + * Create the colour slider. * - * When a marker or harmonic set is selected, this panel restyles that feature - * in place; otherwise it sets the colour/symbol for the next created feature - * (feature 161). The panel also syncs its displayed colour/symbol to whichever - * feature is selected via `instance.interaction.syncStyleControls`. + * When a feature is selected and the panel is targeting it, clicking restyles + * that feature in place; otherwise the colour becomes the one the next created + * feature takes (feature 161). * @param {GramFrame} instance - GramFrame instance - * @returns {HTMLDivElement} The style panel element + * @param {function(string): void} onPick - Told the colour, after it has been routed + * @returns {{element: HTMLDivElement, control: ColorControl}} The slider and its handle */ -export function createColorPicker(instance) { +export function createColorSlider(instance, onPick) { const state = instance.state + const container = document.createElement('div') - container.className = 'gram-frame-color-picker' - container.style.display = 'block' - - // No panel heading, and no caption on the colour band: the gradient slider - // announces itself, and the two bands below it are captioned, so every line of - // text above the slider only cost the control row height the host's pages need - // elsewhere. - - // --- Colour band: applies to markers, harmonic sets and doppler curves --- - const colorGroup = document.createElement('div') - colorGroup.className = 'gram-frame-style-group' - container.appendChild(colorGroup) - - // Palette container - holds the full-width colour slider - const paletteContainer = document.createElement('div') - paletteContainer.className = 'gram-frame-color-palette' - colorGroup.appendChild(paletteContainer) - - // Slider container for canvas and indicator - const sliderContainer = document.createElement('div') - sliderContainer.className = 'gram-frame-color-slider' - sliderContainer.style.position = 'relative' - paletteContainer.appendChild(sliderContainer) - - // Create continuous color palette using canvas + container.className = 'gram-frame-color-slider' + const canvas = document.createElement('canvas') canvas.width = 140 canvas.height = 20 canvas.className = 'gram-frame-color-canvas' - sliderContainer.appendChild(canvas) + container.appendChild(canvas) - // Initialize default color if (!state.selectedColor) { - state.selectedColor = '#ff6b6b' // Default first color + state.selectedColor = '#ff6b6b' } - // Draw the color palette drawColorPalette(canvas) - // Color selection indicator const indicator = document.createElement('div') indicator.className = 'gram-frame-color-indicator' - sliderContainer.appendChild(indicator) - - // --- Symbol band: applies to markers and harmonic sets, not to doppler --- - const symbolGroup = document.createElement('div') - symbolGroup.className = 'gram-frame-style-group' - container.appendChild(symbolGroup) - - const symbolRow = document.createElement('div') - symbolRow.className = 'gram-frame-style-row' - symbolGroup.appendChild(symbolRow) - - symbolRow.appendChild(createGroupLabel('Symbol')) - - // The drop-down's glyphs are tinted with the currently selected colour, so it - // doubles as the colour readout. - const symbol = createSymbolSelect(instance) - const symbolSelect = symbol.element - symbolRow.appendChild(symbolSelect) - - // TEMPORARY (size experiment): the size toggle sits alongside the drop-down - // it modifies, for feedback on whether larger symbols read better on a real - // gram. - const largeSymbols = createLargeSymbolToggle(instance) - symbolRow.appendChild(largeSymbols.element) - - // --- Pin band: the pin toggle is the one control that applies to pin sets - // (harmonic and sideband) rather than to every feature --- - const divider = document.createElement('div') - divider.className = 'gram-frame-style-divider' - container.appendChild(divider) - - const harmonicsGroup = document.createElement('div') - harmonicsGroup.className = 'gram-frame-style-group' - container.appendChild(harmonicsGroup) - - // Caption and control share a row, as in the Symbol band above. - const harmonicsRow = document.createElement('div') - harmonicsRow.className = 'gram-frame-style-row' - harmonicsGroup.appendChild(harmonicsRow) - - harmonicsRow.appendChild(createGroupLabel('Pin sets')) - const pin = createPinToggle(instance) - harmonicsRow.appendChild(pin.element) - - // Add click handler for color selection + container.appendChild(indicator) + canvas.addEventListener('click', (event) => { const rect = canvas.getBoundingClientRect() const x = event.clientX - rect.left @@ -163,59 +87,29 @@ export function createColorPicker(instance) { const color = getColorFromPosition(canvasX, canvas.width) - // Route to the selected feature when one is selected (restyle in place), + // Route to the targeted feature when there is one (restyle in place), // otherwise set the colour for the next created feature (feature 161). - if (!instance.interaction.applyColorToSelectedFeature || !instance.interaction.applyColorToSelectedFeature(color)) { + const apply = instance.interaction.applyColorToSelectedFeature + if (!apply || !apply(color)) { state.selectedColor = color // Dispatch: this is a state change listeners care about, and only the // "Large" toggle used to say so (issue #268, BH-30). dispatch(instance) } - // Tint the symbol drop-down with the newly selected colour - symbolSelect.style.color = color - - // Update indicator position using the same canvasX coordinate for consistency updateIndicatorPosition(indicator, canvasX, canvas.width) + onPick(color) }) - /** - * Move the colour indicator/tint to reflect a given colour (without mutating - * state) — used when selection changes to show the selected feature's colour. - * @param {string} color - Hex colour to display - */ - const showColor = (color) => { - const position = getPositionFromColor(color, canvas.width) - updateIndicatorPosition(indicator, position, canvas.width) - symbolSelect.style.color = color - } - - // Sync both controls (colour indicator + symbol drop-down) to whatever is - // currently selected, or to the next-feature defaults when nothing is - // selected (feature 161, FR-004/FR-013). - // - // The three handles are closed over, not read back off `instance.interaction`. - // Each control used to install itself there during construction and this - // function used to look them up by name: an ordering contract `tsc` could not - // check, which reordering the panel or mounting a picker twice would break in - // silence. They are built here, so they can simply be held here (issue #267). - instance.interaction.syncStyleControls = () => { - const style = getActiveStyle(instance) - showColor(style.color) - symbol.control.setValue(style.symbol) - symbol.control.setTint(style.color) - pin.control.setValue(style.showPin) - // Markers have no pin, so the toggle is disabled while one is selected. - pin.control.setEnabled(style.pinApplies) - // TEMPORARY (size experiment): keep the size toggle in step with selection. - largeSymbols.control.setValue(style.largeSymbols) + /** @type {ColorControl} */ + const control = { + setValue(color) { + updateIndicatorPosition(indicator, getPositionFromColor(color, canvas.width), canvas.width) + } } + control.setValue(state.selectedColor) - // Initialize indicator position (use canvas coordinates directly) - const initialPosition = getPositionFromColor(state.selectedColor, canvas.width) - updateIndicatorPosition(indicator, initialPosition, canvas.width) - - return container + return { element: container, control } } /** @@ -225,22 +119,22 @@ export function createColorPicker(instance) { function drawColorPalette(canvas) { const ctx = canvas.getContext('2d') if (!ctx) { - // A refused 2d context means no palette to draw into. The swatch row above - // it is the primary control, so leaving the strip blank degrades rather - // than throwing during construction. + // A refused 2d context means no palette to draw into. Leaving the strip + // blank degrades rather than throwing during construction; the symbol + // button still shows what colour is in force. return } const width = canvas.width const height = canvas.height - + // Create gradient with HSV color space for better color distribution const gradient = ctx.createLinearGradient(0, 0, width, 0) - + // Create a rainbow gradient using the standard color palette COLOR_PALETTE.forEach((color, index) => { gradient.addColorStop(index / (COLOR_PALETTE.length - 1), color) }) - + ctx.fillStyle = gradient ctx.fillRect(0, 0, width, height) } @@ -258,19 +152,19 @@ function getColorFromPosition(x, width) { const lowerIndex = Math.floor(segmentIndex) const upperIndex = Math.min(lowerIndex + 1, COLOR_PALETTE.length - 1) const t = segmentIndex - lowerIndex - + if (lowerIndex === upperIndex) { return COLOR_PALETTE[lowerIndex] } - + // Interpolate between the two colors const color1 = hexToRgb(COLOR_PALETTE[lowerIndex]) const color2 = hexToRgb(COLOR_PALETTE[upperIndex]) - + const r = Math.round(color1.r * (1 - t) + color2.r * t) const g = Math.round(color1.g * (1 - t) + color2.g * t) const b = Math.round(color1.b * (1 - t) + color2.b * t) - + return `#${r.toString(16).padStart(2, '0')}${g.toString(16).padStart(2, '0')}${b.toString(16).padStart(2, '0')}` } @@ -296,7 +190,7 @@ function getPositionFromColor(hexColor, width) { const targetRgb = hexToRgb(hexColor) let closestIndex = 0 let minDistance = Infinity - + // Find the closest color in our palette COLOR_PALETTE.forEach((color, index) => { const colorRgb = hexToRgb(color) @@ -305,13 +199,13 @@ function getPositionFromColor(hexColor, width) { Math.pow(targetRgb.g - colorRgb.g, 2) + Math.pow(targetRgb.b - colorRgb.b, 2) ) - + if (distance < minDistance) { minDistance = distance closestIndex = index } }) - + // Convert index to position const segmentSize = 1 / (COLOR_PALETTE.length - 1) const position = closestIndex * segmentSize @@ -327,4 +221,4 @@ function getPositionFromColor(hexColor, width) { function updateIndicatorPosition(indicator, x, width) { const percentage = (x / width) * 100 indicator.style.left = `${Math.max(0, Math.min(100, percentage))}%` -} \ No newline at end of file +} diff --git a/src/components/CursorReadout.js b/src/components/CursorReadout.js new file mode 100644 index 00000000..4d19afb1 --- /dev/null +++ b/src/components/CursorReadout.js @@ -0,0 +1,120 @@ +/** + * The instrument face: the numbers the panel is anchored on. + * + * Three readouts in a recessed column, in two ranks. Frequency is the one + * large, lit reading: in use it is by far the most important number in the + * panel, and giving time the same size made the two compete for the eye. + * Time sits immediately beneath it at half the size in the label grey, and + * the doppler speed is pinned to the foot at the same rank. All three are + * monospaced and tabular so a digit never shifts sideways as it changes. + * Everything else in the control row is a control; this is the measurement, + * and it is styled to look like one. + * + * The column has two targets, named by its kicker. With nothing selected it + * follows the pointer over the gram ("CURSOR"). With a feature selected it + * shows that feature's own values instead ("SELECTED", plus what it is called), + * so a marker's numbers can be read without hovering it and losing them again + * on the way to the panel. + */ + +/// + +import { createLEDDisplay, setLEDValue } from './LEDDisplay.js' +import { formatTime } from '../utils/timeFormatter.js' +import { describeSelection } from '../core/selectionTarget.js' + +/** + * The readout column's elements, for the layout that mounts them. + * @typedef {Object} ReadoutElements + * @property {HTMLDivElement} column - The column itself + * @property {HTMLElement} timeLED - Time readout + * @property {HTMLElement} freqLED - Frequency readout + * @property {HTMLElement} speedLED - Doppler speed readout + * @property {HTMLDivElement} kicker - What the column is currently reading + */ + +/** + * Build the readout column. + * @returns {ReadoutElements} The column and its readouts + */ +export function createCursorReadout() { + const column = document.createElement('div') + column.className = 'gram-frame-readout-column' + + const kicker = document.createElement('div') + kicker.className = 'gram-frame-kicker gram-frame-readout-kicker' + kicker.textContent = 'Cursor' + column.appendChild(kicker) + + const freqLED = createLEDDisplay('Frequency (Hz)', '0.0', 'HZ') + freqLED.classList.add('gram-frame-led-accent') + column.appendChild(freqLED) + + // Time is the second-rank reading: directly under the frequency, at half its + // size and in the label grey, with its own caption since it no longer has + // the size to name itself. It is not fenced like doppler — it belongs with + // the frequency, as the other half of the pointer's coordinate. + const timeLED = createLEDDisplay('Time (mm:ss)', formatTime(0), 'MM:SS', 'Time') + timeLED.classList.add('gram-frame-led-secondary') + column.appendChild(timeLED) + + const spacer = document.createElement('div') + spacer.className = 'gram-frame-readout-spacer' + column.appendChild(spacer) + + // Doppler is a derived quantity rather than a coordinate, so it is fenced off + // at the foot between two rules, at the same second rank as time — present, + // but not competing with the reading the column exists for. + // + // Its accessible name stays the full "Doppler Speed (kts)" while the caption + // beside the number is the one word there is room for. The gap in that name + // is a non-breaking space, written as a \u00a0 escape so it stays visible in + // the source; Playwright normalises it to a plain space, so the + // `:text-is("Doppler Speed (kts)")` locators in tests/helpers still match. + const speedLED = createLEDDisplay('Doppler\u00a0Speed (kts)', '0.0', 'KTS', 'Doppler') + speedLED.classList.add('gram-frame-led-secondary', 'gram-frame-led-inline') + column.appendChild(speedLED) + + return { column, timeLED, freqLED, speedLED, kicker } +} + +/** + * Point the column at the pointer, or at the selected feature, and write + * whichever one's values. + * + * Called on every selection change rather than on every pointer move: while + * something is selected the pointer is not what the column is reading, and + * `updateUniversalCursorReadouts` stands aside for exactly that reason. + * @param {GramFrame} instance - GramFrame instance + * @returns {void} + */ +export function refreshReadoutTarget(instance) { + const { kicker, timeLED, freqLED } = instance.ui + if (!kicker) { + return + } + + const selected = describeSelection(instance) + kicker.replaceChildren() + + if (!selected) { + kicker.textContent = 'Cursor' + return + } + + const word = document.createElement('span') + word.textContent = 'Selected' + kicker.appendChild(word) + + const name = document.createElement('span') + name.className = 'gram-frame-readout-target' + name.textContent = selected.label + kicker.appendChild(name) + + if (timeLED) { + setLEDValue(timeLED, formatTime(selected.time)) + } + if (freqLED) { + setLEDValue(freqLED, selected.freq.toFixed(2)) + } +} diff --git a/src/components/DiffingTable.js b/src/components/DiffingTable.js index f81675d4..23bfc9ca 100644 --- a/src/components/DiffingTable.js +++ b/src/components/DiffingTable.js @@ -18,6 +18,8 @@ /// +import { revealTableRow } from './tableScroll.js' + /** * Create a diffing table inside `container`. * @@ -167,6 +169,40 @@ export function createDiffingTable(container, spec) { } } + /** + * The instructional line shown in place of rows when there are none. + * + * An empty table used to be an empty rectangle, which says only that nothing + * is there — not what to do about it. The line is a row of the table rather + * than a sibling of it so it scrolls, aligns and disappears with the rest, + * and it is only built when a consumer supplies the words. + * @type {HTMLTableRowElement|null} + */ + let emptyRow = null + + /** + * Show or hide the empty-state line. + * @param {boolean} empty - Whether there are no rows + */ + function setEmptyState(empty) { + if (!spec.emptyMessage) { + return + } + if (!emptyRow) { + emptyRow = document.createElement('tr') + emptyRow.className = 'gram-frame-table-empty' + const cell = document.createElement('td') + cell.colSpan = spec.columns.length + cell.textContent = spec.emptyMessage + emptyRow.appendChild(cell) + } + if (empty) { + tbody.appendChild(emptyRow) + } else if (emptyRow.parentNode) { + emptyRow.remove() + } + } + /** * Bring the rendered rows into line with `currentRows`. * @@ -175,6 +211,7 @@ export function createDiffingTable(container, spec) { * data. Idempotent: an update that changes nothing performs no DOM writes. */ function applyDiff() { + setEmptyState(false) const existing = tbody.querySelectorAll('tr') for (let index = 0; index < currentRows.length; index++) { @@ -196,45 +233,6 @@ export function createDiffingTable(container, spec) { } } - /** - * Scroll the row at `index` into view, if it is outside the visible band. - * - * Sets `scrollTop` on the scroll wrapper rather than calling - * `Element.scrollIntoView`, which would also scroll every scrollable ancestor - * — including the host page — to bring the table into view. A new marker - * should move the table's own scrollbar and nothing else. - * - * The scroll is minimal, and never happens at all when the row is already - * visible: the wrapper is then left exactly where the user put it. Upward - * scrolling is opt-in (`allowUpward`) because an *addition* only ever appears - * below the fold, whereas a *selection* can be anywhere in the list. - * @param {number} index - Index of the row to reveal - * @param {boolean} [allowUpward] - Also scroll up for a row above the fold - */ - function revealRow(index, allowUpward = false) { - const tr = /** @type {HTMLElement|undefined} */ (tbody.children[index]) - if (!tr) return - - // offsetTop is measured against the wrapper, which is the nearest - // positioned ancestor (it is absolutely positioned over the table area). - const bottom = tr.offsetTop + tr.offsetHeight - wrapper.clientHeight - if (bottom > wrapper.scrollTop) { - wrapper.scrollTop = bottom - return - } - - if (!allowUpward) return - - // The header is sticky at the top of the scrollport, so it floats over the - // first rows of the scrolled body: scrolling to the row's own offsetTop - // would park it underneath the header rather than beside it. - const headerCell = /** @type {HTMLElement|null} */ (headerRow.firstElementChild) - const top = tr.offsetTop - (headerCell ? headerCell.offsetHeight : 0) - if (top < wrapper.scrollTop) { - wrapper.scrollTop = Math.max(0, top) - } - } - /** * Per-row controls that act instead of selecting, in match order. Delete is * one of these — the original and, for the harmonics panel, the only one — @@ -298,6 +296,7 @@ export function createDiffingTable(container, spec) { const keys = currentRows.map((row, index) => spec.rowKey(row, index)) applyDiff() + setEmptyState(currentRows.length === 0) // Keep the last row that wasn't there before in view. Adding a marker // when the list has already overflowed otherwise appends it out of sight, @@ -309,7 +308,7 @@ export function createDiffingTable(container, spec) { } } if (renderedKeys.size > 0 && lastAdded !== -1) { - revealRow(lastAdded) + revealTableRow(wrapper, headerRow, tbody, lastAdded) } // Keep a newly selected row in view too. Selecting a feature on the gram @@ -320,7 +319,7 @@ export function createDiffingTable(container, spec) { const selectedIndex = isSelected ? keys.findIndex(key => isSelected(key)) : -1 const selectedKey = selectedIndex === -1 ? null : keys[selectedIndex] if (renderedKeys.size > 0 && selectedKey !== null && selectedKey !== renderedSelectedKey) { - revealRow(selectedIndex, true) + revealTableRow(wrapper, headerRow, tbody, selectedIndex, true) } renderedKeys = new Set(keys) diff --git a/src/components/GuidancePanel.js b/src/components/GuidancePanel.js new file mode 100644 index 00000000..d6f1cae8 --- /dev/null +++ b/src/components/GuidancePanel.js @@ -0,0 +1,175 @@ +/** + * The per-mode guidance column, and the rail it collapses to. + * + * The column sits immediately beside the mode rail, so the armed tool and the + * gestures that drive it read as one block. Its space is dedicated: collapsing + * it hands the width to the annotation tables and nothing else ever moves in. + * + * The column owns two things the mode does not: the header naming the armed + * mode, and the collapse. What goes in the body is still the mode's — every + * mode's `getGuidanceText()` — rendered through `utils/secureHTML.js`, which is + * the only path guidance takes to the DOM. + */ + +/// + +import { getModeDisplayName } from '../modes/modeRoster.js' +import { updateGuidancePanel } from '../utils/secureHTML.js' +import { withNavigationGuidance } from '../utils/guidanceContent.js' +import { loadGuidancePreference, saveGuidancePreference } from '../core/preferences.js' +import { dispatch } from '../core/state.js' + +/** + * Whether this instance's analyst has decided about the column, as opposed to + * leaving it to the panel's own width. + * + * A WeakMap rather than a field on the instance or a key in state: it is not + * part of what the component broadcasts, and it is read by exactly this module. + * @type {WeakMap} + */ +const hasChosen = new WeakMap() + +/** + * The parts of the guidance column its owner has to hold on to. + * @typedef {Object} GuidanceColumnElements + * @property {HTMLDivElement} column - The column itself, mounted in the control row + * @property {HTMLDivElement} body - Where the mode's guidance is rendered + * @property {HTMLDivElement} title - The header's mode name + */ + +/** + * Build the guidance column: a header naming the armed mode with a Hide + * button, a body for that mode's lines, and — shown in its place when + * collapsed — a 40px rail carrying a reveal button and the word "Guidance". + * + * Both faces are built once and swapped by a class on the column, rather than + * torn down and rebuilt: the body holds the current mode's rendered guidance, + * and collapsing should not throw that away only to re-render it on reveal. + * @param {GramFrame} instance - GramFrame instance + * @returns {GuidanceColumnElements} The column and the handles into it + */ +export function createGuidanceColumn(instance) { + // The stored choice, if there is one. `null` means the analyst has never + // touched the column and the panel decides by its own width. + const stored = loadGuidancePreference() + instance.state.guidanceCollapsed = stored === true + hasChosen.set(instance, stored !== null) + + const column = document.createElement('div') + column.className = 'gram-frame-guidance-column' + + const header = document.createElement('div') + header.className = 'gram-frame-guidance-header' + + const title = document.createElement('div') + title.className = 'gram-frame-guidance-title' + + const hide = document.createElement('button') + hide.type = 'button' + hide.className = 'gram-frame-guidance-hide' + hide.textContent = 'Hide' + hide.title = 'Hide guidance' + hide.addEventListener('click', event => { + event.preventDefault() + setGuidanceCollapsed(instance, true) + }) + + header.appendChild(title) + header.appendChild(hide) + + // `gram-frame-guidance` stays the body's class: it is what the guidance + // renderer and every existing caller address, and what the body *is* has not + // changed — only what surrounds it. + const body = document.createElement('div') + body.className = 'gram-frame-guidance' + + const rail = document.createElement('div') + rail.className = 'gram-frame-guidance-rail' + + const reveal = document.createElement('button') + reveal.type = 'button' + reveal.className = 'gram-frame-guidance-reveal' + reveal.textContent = '›' + reveal.title = 'Show guidance' + reveal.setAttribute('aria-label', 'Show guidance') + reveal.addEventListener('click', event => { + event.preventDefault() + setGuidanceCollapsed(instance, false) + }) + + const railLabel = document.createElement('div') + railLabel.className = 'gram-frame-guidance-rail-label' + railLabel.textContent = 'Guidance' + + rail.appendChild(reveal) + rail.appendChild(railLabel) + + column.appendChild(header) + column.appendChild(body) + column.appendChild(rail) + + return { column, body, title } +} + +/** + * Collapse the guidance column to its rail, or open it again, and remember the + * choice for next time. + * + * Dispatched like any other state change. The column is chrome rather than + * annotation, but "what does this instance look like right now" is exactly what + * a listener is asking, and a host page that mirrors the panel's state has no + * other way to hear about it. + * @param {GramFrame} instance - GramFrame instance + * @param {boolean} collapsed - Whether the column shows its rail + * @returns {void} + */ +function setGuidanceCollapsed(instance, collapsed) { + instance.state.guidanceCollapsed = collapsed + hasChosen.set(instance, true) + saveGuidancePreference(collapsed) + applyGuidanceCollapsed(instance) + dispatch(instance) +} + +/** + * Put the column into the state the flag describes. + * + * Three states, stamped as two classes. Once the analyst has decided, + * collapsing adds `-collapsed` and opening adds `-open`, and either beats the + * width rule in the stylesheet that otherwise collapses a narrow panel's + * guidance for them. With neither class the column is automatic — which is + * where it starts, and where it stays until someone presses Hide or the reveal. + * @param {GramFrame} instance - GramFrame instance + * @returns {void} + */ +export function applyGuidanceCollapsed(instance) { + const { guidanceColumn } = instance.ui + if (!guidanceColumn) { + return + } + const collapsed = !!instance.state.guidanceCollapsed + const chosen = hasChosen.get(instance) === true + guidanceColumn.classList.toggle('gram-frame-guidance-collapsed', collapsed) + guidanceColumn.classList.toggle('gram-frame-guidance-open', chosen && !collapsed) +} + +/** + * Show a mode's guidance: its name in the header, its lines in the body. + * + * The one call site for both halves, so the header can never name one mode + * while the body describes another — which is exactly what a second, + * independent "also set the title" call would eventually allow. + * @param {GramFrame} instance - GramFrame instance + * @param {import('../modes/BaseMode.js').BaseMode} mode - The armed mode + * @returns {void} + */ +export function showGuidanceForMode(instance, mode) { + const { guidanceTitle, guidancePanel } = instance.ui + if (guidanceTitle) { + guidanceTitle.textContent = getModeDisplayName(instance.state.mode) + } + if (guidancePanel) { + // Every mode's own lines, then the gestures that work in all of them. + updateGuidancePanel(guidancePanel, withNavigationGuidance(mode.getGuidanceText())) + } +} diff --git a/src/components/HarmonicPanel.js b/src/components/HarmonicPanel.js index ac7012e7..f6f5f717 100644 --- a/src/components/HarmonicPanel.js +++ b/src/components/HarmonicPanel.js @@ -45,6 +45,25 @@ function createColorCellContent(harmonicSet) { return colorDiv } +/** + * Build the spacing cell: the number, then its unit set smaller and quieter. + * + * The unit is part of the cell rather than the heading because the heading is + * the label an analyst scans for across three side-by-side tables, and + * "Spacing (Hz)" made that scan longer for a unit that never changes. + * @param {HarmonicSet} harmonicSet - The harmonic set data + * @returns {HTMLSpanElement} The cell content + */ +function createSpacingCellContent(harmonicSet) { + const wrapper = document.createElement('span') + wrapper.textContent = `${harmonicSet.spacing.toFixed(2)} ` + const unit = document.createElement('span') + unit.className = 'gram-frame-cell-unit' + unit.textContent = 'Hz' + wrapper.appendChild(unit) + return wrapper +} + /** * The ratio of the cursor's frequency to a set's spacing, as displayed. * @param {HarmonicSet} harmonicSet - The harmonic set data @@ -52,8 +71,9 @@ function createColorCellContent(harmonicSet) { * @returns {string} Formatted ratio */ function formatRatio(harmonicSet, instance) { - if (instance.state.cursorPosition && instance.state.cursorPosition.freq > 0) { - return (instance.state.cursorPosition.freq / harmonicSet.spacing).toFixed(3) + const cursor = instance.state.cursorPosition + if (cursor && cursor.freq > 0) { + return (cursor.freq / harmonicSet.spacing).toFixed(3) } return '5.000' // Representative ratio for the 5th harmonic } @@ -88,35 +108,29 @@ function createHarmonicDeleteButton(harmonicSet) { export function createHarmonicPanel(container, instance) { const table = createDiffingTable(container, { columns: [ - { label: '', width: '15%' }, - { label: 'Spacing (Hz)', width: '35%', cellClassName: 'gram-frame-harmonic-spacing' }, - { label: 'Ratio', width: '35%', cellClassName: 'gram-frame-harmonic-ratio' }, - { label: '', width: '15%' } + { label: '', width: '14%' }, + // "Hz" rides each value rather than the heading: the unit belongs to the + // number, and the heading is what an analyst scans down the row of three + // tables to find the right one. + { label: 'Spacing', width: '40%', cellClassName: 'gram-frame-harmonic-spacing' }, + { label: 'Ratio', width: '32%', cellClassName: 'gram-frame-harmonic-ratio gram-frame-cell-numeric' }, + { label: '', width: '14%', cellClassName: 'gram-frame-cell-action' } ], + emptyMessage: 'Drag on the gram to add a harmonic set', rowAttribute: 'data-harmonic-id', rowClassName: 'gram-frame-harmonic-row', rowKey: (harmonicSet) => harmonicSet.id, cells: (harmonicSet) => [ createColorCellContent(harmonicSet), - harmonicSet.spacing.toFixed(2), + createSpacingCellContent(harmonicSet), formatRatio(harmonicSet, instance), createHarmonicDeleteButton(harmonicSet) ], deleteSelector: '.gram-frame-harmonic-delete', - onSelect: (harmonicSetId, _harmonicSet, index) => { - // Toggle selection - if (instance.state.selection.selectedType === 'harmonicSet' && - instance.state.selection.selectedId === harmonicSetId) { - instance.interaction.clearSelection() - } else { - instance.interaction.setSelection('harmonicSet', harmonicSetId, index) - } - }, + onSelect: (harmonicSetId, _harmonicSet, index) => + instance.interaction.toggleSelection('harmonicSet', harmonicSetId, index), onDelete: (harmonicSetId) => instance.interaction.removeHarmonicSet(harmonicSetId), - isSelected: (harmonicSetId) => ( - instance.state.selection.selectedType === 'harmonicSet' && - instance.state.selection.selectedId === harmonicSetId - ) + isSelected: (harmonicSetId) => instance.interaction.isFeatureSelected('harmonicSet', harmonicSetId) }) const panel = /** @type {HTMLElement} */ (table.element.parentElement) diff --git a/src/components/LEDDisplay.js b/src/components/LEDDisplay.js index 040fbe3d..722b72cd 100644 --- a/src/components/LEDDisplay.js +++ b/src/components/LEDDisplay.js @@ -10,27 +10,49 @@ import { getModeDisplayName } from '../modes/modeRoster.js' /** - * Creates a single LED display element - * @param {string} label - Display label + * Creates a single LED display element. + * + * Three pieces of text, because the readout has three audiences. The `label` + * is the full name ("Time (mm:ss)") and is the LED's accessible name — visually + * hidden, since the column's own kicker already says what is being read and + * repeating it costs the control row height it does not have. The `unit` is + * what an instrument prints beside a number ("MM:SS"), set small and to the + * right. The optional `caption` is a short word shown to the left of the value, + * for a readout that needs naming in place rather than by the column. + * @param {string} label - Full display name; the accessible name * @param {string} value - Initial display value + * @param {string} [unit] - Unit shown beside the value + * @param {string} [caption] - Short visible name shown before the value * @returns {HTMLDivElement} The LED display element */ -export function createLEDDisplay(label, value) { +export function createLEDDisplay(label, value, unit, caption) { const led = document.createElement('div') led.className = 'gram-frame-led' - - // Create label element safely + const labelDiv = document.createElement('div') - labelDiv.className = 'gram-frame-led-label' + labelDiv.className = 'gram-frame-led-label gram-frame-visually-hidden' labelDiv.textContent = label - - // Create value element safely + led.appendChild(labelDiv) + + if (caption) { + const captionDiv = document.createElement('div') + captionDiv.className = 'gram-frame-led-caption' + captionDiv.textContent = caption + led.appendChild(captionDiv) + } + const valueDiv = document.createElement('div') valueDiv.className = 'gram-frame-led-value' valueDiv.textContent = value - - led.appendChild(labelDiv) led.appendChild(valueDiv) + + if (unit) { + const unitDiv = document.createElement('div') + unitDiv.className = 'gram-frame-led-unit' + unitDiv.textContent = unit + led.appendChild(unitDiv) + } + return led } diff --git a/src/components/MainUI.js b/src/components/MainUI.js index 71400d38..8740f588 100644 --- a/src/components/MainUI.js +++ b/src/components/MainUI.js @@ -1,254 +1,107 @@ /** - * MainUI module for GramFrame - * - * This module handles the creation and management of the main UI layout - * including the unified 4-column layout, LED displays, and container setup. + * The control row: five columns above the gram. + * + * Left to right — the mode rail, the guidance for the armed mode, the cursor + * readouts, the style panel, and the three annotation tables. Each is separated + * from the next by a single hairline rather than being boxed: five bordered + * cards of equal weight gave the eye no entry point, which is the problem this + * layout exists to solve. + * + * The first three columns are fixed-width and the tables take the rest, so a + * narrow host squeezes the tables (and, past a point, collapses the guidance to + * its rail) rather than reflowing the instrument face or the tool list. + * + * This module assembles; it builds almost nothing itself. The rail is + * `ModeButtons.js`, the guidance column `GuidancePanel.js`, the readouts + * `CursorReadout.js`, the style panel `StylePanel.js` and the tables + * `AnnotationTables.js`. */ /// -import { isPanelOwner } from '../modes/capabilities.js' -import { - createLEDDisplay, - createColorPicker, - createFullFlexLayout, - createFlexColumn -} from './UIComponents.js' +import { createStylePanel } from './StylePanel.js' +import { createCursorReadout } from './CursorReadout.js' +import { createGuidanceColumn } from './GuidancePanel.js' +import { createAnnotationTables } from './AnnotationTables.js' import { formatTime } from '../utils/timeFormatter.js' /** - * Create the unified layout for readouts: the left readout column plus the - * three persistent feature tables (markers, harmonics, sidebands). + * Create the control row and hand back the pieces the instance holds on to. * * Returns the handles rather than writing them onto the instance, so the * constructor is the one place they are adopted (spec 167, FR-009). * @param {GramFrame} instance - GramFrame instance - * @returns {UnifiedLayoutElements} The columns, LEDs and containers just built + * @returns {UnifiedLayoutElements} The columns, readouts and containers just built */ export function createUnifiedLayout(instance) { - // Create main container for unified layout - const unifiedLayoutContainer = /** @type {HTMLDivElement} */ (createFullFlexLayout('gram-frame-unified-layout', '2px')) - unifiedLayoutContainer.style.flexDirection = 'row' - unifiedLayoutContainer.style.flexWrap = 'nowrap' - - // Left Panel - Multi-column horizontal layout. Basis widened by 150px - // (600 → 750) so the guidance column (flex:1 between the fixed mode + controls - // columns) gets ~150px more room for the Pan-mode mouse-wheel guidance. It does - // NOT grow beyond that, so the markers/harmonics tables stay grouped right - // alongside rather than being pushed to the far edge; but it MAY shrink - // (shrink:1, min-width:0) so a narrow host stays clip-free instead of cutting - // off the tables. - const leftColumn = /** @type {HTMLDivElement} */ (createFullFlexLayout('gram-frame-left-column', '4px')) - // Grows into whatever the tables leave, but never past its 750px basis, so - // the spare width of a wide host reaches the guidance column instead of - // pooling at the right-hand edge — while the tables still stay grouped - // alongside rather than being pushed away (issue #241). Growing matters now - // that the guidance panel is absolutely positioned: it contributes no - // intrinsic width of its own, so without this the left column sat at the sum - // of its minimums whatever room was going spare. - leftColumn.style.flex = '1 1 750px' - leftColumn.style.maxWidth = '750px' - leftColumn.style.width = 'auto' - leftColumn.style.minWidth = '0' - leftColumn.style.flexDirection = 'row' - - // Column 1: Mode buttons - const modeColumn = /** @type {HTMLDivElement} */ (createFlexColumn('gram-frame-mode-column', '8px')) - modeColumn.style.flex = '0 0 130px' - modeColumn.style.width = '130px' - - // Column 2: Guidance panel. Its minimum width lives in CSS beside the rest of - // the column's styling — an inline one here silently outranked it. - const guidanceColumn = /** @type {HTMLDivElement} */ (createFlexColumn('gram-frame-guidance-column', '8px')) - guidanceColumn.style.flex = '1' - - // Column 3: Controls (time/freq displays, speed, color selector) - const controlsColumn = /** @type {HTMLDivElement} */ (createFlexColumn('gram-frame-controls-column', '1px')) - controlsColumn.style.flex = '0 0 220px' - controlsColumn.style.width = '220px' - - // Create universal cursor readouts in controls column - const cursorContainer = document.createElement('div') - cursorContainer.className = 'gram-frame-cursor-leds' - const timeLED = createLEDDisplay('Time (mm:ss)', formatTime(0)) - cursorContainer.appendChild(timeLED) - - const freqLED = createLEDDisplay('Frequency (Hz)', '0.0') - cursorContainer.appendChild(freqLED) - - // Create doppler speed LED (spans full width). Unlike the time/frequency - // readouts it lays its label out beside the value rather than above it: the - // label wraps into the space the stacked form wasted to its right, so the - // readout costs the control row less height. - // - // The gap between "Doppler" and "Speed" is a non-breaking space, written as a - // \u00a0 escape so it stays visible in the source. The label is sized - // `width: min-content`, which breaks at every ordinary space and would stack - // three lines; gluing the first two words holds it to two - "Doppler Speed" - // over "(kts)". Playwright normalises \u00a0 to a plain space, so the - // `:text-is("Doppler Speed (kts)")` locators in tests/helpers still match. - const speedLED = createLEDDisplay('Doppler\u00a0Speed (kts)', '0.0') - speedLED.classList.add('gram-frame-led-inline') - speedLED.style.gridColumn = '1 / -1' // Span both columns - cursorContainer.appendChild(speedLED) - - controlsColumn.appendChild(cursorContainer) - - // Style panel (colour, symbol, size, pin) in controls column - const colorPicker = createColorPicker(instance) - controlsColumn.appendChild(colorPicker) + const unifiedLayoutContainer = document.createElement('div') + unifiedLayoutContainer.className = 'gram-frame-unified-layout' + + // Column 1 — the mode rail. `ModeButtons.js` fills it, twice: once before the + // modes exist and again once they can declare their command buttons. + const modeColumn = document.createElement('div') + modeColumn.className = 'gram-frame-mode-column' - // Add columns to left panel - leftColumn.appendChild(modeColumn) - leftColumn.appendChild(guidanceColumn) - leftColumn.appendChild(controlsColumn) - - // Middle Column (185-235px) - Analysis Markers table. Elastic since the Label - // column was added (feature 231): up to 235px where the host has the width, - // down to 185px where it does not, so a narrow page keeps the control row the - // height it always had. See the note in gramframe.css. - const middleColumn = /** @type {HTMLDivElement} */ (createFlexColumn('gram-frame-middle-column')) - middleColumn.style.flex = '0 3 235px' - middleColumn.style.width = 'auto' - - // Create markers container in middle column - const markersContainer = createMarkersContainer() - middleColumn.appendChild(markersContainer) - - // Right column - Harmonics sets table, and beside it the Sidebands table. - // Both are always visible: a pin set of either kind is managed from its own - // table whatever mode is active, exactly as the markers table is (issue #241). - // - // Both are 175px, the width the harmonics table has always had, so the pair - // reads as a pair. The row is that much wider for it: where a host cannot give - // it the room, the guidance column takes the squeeze and its panel scrolls - // rather than the row growing taller and pushing the gram down the page — see - // the note on `.gram-frame-guidance` in gramframe.css. Widths live in CSS with - // the rest of each column's styling. - const rightColumn = /** @type {HTMLDivElement} */ (createFlexColumn('gram-frame-right-column')) - const harmonicsContainer = createHarmonicsContainer() - rightColumn.appendChild(harmonicsContainer) + // Column 2 — the armed mode's gestures, immediately beside the tool they + // belong to, so the two read as one block. + const guidance = createGuidanceColumn(instance) - const sidebandsColumn = /** @type {HTMLDivElement} */ (createFlexColumn('gram-frame-sidebands-column')) - const sidebandsContainer = createSidebandsContainer() - sidebandsColumn.appendChild(sidebandsContainer) + // Column 3 — the instrument face. + const readout = createCursorReadout() - // Assemble the unified layout - unifiedLayoutContainer.appendChild(leftColumn) - unifiedLayoutContainer.appendChild(middleColumn) - unifiedLayoutContainer.appendChild(rightColumn) - unifiedLayoutContainer.appendChild(sidebandsColumn) + // Column 4 — what the next feature will look like, or what the selected one + // does. + const stylePanel = createStylePanel(instance) + + // Column 5 — the three annotation tables, taking whatever width is left. + const tables = createAnnotationTables() + + unifiedLayoutContainer.appendChild(modeColumn) + unifiedLayoutContainer.appendChild(guidance.column) + unifiedLayoutContainer.appendChild(readout.column) + unifiedLayoutContainer.appendChild(stylePanel) + unifiedLayoutContainer.appendChild(tables.tables) return { unifiedLayoutContainer, - leftColumn, - middleColumn, - rightColumn, - sidebandsColumn, modeColumn, - guidanceColumn, - controlsColumn, - markersContainer, - harmonicsContainer, - sidebandsContainer, - timeLED, - freqLED, - speedLED, - colorPicker + guidanceColumn: guidance.column, + guidancePanel: guidance.body, + guidanceTitle: guidance.title, + readoutColumn: readout.column, + markersContainer: tables.markersContainer, + harmonicsContainer: tables.harmonicsContainer, + sidebandsContainer: tables.sidebandsContainer, + timeLED: readout.timeLED, + freqLED: readout.freqLED, + speedLED: readout.speedLED, + kicker: readout.kicker, + colorPicker: stylePanel } } /** - * Create the sidebands container for sidebands mode. + * Update the universal cursor readouts as the pointer moves over the gram. * - * The same header-plus-table structure as the markers panel (no action slot — - * sidebands have no "+ Manual" equivalent), so all three panels carry their - * rule, spacing and heading position from the one `gram-frame-panel-header` - * rule rather than from per-panel inline styles. It shares the right column - * with the harmonics panel, one shown at a time — see the note there — so it is - * `flex: 1` like its counterpart and fills the column when it is the one shown. - * @returns {HTMLDivElement} The sidebands container - */ -function createSidebandsContainer() { - const sidebandsContainer = document.createElement('div') - sidebandsContainer.className = 'gram-frame-sidebands-persistent-container' - - const header = document.createElement('div') - header.className = 'gram-frame-panel-header' - - const label = document.createElement('h4') - label.textContent = 'Sidebands' - header.appendChild(label) - sidebandsContainer.appendChild(header) - - return sidebandsContainer -} - -/** - * Create markers container for analysis mode - * @returns {HTMLDivElement} The markers container - */ -function createMarkersContainer() { - // Layout comes from the shared `*-persistent-container` CSS rule, not from - // inline styles: they duplicated it exactly, and an inline `display` cannot be - // overridden by the stylesheet — which is how the sidebands panel takes its - // turn in the right column. - const markersContainer = document.createElement('div') - markersContainer.className = 'gram-frame-markers-persistent-container' - - // Same header row as the harmonics panel, minus the action slot: both panels - // then carry their rule, their spacing and their heading position from one - // CSS rule instead of two sets of inline styles that had drifted apart. - const markersHeader = document.createElement('div') - markersHeader.className = 'gram-frame-panel-header' - - const markersLabel = document.createElement('h4') - markersLabel.textContent = 'Markers' - markersHeader.appendChild(markersLabel) - markersContainer.appendChild(markersHeader) - - return markersContainer -} - -/** - * Create harmonics container for harmonics mode - * @returns {HTMLDivElement} The harmonics container - */ -function createHarmonicsContainer() { - const harmonicsContainer = document.createElement('div') - harmonicsContainer.className = 'gram-frame-harmonics-persistent-container' - - // Create header container with title and button area - const harmonicsHeader = document.createElement('div') - harmonicsHeader.className = 'gram-frame-panel-header gram-frame-harmonics-header' - - const harmonicsLabel = document.createElement('h4') - harmonicsLabel.textContent = 'Harmonics' - - const harmonicsButtonContainer = document.createElement('div') - harmonicsButtonContainer.className = 'gram-frame-harmonics-button-container' - harmonicsButtonContainer.style.flexShrink = '0' - - harmonicsHeader.appendChild(harmonicsLabel) - harmonicsHeader.appendChild(harmonicsButtonContainer) - harmonicsContainer.appendChild(harmonicsHeader) - - return harmonicsContainer -} - -/** - * Update universal cursor readouts (time/freq LEDs) regardless of active mode + * Stands aside while a feature is selected: the column is then reading that + * feature, not the pointer, and its kicker says so. `refreshReadoutTarget` in + * `CursorReadout.js` owns what it shows in that case. * @param {GramFrame} instance - GramFrame instance * @param {DataCoordinates} dataCoords - Data coordinates {freq, time} */ export function updateUniversalCursorReadouts(instance, dataCoords) { + const { selection } = instance.state + if (selection && selection.selectedId) { + return + } + if (instance.ui.timeLED) { const timeValue = instance.ui.timeLED.querySelector('.gram-frame-led-value') if (timeValue) { timeValue.textContent = formatTime(dataCoords.time) } } - + if (instance.ui.freqLED) { const freqValue = instance.ui.freqLED.querySelector('.gram-frame-led-value') if (freqValue) { @@ -260,15 +113,8 @@ export function updateUniversalCursorReadouts(instance, dataCoords) { /** * Refresh every persistent panel, regardless of which mode is active. * - * Modes are discovered by capability, not by name. This function used to name - * `analysis` and `harmonics`, cast both to `any` to reach methods the mode - * interface did not declare, and resolve the harmonics panel element on that - * mode's behalf. A fifth mode owning a panel now refreshes here with no edit to - * this file (spec 167, FR-006, AS-4.2, SC-003). - * @param {GramFrame} instance - GramFrame instance + * Re-exported from `core/panelRefresh.js`, where the loop lives so the + * selection and restyle layers can share it: this module's name is the one the + * rest of the UI knows it by. */ -export function updatePersistentPanels(instance) { - Object.values(instance.modes) - .filter(isPanelOwner) - .forEach(mode => mode.refreshPanel()) -} \ No newline at end of file +export { refreshPanels as updatePersistentPanels } from '../core/panelRefresh.js' diff --git a/src/components/MarkerLabelModal.js b/src/components/MarkerLabelModal.js deleted file mode 100644 index 5cc97f91..00000000 --- a/src/components/MarkerLabelModal.js +++ /dev/null @@ -1,155 +0,0 @@ -/** - * The marker-label dialog (feature 231). - * - * Opened from the Label button in a markers-table row, it lets an analyst enter - * or edit the free text shown next to that marker on the gram. Clearing the - * field and saving removes the label, so one dialog covers add, edit and - * remove. - * - * Built with `createElement` rather than `innerHTML`: the current label is user - * text being put back into the DOM, and `textContent`/`value` assignment keeps - * that unambiguously inert. - * - * The label wraps its input rather than pointing at one by id, so no - * page-global name escapes into the host document (R9-08). - */ - -/// - -import { MAX_MARKER_LABEL_LENGTH, normalizeMarkerLabel } from '../utils/markerLabel.js' - -/** - * Show the label dialog for one marker. - * - * The dialog is modal-by-convention (a full-screen overlay), self-closing, and - * removes itself on save, cancel, Escape, or a click on the backdrop. `onSave` - * fires only on save, with the normalised label — `undefined` when the analyst - * cleared the field. - * - * @param {string|undefined} currentLabel - The marker's existing label, if any - * @param {function(string|undefined): void} onSave - Called with the new label on save - * @returns {HTMLDivElement} The overlay element, for callers that need to dismiss it - */ -export function showMarkerLabelModal(currentLabel, onSave) { - const overlay = document.createElement('div') - overlay.className = 'gram-frame-modal-overlay gram-frame-marker-label-modal' - - const modal = document.createElement('div') - modal.className = 'gram-frame-modal' - - const header = document.createElement('div') - header.className = 'gram-frame-modal-header' - const heading = document.createElement('h3') - heading.textContent = currentLabel ? 'Edit Marker Label' : 'Add Marker Label' - header.appendChild(heading) - - const body = document.createElement('div') - body.className = 'gram-frame-modal-body' - const inputGroup = document.createElement('div') - inputGroup.className = 'gram-frame-modal-input-group' - - const inputLabel = document.createElement('label') - inputLabel.appendChild(document.createTextNode('Label:')) - - const input = document.createElement('input') - input.type = 'text' - input.className = 'gram-frame-marker-label-input' - input.maxLength = MAX_MARKER_LABEL_LENGTH - input.placeholder = 'Enter a label for this marker' - input.value = currentLabel || '' - inputLabel.appendChild(input) - - const hint = document.createElement('div') - hint.className = 'gram-frame-modal-hint' - hint.textContent = 'Leave empty to remove the label.' - - inputGroup.appendChild(inputLabel) - inputGroup.appendChild(hint) - body.appendChild(inputGroup) - - const footer = document.createElement('div') - footer.className = 'gram-frame-modal-footer' - const cancelButton = document.createElement('button') - cancelButton.type = 'button' - cancelButton.className = 'gram-frame-modal-btn gram-frame-modal-cancel' - cancelButton.textContent = 'Cancel' - const saveButton = document.createElement('button') - saveButton.type = 'button' - saveButton.className = 'gram-frame-modal-btn gram-frame-modal-add gram-frame-modal-save' - saveButton.textContent = 'Save' - footer.appendChild(cancelButton) - footer.appendChild(saveButton) - - modal.appendChild(header) - modal.appendChild(body) - modal.appendChild(footer) - overlay.appendChild(modal) - - // Remembered before the dialog steals focus, so it can be given back. - const opener = /** @type {HTMLElement|null} */ (document.activeElement) - - document.body.appendChild(overlay) - - /** - * Remove the dialog and hand focus back to whatever opened it. - * - * Without the restore, `document.activeElement` was left on `` after - * every Save or Cancel, so a keyboard user was returned to the top of the - * document rather than to the Label button they pressed (R9-08). - */ - function closeModal() { - document.removeEventListener('keydown', onDocumentKeydown, true) - if (overlay.parentNode) { - overlay.parentNode.removeChild(overlay) - } - if (opener && typeof opener.focus === 'function' && opener.isConnected) { - opener.focus() - } - } - - /** - * Escape closes the dialog wherever the focus happens to be. - * - * Bound on the document, in the capture phase, rather than on the input: - * Escape used to work only while the text field had focus, so tabbing to - * Cancel and pressing it left the dialog open with no way out but the mouse. - * The listener is removed on close, and the event is stopped so it cannot - * also reach the component's own document-level key handling. - * @param {KeyboardEvent} e - The key event - */ - function onDocumentKeydown(e) { - if (e.key === 'Escape') { - e.stopPropagation() - e.preventDefault() - closeModal() - } - } - - /** - * Commit the entered text and close. - */ - function save() { - onSave(normalizeMarkerLabel(input.value)) - closeModal() - } - - input.addEventListener('keydown', (e) => { - if (e.key === 'Enter') { - save() - } - }) - document.addEventListener('keydown', onDocumentKeydown, true) - cancelButton.addEventListener('click', closeModal) - saveButton.addEventListener('click', save) - overlay.addEventListener('click', (e) => { - if (e.target === overlay) { - closeModal() - } - }) - - // Focus and select so editing an existing label can start by typing over it. - input.focus() - input.select() - - return overlay -} diff --git a/src/components/ModeButtons.js b/src/components/ModeButtons.js index 3e938262..c263e0ad 100644 --- a/src/components/ModeButtons.js +++ b/src/components/ModeButtons.js @@ -1,7 +1,18 @@ /** - * Mode Buttons Component for GramFrame - * - * Provides mode switching UI with buttons and guidance panel + * The mode rail: which tool is armed, and the view controls beneath it. + * + * A kicker, five mode buttons stacked in roster order, then — pushed to the + * foot of the rail by a spacer and fenced off by a rule — the view controls. + * + * The view controls used to sit inside Pan's row, because Pan is the mode that + * declares them. That made one row of five four controls wide while the rest + * held one, and the armed-mode read had to compete with it. They are the same + * buttons, still declared by the mode that owns the viewport; only where they + * are drawn has changed. They stay visible in every mode because zooming and + * fitting are things an analyst does while measuring, not instead of it. + * + * Clear is deliberately NOT here: it removes annotations, not view state, and + * it lives in the sidebands table's footer (see `MainUI.js`). */ /// @@ -12,19 +23,31 @@ import { createIcon, createIconLabel } from './icons.js' /** @typedef {import('../modes/BaseMode.js').BaseMode} BaseMode */ /** - * Create mode switching UI with buttons and guidance panel + * Build the mode rail. * @param {HTMLElement} modeCell - Container element for mode UI - * @param {GramFrameState} state - Current state object + * @param {ModeType} activeMode - The mode armed when the rail is built * @param {Function} modeSwitchCallback - Callback function for mode changes * @param {Object} modes - Mode instances to get command buttons from * @returns {ModeUIElements} Object containing references to mode UI elements */ -export function createModeSwitchingUI(modeCell, state, modeSwitchCallback, modes = {}) { - // Create mode buttons container +export function createModeSwitchingUI(modeCell, activeMode, modeSwitchCallback, modes = {}) { const modesContainer = document.createElement('div') modesContainer.className = 'gram-frame-modes' - - // Create mode buttons, in the roster's order (R9-12). This list used to be + + const kicker = document.createElement('div') + kicker.className = 'gram-frame-kicker' + kicker.textContent = 'Mode' + modesContainer.appendChild(kicker) + + const list = document.createElement('div') + list.className = 'gram-frame-mode-list' + modesContainer.appendChild(list) + + // The rail's foot, where the view controls sit whatever mode is armed. + const commandRow = document.createElement('div') + commandRow.className = 'gram-frame-mode-commands' + + // Modes are listed in the roster's order (R9-12). This list used to be // written out here, so landing Sidebands meant editing a component that has // no business knowing which modes exist. /** @type {ModeType[]} */ @@ -33,93 +56,96 @@ export function createModeSwitchingUI(modeCell, state, modeSwitchCallback, modes const modeButtons = {} /** @type {Object} */ const commandButtons = {} - + modeTypes.forEach(modeType => { const modeInstance = modes[modeType] - const commandButtonDefs = modeInstance && typeof modeInstance.getCommandButtons === 'function' - ? modeInstance.getCommandButtons() - : [] - // Create command buttons container for this mode - const modeGroup = document.createElement('div') - modeGroup.className = 'gram-frame-mode-group' - - // Store command buttons for this mode commandButtons[modeType] = [] - // Create main mode button. - // - // It leads its group, and its commands follow it. They used to be split in - // half and wrapped around it, which put Pan's mode button a button's width - // in from the left while the other four started at the column's edge — the - // one row out of line was the row already short of space (issue #310). const button = document.createElement('button') + button.type = 'button' button.className = 'gram-frame-mode-btn' const displayName = getModeDisplayName(modeType) - applyButtonFace(button, displayName, getModeIcon(modeType)) - // Named on hover like the command buttons beside it. It matters most for - // the one showing a glyph, which is why it arrived with the icons. + applyModeFace(button, displayName, getModeIcon(modeType)) button.title = displayName button.dataset.mode = modeType - // Set active state for current mode - if (modeType === state.mode) { + if (modeType === activeMode) { button.classList.add('active') } - + // Set disabled state based on mode's isEnabled method - const modeInstanceForDisabled = modes[modeType] - if (modeInstanceForDisabled && typeof modeInstanceForDisabled.isEnabled === 'function') { - if (!modeInstanceForDisabled.isEnabled()) { - button.disabled = true - button.classList.add('disabled') - } + if (modeInstance && typeof modeInstance.isEnabled === 'function' && !modeInstance.isEnabled()) { + button.disabled = true + button.classList.add('disabled') } - - // Add click handler + button.addEventListener('click', (event) => { event.preventDefault() if (!button.disabled) { modeSwitchCallback(modeType) } }) - + modeButtons[modeType] = button - modeGroup.appendChild(button) + list.appendChild(button) - // The mode's own commands, in the order the mode lists them. + // The mode's own commands, in the order the mode lists them, into the + // rail's footer. + const commandButtonDefs = modeInstance && typeof modeInstance.getCommandButtons === 'function' + ? modeInstance.getCommandButtons() + : [] commandButtonDefs.forEach((/** @type {CommandButton} */ buttonDef) => { const cmdButton = createCommandButton(buttonDef) - modeGroup.appendChild(cmdButton) + commandRow.appendChild(cmdButton) commandButtons[modeType].push(cmdButton) }) - - modesContainer.appendChild(modeGroup) }) - - // Create guidance panel (content will be set by current mode) - const guidancePanel = document.createElement('div') - guidancePanel.className = 'gram-frame-guidance' - - // Append all to mode header + + // A spacer, so the view controls sit against the foot of the rail however + // many modes there are. + const spacer = document.createElement('div') + spacer.className = 'gram-frame-mode-spacer' + modesContainer.appendChild(spacer) + modesContainer.appendChild(commandRow) + modeCell.appendChild(modesContainer) - modeCell.appendChild(guidancePanel) - + return { modesContainer, modeButtons, - commandButtons, - guidancePanel + commandButtons + } +} + +/** + * Put a mode's glyph and its word on its button. + * + * Both, not one or the other: in a stacked rail the shape is what the eye + * lands on and the word is what settles it. A mode with no glyph in the + * catalogue simply shows its word. + * @param {HTMLButtonElement} button - The button to face + * @param {string} text - The mode's name + * @param {string} [icon] - Name of a glyph in `components/icons.js` + */ +function applyModeFace(button, text, icon) { + const glyph = createIcon(icon) + if (glyph) { + button.appendChild(glyph) } + const label = document.createElement('span') + label.className = 'gram-frame-mode-btn-label' + label.textContent = text + button.appendChild(label) } /** - * Put a word, or a glyph standing for that word, on a button. + * Put a word, or a glyph standing for that word, on a command button. * * An icon button keeps the word in a visually hidden span rather than dropping - * it: that span is the button's accessible name, so a screen reader still hears - * "Pan" and "Fit", and a test still finds the button by the name an analyst - * would call it. + * it: that span is the button's accessible name, so a screen reader still + * hears "Fit", and a test still finds the button by the name an analyst would + * call it. * @param {HTMLButtonElement} button - The button to face * @param {string} text - The word the button stands for * @param {string} [icon] - Name of a glyph in `components/icons.js` @@ -142,22 +168,22 @@ function applyButtonFace(button, text, icon) { */ function createCommandButton(buttonDef) { const button = document.createElement('button') + button.type = 'button' button.className = 'gram-frame-command-btn' applyButtonFace(button, buttonDef.label, buttonDef.icon) button.title = buttonDef.title - + // Set initial enabled state if (buttonDef.isEnabled) { button.disabled = !buttonDef.isEnabled() } - - // Add click handler + button.addEventListener('click', (event) => { event.preventDefault() event.stopPropagation() buttonDef.action() }) - + return button } @@ -172,7 +198,7 @@ export function updateCommandButtonStates(commandButtons, modes) { if (modeInstance && typeof modeInstance.getCommandButtons === 'function') { const buttonDefs = modeInstance.getCommandButtons() const buttons = commandButtons[modeType] - + buttons.forEach((button, index) => { const buttonDef = buttonDefs[index] if (buttonDef && buttonDef.isEnabled) { @@ -192,11 +218,11 @@ export function updateModeButtonStates(modeButtons, modes) { Object.keys(modeButtons).forEach(modeType => { const modeInstance = modes[modeType] const button = modeButtons[modeType] - + if (modeInstance && typeof modeInstance.isEnabled === 'function' && button) { const isEnabled = modeInstance.isEnabled() button.disabled = !isEnabled - + if (isEnabled) { button.classList.remove('disabled') } else { @@ -205,4 +231,3 @@ export function updateModeButtonStates(modeButtons, modes) { } }) } - diff --git a/src/components/PinToggle.js b/src/components/PinToggle.js index 99907e25..e218cf2e 100644 --- a/src/components/PinToggle.js +++ b/src/components/PinToggle.js @@ -1,101 +1,91 @@ /** - * Pin visibility toggle for GramFrame overlays. + * Pin style for GramFrame pin sets. * - * A checkbox in the style panel's Pin sets band (see ColorPicker.js) that - * controls whether a pin set — a harmonic set, or a sideband set (issue #241) — - * draws its full-height vertical pin lines. Pins are the only style in that - * panel that pin sets alone understand, which is why the band is fenced off - * below a rule. With the toggle off, a set renders as its symbols and numbers - * over short mini-pins — the style experienced analysts use to stack many sets - * over dense data without the lines swamping it. + * A two-option segmented control in the style panel deciding whether a pin set + * — a harmonic set, or a sideband set (issue #241) — draws full-height vertical + * pin lines or short mini-pins. The short style is what experienced analysts + * use to stack many sets over dense data without the lines swamping it. * - * It is labelled "Tall Pins" rather than "Pin" because a pin-less set still - * draws mini-pins (issue #232): the choice is between tall pins and short ones, - * not between pins and none. + * TALL / MINI rather than a "Tall Pins" checkbox, because that is what the + * choice actually is (issue #232): a pin-less set still draws mini-pins, so an + * unchecked box was naming an option that does not exist. Both options are now + * on screen and neither has to be inferred. * - * When a pin set is selected, toggling restyles that set in place; otherwise - * the choice is written to `state.showHarmonicPin` and applied to the next - * created set. The preference is on at the start of each browser session - * and remembered (sessionStorage) for the rest of it. + * When a pin set is selected and the panel is targeting it, choosing restyles + * that set in place; otherwise the choice is written to + * `state.showHarmonicPin` and applied to the next created set. The preference + * is tall at the start of each browser session and remembered (sessionStorage) + * for the rest of it. * - * The toggle has no meaning for analysis markers (they have no pin), so it is - * disabled while a marker is selected. + * The choice has no meaning for analysis markers (they have no pin), so the + * control is disabled while a marker is targeted, and says why on hover. */ /// -import { savePinPreference } from '../core/storage.js' +import { savePinPreference } from '../core/preferences.js' import { dispatch } from '../core/state.js' +import { createSegmented } from './Segmented.js' import { showStorageWarning, clearStorageWarning } from './StorageWarning.js' /** - * The toggle's handle. `setEnabled(false)` explains itself in the title too, - * since a marker has no pin for the toggle to act on. + * The control's handle. * @typedef {Object} PinControl - * @property {function(boolean): void} setValue - Show pins as on or off - * @property {function(boolean): void} setEnabled - Enable or disable the toggle + * @property {function(boolean): void} setValue - Show tall pins as chosen, or mini + * @property {function(boolean): void} setEnabled - Enable or disable the control */ /** - * Create the pin toggle row. + * Create the pin-style control. * @param {GramFrame} instance - GramFrame instance - * @returns {{element: HTMLLabelElement, control: PinControl}} The toggle row and its handle + * @returns {{element: HTMLElement, control: PinControl}} The control and its handle */ export function createPinToggle(instance) { const state = instance.state + const segmented = createSegmented( + [{ value: true, label: 'Tall' }, { value: false, label: 'Mini' }], + 'Pin style', + (showPin) => choosePinStyle(instance, showPin, segmented.setValue) + ) + segmented.element.classList.add('gram-frame-pin-toggle') + segmented.setValue(state.showHarmonicPin !== false) - const row = document.createElement('label') - row.className = 'gram-frame-pin-toggle' - row.title = 'Draw harmonic and sideband sets with full-height pin lines instead of mini-pins' - - const checkbox = document.createElement('input') - checkbox.type = 'checkbox' - checkbox.className = 'gram-frame-pin-toggle-input' - checkbox.checked = state.showHarmonicPin !== false - checkbox.setAttribute('aria-label', 'Show tall pins') - - const text = document.createElement('span') - text.className = 'gram-frame-pin-toggle-label' - text.textContent = 'Tall Pins' - - row.appendChild(checkbox) - row.appendChild(text) - - checkbox.addEventListener('change', () => { - const showPin = checkbox.checked - // Route to the selected pin set — harmonic or sideband — when one is - // selected (restyle in place), otherwise set the style for the next created - // set and remember it for the rest of the session. - if (!instance.interaction.applyPinToSelectedFeature || !instance.interaction.applyPinToSelectedFeature(showPin)) { - state.showHarmonicPin = showPin - // The choice applies immediately either way; the warning only says it - // will not survive the next page load (GF-16). - if (savePinPreference(showPin)) { - clearStorageWarning(instance) - } else { - showStorageWarning(instance, 'The pin preference could not be saved — it applies to this page only.') - } - // Dispatch: this is a state change listeners care about, and only the - // "Large" toggle used to say so (issue #268, BH-30). - dispatch(instance) - } - }) - - // Returned rather than written onto `instance.interaction`: see the note in - // SymbolPicker (issue #267). /** @type {PinControl} */ const control = { setValue(showPin) { - checkbox.checked = showPin + segmented.setValue(showPin) }, setEnabled(enabled) { - checkbox.disabled = !enabled - row.classList.toggle('gram-frame-pin-toggle-disabled', !enabled) - row.title = enabled - ? 'Draw harmonic and sideband sets with full-height pin lines instead of mini-pins' - : 'Tall pins apply to harmonic and sideband sets only' + segmented.setEnabled(enabled, 'Tall pins apply to harmonic and sideband sets only') } } - return { element: row, control } + return { element: segmented.element, control } +} + +/** + * Route a chosen pin style: to the targeted pin set, or to the defaults for the + * next created one. + * @param {GramFrame} instance - GramFrame instance + * @param {boolean} showPin - Whether pins are drawn full height + * @param {function(boolean): void} show - Reflect the choice in the control + * @returns {void} + */ +function choosePinStyle(instance, showPin, show) { + const state = instance.state + const apply = instance.interaction.applyPinToSelectedFeature + if (!apply || !apply(showPin)) { + state.showHarmonicPin = showPin + // The choice applies immediately either way; the warning only says it will + // not survive the next page load (GF-16). + if (savePinPreference(showPin)) { + clearStorageWarning(instance) + } else { + showStorageWarning(instance, 'The pin preference could not be saved — it applies to this page only.') + } + // Dispatch: this is a state change listeners care about, and only the + // "Large" toggle used to say so (issue #268, BH-30). + dispatch(instance) + } + show(showPin) } diff --git a/src/components/Segmented.js b/src/components/Segmented.js new file mode 100644 index 00000000..d160d94c --- /dev/null +++ b/src/components/Segmented.js @@ -0,0 +1,76 @@ +/** + * A two-or-more option segmented control. + * + * The panel needs three of these — tall pins or mini, normal symbols or large, + * and nothing stops a fourth — and they are all the same object: a short row of + * mutually exclusive choices where both options are worth showing, because the + * pair is the question. A checkbox can only show one of them and leaves the + * analyst to infer the other, which is what the pin toggle used to do. + * + * Rendered as buttons in a `radiogroup` rather than as radio inputs: the + * styling is a joined row with one filled segment, which native radios cannot + * be talked into, and `aria-checked` on a `radio` role says the same thing to a + * screen reader. + */ + +/** + * The control's handle. + * @typedef {Object} SegmentedControl + * @property {HTMLDivElement} element - The control, ready to mount + * @property {function(*): void} setValue - Show this option as chosen + * @property {function(boolean, string=): void} setEnabled - Enable or disable the whole control + */ + +/** + * Build a segmented control. + * @template T + * @param {Array<{value: T, label: string}>} options - The choices, in display order + * @param {string} groupLabel - Accessible name for the group + * @param {function(T): void} onChange - Called with the chosen value + * @returns {SegmentedControl} The control and its handle + */ +export function createSegmented(options, groupLabel, onChange) { + const element = document.createElement('div') + element.className = 'gram-frame-segmented' + element.setAttribute('role', 'radiogroup') + element.setAttribute('aria-label', groupLabel) + + /** @type {HTMLButtonElement[]} */ + const buttons = options.map(option => { + const button = document.createElement('button') + button.type = 'button' + button.className = 'gram-frame-segmented-option' + button.setAttribute('role', 'radio') + button.setAttribute('aria-checked', 'false') + button.textContent = option.label + button.addEventListener('click', event => { + event.preventDefault() + if (!button.disabled) { + onChange(option.value) + } + }) + element.appendChild(button) + return button + }) + + return { + element, + + setValue(value) { + options.forEach((option, index) => { + const chosen = option.value === value + buttons[index].classList.toggle('gram-frame-segmented-selected', chosen) + buttons[index].setAttribute('aria-checked', chosen ? 'true' : 'false') + }) + }, + + setEnabled(enabled, reason) { + element.classList.toggle('gram-frame-segmented-disabled', !enabled) + // The reason rides the control rather than being printed beside it: the + // row is 46px of label and a 24px control, with nowhere to put a + // sentence, and "why is this greyed out" is a hover question. + element.title = enabled ? '' : (reason || '') + buttons.forEach(button => { button.disabled = !enabled }) + } + } +} diff --git a/src/components/SidebandPanel.js b/src/components/SidebandPanel.js index 96773c48..b34a4f8d 100644 --- a/src/components/SidebandPanel.js +++ b/src/components/SidebandPanel.js @@ -65,11 +65,12 @@ function createSidebandDeleteButton(sidebandSet) { export function createSidebandPanel(container, instance) { const table = createDiffingTable(container, { columns: [ - { label: '', width: '15%' }, - { label: 'Freq (Hz)', width: '35%', cellClassName: 'gram-frame-sideband-freq' }, - { label: 'Spacing (Hz)', width: '35%', cellClassName: 'gram-frame-sideband-spacing' }, - { label: '', width: '15%' } + { label: '', width: '14%' }, + { label: 'Freq', width: '36%', cellClassName: 'gram-frame-sideband-freq gram-frame-cell-numeric' }, + { label: 'Spacing', width: '36%', cellClassName: 'gram-frame-sideband-spacing gram-frame-cell-numeric' }, + { label: '', width: '14%', cellClassName: 'gram-frame-cell-action' } ], + emptyMessage: 'Click to set the sideband origin', rowAttribute: 'data-sideband-id', rowClassName: 'gram-frame-sideband-row', rowKey: (sidebandSet) => sidebandSet.id, @@ -80,20 +81,10 @@ export function createSidebandPanel(container, instance) { createSidebandDeleteButton(sidebandSet) ], deleteSelector: '.gram-frame-sideband-delete', - onSelect: (sidebandSetId, _sidebandSet, index) => { - // Toggle selection - if (instance.state.selection.selectedType === 'sidebandSet' && - instance.state.selection.selectedId === sidebandSetId) { - instance.interaction.clearSelection() - } else { - instance.interaction.setSelection('sidebandSet', sidebandSetId, index) - } - }, + onSelect: (sidebandSetId, _sidebandSet, index) => + instance.interaction.toggleSelection('sidebandSet', sidebandSetId, index), onDelete: (sidebandSetId) => instance.interaction.removeSidebandSet(sidebandSetId), - isSelected: (sidebandSetId) => ( - instance.state.selection.selectedType === 'sidebandSet' && - instance.state.selection.selectedId === sidebandSetId - ) + isSelected: (sidebandSetId) => instance.interaction.isFeatureSelected('sidebandSet', sidebandSetId) }) const panel = /** @type {HTMLElement} */ (table.element.parentElement) diff --git a/src/components/StylePanel.js b/src/components/StylePanel.js new file mode 100644 index 00000000..fee5eb32 --- /dev/null +++ b/src/components/StylePanel.js @@ -0,0 +1,242 @@ +/** + * The style panel, and the twin tabs that say what it is about to change. + * + * One question caused most of the confusion in the old panel: does the colour + * slider set the colour of the next feature, or restyle the one I just clicked? + * It did both, silently, depending on state the analyst could not see. The tabs + * make that state visible and selectable — "New features" or "Selected: ABS" — + * and the panel's contents follow the armed tab: + * + * - **New features** — colour, symbol, pin style, and a footer saying the + * choices apply to everything added from now on, in any mode. + * - **Selected** — the same controls pointed at that feature, plus the two + * things that only make sense for one that already exists: its label, edited + * in place rather than through a dialog, and a nudge pair mirroring the arrow + * keys. Its footer offers Delete. + * + * The routing itself is unchanged (`applyColorToSelectedFeature` and friends); + * this panel's job is to make the target visible, and to let the analyst change + * targets without giving up their selection. + */ + +/// + +import { createColorSlider } from './ColorPicker.js' +import { createSymbolSelect, symbolGlyph } from './SymbolPicker.js' +import { createPinToggle } from './PinToggle.js' +import { nudgeSelection, MOVEMENT_INCREMENTS } from '../core/keyboardControl.js' +import { getActiveStyle } from '../core/featureStyle.js' +import { MAX_MARKER_LABEL_LENGTH } from '../utils/markerLabel.js' +import { describeStyleTarget, deleteStyleTarget, setStyleTarget, renameSelectedMarker } from './styleTarget.js' + +/** + * Build one labelled row of the panel. + * @param {string} text - The row's caption + * @param {HTMLElement} control - The control it captions + * @param {string} [className] - Extra class for the row + * @returns {HTMLDivElement} The row + */ +function createRow(text, control, className) { + const row = document.createElement('div') + row.className = className ? `gram-frame-style-row ${className}` : 'gram-frame-style-row' + const label = document.createElement('div') + label.className = 'gram-frame-style-group-label' + label.textContent = text + row.appendChild(label) + row.appendChild(control) + return row +} + +/** + * Create the style panel. + * @param {GramFrame} instance - GramFrame instance + * @returns {HTMLDivElement} The panel element + */ +export function createStylePanel(instance) { + // `gram-frame-color-picker` stays on the panel: it is the handle the instance + // and the stylesheet know it by, and the panel is still the thing that owns + // colour. + const container = document.createElement('div') + container.className = 'gram-frame-color-picker gram-frame-style-column' + + const tabs = document.createElement('div') + tabs.className = 'gram-frame-style-tabs' + + const newTab = document.createElement('button') + newTab.type = 'button' + newTab.className = 'gram-frame-style-tab gram-frame-style-tab-new' + newTab.textContent = 'New features' + newTab.title = 'Set the style every feature you add from now on will take' + newTab.addEventListener('click', event => { + event.preventDefault() + setStyleTarget(instance, 'new') + }) + + const selectedTab = document.createElement('button') + selectedTab.type = 'button' + selectedTab.className = 'gram-frame-style-tab gram-frame-style-tab-selected' + selectedTab.addEventListener('click', event => { + event.preventDefault() + if (!selectedTab.disabled) { + setStyleTarget(instance, 'selected') + } + }) + + tabs.appendChild(newTab) + tabs.appendChild(selectedTab) + container.appendChild(tabs) + + const body = document.createElement('div') + body.className = 'gram-frame-style-body' + container.appendChild(body) + + // --- Label: the selected feature's own text, edited where it is read ------- + const labelInput = document.createElement('input') + labelInput.type = 'text' + labelInput.className = 'gram-frame-style-label-input' + labelInput.maxLength = MAX_MARKER_LABEL_LENGTH + labelInput.title = 'Optional — clear the field to remove the label' + labelInput.setAttribute('aria-label', 'Marker label') + labelInput.addEventListener('input', () => renameSelectedMarker(instance, labelInput.value)) + const labelRow = createRow('Label', labelInput, 'gram-frame-style-row-label') + + // --- Colour --------------------------------------------------------------- + const color = createColorSlider(instance, () => { + if (instance.interaction.syncStyleControls) { + instance.interaction.syncStyleControls() + } + }) + + // --- Symbol --------------------------------------------------------------- + const symbol = createSymbolSelect(instance) + const symbolRow = createRow('Symbol', symbol.element, 'gram-frame-style-row-symbol') + + // --- Pin style ------------------------------------------------------------ + const pin = createPinToggle(instance) + const pinRow = createRow('Pin sets', pin.element) + + // --- Nudge: the arrow keys, reachable from the panel ---------------------- + const nudge = document.createElement('div') + nudge.className = 'gram-frame-nudge' + ;[['←', -1, 'Nudge left'], ['→', 1, 'Nudge right']].forEach(([glyph, sign, title]) => { + const button = document.createElement('button') + button.type = 'button' + button.className = 'gram-frame-nudge-btn' + button.textContent = /** @type {string} */ (glyph) + button.title = /** @type {string} */ (title) + button.addEventListener('click', event => { + event.preventDefault() + const step = event.shiftKey ? MOVEMENT_INCREMENTS.fast : MOVEMENT_INCREMENTS.normal + nudgeSelection(instance, { dx: /** @type {number} */ (sign) * step, dy: 0 }) + }) + nudge.appendChild(button) + }) + const nudgeNote = document.createElement('span') + nudgeNote.className = 'gram-frame-nudge-note' + nudgeNote.textContent = 'or arrow keys' + nudge.appendChild(nudgeNote) + const nudgeRow = createRow('Nudge', nudge) + + body.appendChild(labelRow) + body.appendChild(color.element) + body.appendChild(symbolRow) + body.appendChild(pinRow) + body.appendChild(nudgeRow) + + const spacer = document.createElement('div') + spacer.className = 'gram-frame-style-spacer' + body.appendChild(spacer) + + // --- Footer: what the panel is about to affect, and the way out ----------- + const footer = document.createElement('div') + footer.className = 'gram-frame-style-footer' + + const footerGlyph = document.createElement('span') + footerGlyph.className = 'gram-frame-style-footer-glyph' + + const footerNote = document.createElement('div') + footerNote.className = 'gram-frame-style-footer-note' + + const deleteButton = document.createElement('button') + deleteButton.type = 'button' + deleteButton.className = 'gram-frame-style-delete' + deleteButton.textContent = 'Delete' + deleteButton.title = 'Delete the selected feature' + deleteButton.addEventListener('click', event => { + event.preventDefault() + deleteStyleTarget(instance) + }) + + footer.appendChild(footerGlyph) + footer.appendChild(footerNote) + footer.appendChild(deleteButton) + container.appendChild(footer) + + // Sync every control — and the panel's whole face — to whatever is targeted. + // + // The three handles are closed over, not read back off `instance.interaction`. + // Each control used to install itself there during construction and this + // function used to look them up by name: an ordering contract `tsc` could not + // check, which reordering the panel or mounting a picker twice would break in + // silence. They are built here, so they can simply be held here (issue #267). + instance.interaction.syncStyleControls = () => { + const style = getActiveStyle(instance) + color.control.setValue(style.color) + symbol.control.setValue(style.symbol) + symbol.control.setTint(style.color) + pin.control.setValue(style.showPin) + // Markers have no pin, so the control is disabled while one is targeted. + pin.control.setEnabled(style.pinApplies) + + const target = describeStyleTarget(instance) + container.classList.toggle('gram-frame-style-targeting', target.editing) + newTab.classList.toggle('gram-frame-style-tab-armed', !target.editing) + selectedTab.classList.toggle('gram-frame-style-tab-armed', target.editing) + selectedTab.disabled = !target.selectable + selectedTab.title = target.selectable + ? `Restyle ${target.name}` + : 'Select a row to restyle that feature' + writeTabFace(selectedTab, target.selectable ? `Selected: ${target.name}` : 'Selected: none', + target.selectable ? style.color : '', symbolGlyph(style.symbol)) + + labelRow.hidden = !target.labelled + if (target.labelled && document.activeElement !== labelInput) { + labelInput.value = target.label + } + nudgeRow.hidden = !target.editing + footerGlyph.style.color = style.color + // The footer's ghosted mark is what a new feature would be drawn as; with + // one targeted, the tab beside its name is already carrying its mark. + footerGlyph.textContent = target.editing ? '' : symbolGlyph(style.symbol) + footerNote.textContent = target.editing + ? `changes ${target.name} only` + : 'applies to every feature you add, in any mode' + deleteButton.hidden = !target.editing + } + instance.interaction.syncStyleControls() + + return container +} + +/** + * Write the selected tab's face: the targeted feature's own mark in its own + * colour, then the text. + * @param {HTMLButtonElement} tab - The tab + * @param {string} text - What it says + * @param {string} color - Swatch colour, or '' for no swatch + * @param {string} glyph - The targeted feature's mark + * @returns {void} + */ +function writeTabFace(tab, text, color, glyph) { + tab.replaceChildren() + if (color) { + const swatch = document.createElement('span') + swatch.className = 'gram-frame-style-tab-swatch' + swatch.style.color = color + swatch.textContent = glyph + tab.appendChild(swatch) + } + const word = document.createElement('span') + word.textContent = text + tab.appendChild(word) +} diff --git a/src/components/SymbolPicker.js b/src/components/SymbolPicker.js index 5e079924..ab2562df 100644 --- a/src/components/SymbolPicker.js +++ b/src/components/SymbolPicker.js @@ -1,39 +1,38 @@ /** * Symbol selector for GramFrame overlays. * - * Renders a compact native drop-down of symbol glyphs. It heads the Symbol band - * of the style panel (see ColorPicker.js), below the colour slider, and its - * glyphs are tinted with the currently selected colour. + * A button showing the current symbol, its name and a caret; clicking it opens + * a popup holding the seven shipped symbols as a grid, plus the size trial. * - * When a marker or harmonic set is selected, changing the symbol restyles that - * feature in place (feature 161); otherwise the chosen symbol is written to - * `state.selectedSymbol` and applied to the next created feature. The default - * option is `cross` — the symbol-less style. + * It replaced a native `