diff --git a/BACKLOG.md b/BACKLOG.md index 2356517..f33e3e1 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -220,6 +220,12 @@ Two items jump the queue by owner decision. Full reasoning in `decisions.md`; th - [ ] **N38 The current line's box keeps its pixels while its contents scale, so a large interface loses a control off the bottom (filed 2026-09-06).** The top block opens at a pixel count and an interface size change deliberately does not move it: `interface-scale.spec.js`'s "leaves the three panels at the proportions the sashes were left at when the size changes" asserts `bigger.block === left.block`, and its reason is the waveform, whose measurements are in device pixels because a peak bucket is one millisecond. Everything inside the panel is in rem and does scale. So at 150 per cent in the narrowest window the panel is 1.5 times fuller in a box that has not moved, and the text box needs the panel's scroll to be reached. It has been paid for three times by raising the block's opening height, from 13.5rem to 21rem as the panel gained the effect, the drawing order, the three margins, Next line and the style dropdown, and raising it cannot fix the 150 per cent case because the shortfall there grows with the size. The two candidates are scaling the stored heights with the interface size, which contradicts that criterion and needs the owner, and the reference's own answer, which is merging and splitting the panel's rows by width (edit-bar-tasks.md question 1). +- [ ] **N39 The font button of the current line's row three, and the two halves of the colour picker that are not built (filed 2026-09-06).** Row three names nine buttons before Next line: four style flags, a font, and four colours. The flags and the colours are built; the font is not, and it is the one that needs something this repository does not have, a list of the fonts installed on the machine. Beside it two halves of the colour picker are missing. The alpha: a colour is written with its transparency beside it, `\1a` to `\4a`, and only when that transparency changed, which is two tags in one undo step and `Edit::SetOverrideTag` writes one. The spectrum: the picker offers sixteen colours and a field that takes `#RRGGBB`, where a saturation square, a hue slider, the three number notations and a screen eyedropper belong. The alpha and the font want the same thing first, a planner that writes a list of tags as one step. + +- [ ] **N40 One check in `video-aspect.spec.js` fails on the CI runner about half the time, and its own evidence does not name the cause (filed 2026-09-06).** "says a media with no picture has none, and is as quiet about it as about no audio" opens a media with no video track and asserts that nothing alarming is on the status bar. On the CI runner it twice found "Open a video first." there: on the merge run of #99 at 16:29 and on #101 at 18:39, both on 2026-09-06, and it has never failed on this repository's own runner. What the check already collects rules out two of the three ways that sentence can appear: the app logged no `was refused as` line, so no command was refused through `refused()`, and the run counted zero `video://error` events. That leaves a rejected `invoke` inside `useVideoPlayer`, whose five commands each set `errorCode` from a rejection, and `VideoErrorCode::NotLoaded` is what `from_mpv` answers for `mpv_error::PropertyUnavailable`, which is what a video property is on a media that carries no video. Which of the five it is has not been proven and must not be guessed: the check has to name the node and the code it found before anything is changed, because a fix aimed at the wrong one of them would look like it worked. **A flaky check is a check that says nothing**, so this is a defect in the suite as much as in the app, and the first change is to the check. + - **2026-09-06, found by reading and fixed, not proven to be the cause.** Every one of those five setters wrote its answer unconditionally, so a command sent against the file that was open could still be in flight when the next file opened and then set the error state about a document nobody had asked about. `useVideoPlayer` now stamps each command with the open it belongs to and drops an answer carrying an older one. It is a real race and the fix is right on its own terms; whether it is what this check keeps catching is only knowable from CI, because it has never reproduced here. Two attempts to reproduce it by taking the audio output away, which is the one difference this machine and the runner are known to have, did not: mpv found an output both times and the spec passed. + - **A second check joined it on 2026-09-06:** `editor.spec.js`'s "scrolls a viewport at a time without falling behind" failed on the runner and has never failed here. Two timing-sensitive checks failing only on CI is a property of the runner as much as of either check, and it blocks every pull request, so it is the owner's to weigh: the suite cannot be trusted to be green there today. + - [ ] **N37 The scaled surface check misses the doubling by eight pixels, about one run in several (filed 2026-09-06).** `e2e/scripts/scaled-surface-check.js` asserts the video surface doubles when `GDK_SCALE` doubles, within three pixels. On CI job 101433553363 it read 346x166 at ratio 1 and 700x342 at ratio 2, which is eight and ten pixels over twice, and the same script printed `5/5 checks` on the run before it with nothing changed between them. The failure message reads as though the surface had not been resolved to native pixels, which is not what these numbers say: the scale is applied, and the layout under it settles a few pixels away from where it settles at ratio 1. The candidates are the ruler band, whose height is a rounded number of device pixels and therefore not exactly half at ratio 2, and the surface being measured before the last layout pass at a size where that pass takes longer. **Second sighting, 2026-09-06, and it says the difference is fixed rather than random.** Job 101441553639 on a build whose top block is taller read 346x230 at ratio 1 and 700x470 at ratio 2. The first sighting read 346x166 and 700x342. The heights differ between the two runs and **the shortfall does not**: eight pixels of width and ten of height over twice, both times. That is the shape of a fixed inset in device pixels that is applied once at each ratio instead of scaling with it, not of a layout that settles late, and it rules out the ruler band, whose height does change between those runs. The script still prints only the surface. The way to name it is to print the stage rectangle the page sends and the rectangle the backend applies, at both ratios, and see which of the two carries the eight and the ten. - [ ] **N36 A media with no picture sometimes says "Open a video first." to the translator, on the runner and not here (filed 2026-09-06).** `video-aspect.spec.js`'s "says a media with no picture has none, and is as quiet about it as about no audio" failed on CI job 101422670879 with `["Open a video first."]` where it expects no alert at all, at `video-aspect.spec.js:281`. That string is `video.errors.notLoaded`, which the status bar draws from `useVideoPlayer`'s error code, and the backend answers `NotLoaded` for mpv's `PropertyUnavailable` as well as for a genuinely closed player (`src-tauri/src/video/error.rs:81`). So a property read that lands in the window around an open is shown to a person as an instruction to do the thing they just did. This is the twin of the audio fix of 2026-09-05: a media with no video track is not an error and must be as quiet as a machine with no sound. The fix is to find which call answers it and stop that answer reaching the status bar, and the way to prove it is to force the answer and watch the check go red. diff --git a/crates/sublore-edit/src/plan.rs b/crates/sublore-edit/src/plan.rs index d607b4e..7fa4cbc 100644 --- a/crates/sublore-edit/src/plan.rs +++ b/crates/sublore-edit/src/plan.rs @@ -60,6 +60,16 @@ pub enum Edit { from: usize, to: usize, }, + /// Write one override tag with a value the caller chose, over the same stretch a style toggle + /// works on. The pickers use this where the four flags use `ToggleStyle`: a colour is picked + /// rather than flipped, so there is no state to read first. See edit-bar-tasks.md B12. + SetOverrideTag { + cue: usize, + tag: String, + value: String, + from: usize, + to: usize, + }, /// Turn an ASS event into a `Comment:` or back into a `Dialogue:`. The descriptor is not one /// of the fields `AssField` can name, and this changes how many cues a player would draw, so it /// is its own edit. See edit-bar-tasks.md B8. @@ -148,6 +158,13 @@ pub fn plan(document: &SubtitleDocument, edit: &Edit) -> Result plan_toggle_style(document, *cue, *flag, *from, *to), + Edit::SetOverrideTag { + cue, + tag, + value, + from, + to, + } => plan_set_override_tag(document, *cue, tag, value, *from, *to), Edit::Insert { before, start_ms, @@ -1105,6 +1122,84 @@ fn validate_field_value(field: AssField, value: &str) -> Result<(), EditError> { /// The flag's state at the caret, then the opposite of it written there, and the state it had put /// back at the far end of the selection shifted by whatever the first write inserted. That is the /// whole of it, and it is why the writer returns a shift. +/// The same write a style toggle makes, with the value given rather than worked out. A tag name +/// that is not a backslash and letters is refused: everything downstream reads a name that way, and +/// a value carrying a brace would close the block it was written into. +fn plan_set_override_tag( + document: &SubtitleDocument, + index: usize, + tag: &str, + value: &str, + from: usize, + to: usize, +) -> Result { + let named = tag.strip_prefix('\\').unwrap_or(""); + // One digit may lead, because the numbered colours and alphas are spelt `\\2c` and `\\1a`, and + // after it the name is letters to the end: whatever follows those is the value. + let letters = named + .strip_prefix(|first: char| first.is_ascii_digit()) + .unwrap_or(named); + if letters.is_empty() || !letters.bytes().all(|byte| byte.is_ascii_alphabetic()) { + return Err(EditError::new( + EditErrorKind::NotApplicable, + format!( + "{tag} is not a tag name: a name is a backslash, one digit at most, then letters" + ), + )); + } + if value.contains(['{', '}', '\\']) { + return Err(EditError::new( + EditErrorKind::NotApplicable, + "a tag value may not carry a brace or a backslash", + )); + } + let located = locate(document, index)?; + if !matches!(&located.cue.detail, CueDetail::Ass(_)) { + return Err(EditError::new( + EditErrorKind::NotApplicable, + "only an ASS event carries override tags", + )); + } + let text = document.slice(located.cue.text); + if from > text.len() + || to > text.len() + || !text.is_char_boundary(from) + || !text.is_char_boundary(to) + { + return Err(EditError::new( + EditErrorKind::NotApplicable, + format!("the range {from}..{to} is outside the cue's text or cuts a character"), + )); + } + let (start, _) = if from <= to { (from, to) } else { (to, from) }; + let (written, _) = override_tags::set_tag(text, start, tag, value); + + let write = plan_text_write(document, &located, &written)?; + Ok(Planned { + splice: Splice::new( + write.range.start, + document.slice(write.range).to_owned(), + write.inserted, + ), + label: EditLabel { + kind: EditKind::SetOverrideTag, + cue: index, + }, + expect: Expectation { + from: index, + removed: 1, + cues: vec![ExpectedCue { + text_raw: write.written, + start_ms: located.cue.start.millis(), + end_ms: located.cue.end.millis(), + }], + segments_from: located.segment_index, + segments_removed: 1, + segments_inserted: 1, + }, + }) +} + fn plan_toggle_style( document: &SubtitleDocument, index: usize, diff --git a/crates/sublore-edit/src/splice.rs b/crates/sublore-edit/src/splice.rs index d818af6..910b62d 100644 --- a/crates/sublore-edit/src/splice.rs +++ b/crates/sublore-edit/src/splice.rs @@ -69,6 +69,9 @@ pub enum EditKind { /// Which flag is on the label, for the reason `SetField` carries its field: bold and italic on /// one line must never merge into one undo step. ToggleStyle(sublore_formats::override_tags::StyleFlag), + /// A tag written with a value the caller chose. One kind rather than one per tag, unlike the + /// flag above: the picker commits on the pick, so every pick opens its own step. + SetOverrideTag, Insert, Delete, Split, diff --git a/crates/sublore-edit/tests/session.rs b/crates/sublore-edit/tests/session.rs index 4b21e9d..7c15b9e 100644 --- a/crates/sublore-edit/tests/session.rs +++ b/crates/sublore-edit/tests/session.rs @@ -986,6 +986,67 @@ fn a_field_committed_as_whitespace_writes_nothing_however_often_it_is_committed( ); } +fn set_override_tag(cue: usize, tag: &str, value: &str, from: usize, to: usize) -> Edit { + Edit::SetOverrideTag { + cue, + tag: tag.to_owned(), + value: value.to_owned(), + from, + to, + } +} + +#[test] +fn a_tag_with_a_chosen_value_is_written_where_the_caret_is() { + // B12: a colour is picked rather than flipped, so the value comes from the caller. + let mut session = session("ass/clean/basic.ass"); + let text = raw_text(&session, 0); + session + .apply( + &set_override_tag(0, "\\c", "&H0000FF&", 0, 0), + Run::New, + Instant::now(), + ) + .expect("an ASS event takes an override tag"); + assert_eq!(raw_text(&session, 0), format!("{{\\c&H0000FF&}}{text}")); + + session.undo().expect("a step to undo").expect("a patch"); + assert_eq!(raw_text(&session, 0), text); +} + +#[test] +fn a_numbered_colour_replaces_the_one_already_in_the_block_rather_than_joining_it() { + // B12: `\\2c` is one name, so a second pick of the same colour is not a second tag. + let mut session = session("ass/clean/basic.ass"); + let text = raw_text(&session, 0); + for value in ["&H0000FF&", "&H00FF00&"] { + session + .apply( + &set_override_tag(0, "\\2c", value, 0, 0), + Run::New, + Instant::now(), + ) + .expect("an ASS event takes a numbered colour"); + } + assert_eq!(raw_text(&session, 0), format!("{{\\2c&H00FF00&}}{text}")); +} + +#[test] +fn a_tag_name_that_is_not_a_name_and_a_value_that_could_close_a_block_are_both_refused() { + let mut session = session("ass/clean/basic.ass"); + let before = session.to_bytes(); + for (tag, value) in [("c", "&H0&"), ("\\1c1", "&H0&"), ("\\c", "&H0&}x{\\b1")] { + session + .apply( + &set_override_tag(0, tag, value, 0, 0), + Run::New, + Instant::now(), + ) + .expect_err("neither a bare name nor a value carrying a brace is written"); + } + assert_eq!(session.to_bytes(), before, "a refusal writes nothing"); +} + fn toggle_style(cue: usize, flag: StyleFlag, from: usize, to: usize) -> Edit { Edit::ToggleStyle { cue, diff --git a/crates/sublore-formats/src/override_tags.rs b/crates/sublore-formats/src/override_tags.rs index cc9ac5e..2f8c3f4 100644 --- a/crates/sublore-formats/src/override_tags.rs +++ b/crates/sublore-formats/src/override_tags.rs @@ -31,14 +31,21 @@ pub struct Block { pub span: Span, } -/// Whether a braced run holds a tag. A tag is a backslash followed by a letter, and a run with none -/// is a comment: `{note}` is a note and `{\b1}` is styling. +/// Whether a braced run holds a tag. A tag is a backslash and a name, and a run with none is a +/// comment: `{note}` is a note and `{\b1}` is styling. fn holds_a_tag(inside: &str) -> bool { let bytes = inside.as_bytes(); bytes .iter() .enumerate() - .any(|(at, byte)| *byte == b'\\' && bytes.get(at + 1).is_some_and(u8::is_ascii_alphabetic)) + .any(|(at, byte)| *byte == b'\\' && names_a_tag(bytes, at)) +} + +/// Whether the backslash at `at` opens a name: one digit at most, then at least one letter. The +/// digit is there because the numbered colours and alphas are spelt `\2c` and `\1a`. +fn names_a_tag(bytes: &[u8], at: usize) -> bool { + let letters = at + 1 + usize::from(bytes.get(at + 1).is_some_and(u8::is_ascii_digit)); + bytes.get(letters).is_some_and(u8::is_ascii_alphabetic) } /// The drawing scale a braced run leaves behind it: the last `\p` in it, or `None` when it @@ -185,13 +192,14 @@ pub fn tags_in(text: &str, block: Block) -> Vec { at += 1; continue; } - let mut after = at + 1; + let letters = at + 1 + usize::from(bytes.get(at + 1).is_some_and(u8::is_ascii_digit)); + let mut after = letters; while after < end && bytes[after].is_ascii_alphabetic() { after += 1; } // A backslash with no letter after it is not a tag: `\\N` is a line break and its letter is // taken by the name, which is right, and a trailing backslash names nothing. - if after == at + 1 { + if after == letters { at += 1; continue; } @@ -550,4 +558,23 @@ mod tests { ] ); } + + #[test] + fn a_numbered_colour_is_one_name_and_not_a_digit_before_a_value() { + let text = "{\\2c&H0000FF&}word"; + let parsed = blocks(text); + assert_eq!(parsed[0].kind, BlockKind::Override); + let found = tags_in(text, parsed[0]); + assert_eq!(found.len(), 1); + assert_eq!(&text[found[0].name.range()], "\\2c"); + assert_eq!(&text[found[0].value.range()], "&H0000FF&"); + } + + #[test] + fn a_backslash_and_a_digit_with_no_letter_after_it_names_nothing() { + let text = "{\\3}word"; + let parsed = blocks(text); + assert_eq!(parsed[0].kind, BlockKind::Comment); + assert!(tags_in(text, parsed[0]).is_empty()); + } } diff --git a/e2e/specs/command-registry.spec.js b/e2e/specs/command-registry.spec.js index 3721d5c..8d120b0 100644 --- a/e2e/specs/command-registry.spec.js +++ b/e2e/specs/command-registry.spec.js @@ -39,6 +39,8 @@ import { findToplevel } from "../lib/x11.js"; */ const DECLARED = [ "file-open-subtitle", + "file-open-source", + "file-close-source", "video-open", "file-save", "file-save-copy", @@ -103,9 +105,14 @@ const TITLES = [ { id: "help", label: "Help", disabled: false }, ]; -/** File with nothing open: Save, Save a copy and Discard are drawn, and all three are greyed. */ +/** + * File with nothing open: Save, Save a copy and Discard are drawn, and all three are greyed, and + * so are the two that read a second document beside one that is not there (M2.6 S1). + */ const FILE_ITEMS = [ { id: "file-open-subtitle", disabled: false }, + { id: "file-open-source", disabled: true }, + { id: "file-close-source", disabled: true }, { id: "video-open", disabled: false }, { id: "file-save", disabled: true }, { id: "file-save-copy", disabled: true }, @@ -501,6 +508,9 @@ describe("the command registry", () => { // seed the cursor onto row 0 (decision 5): insert, delete and merge all only need that, so they // ungrey too. Split stays gated behind a caret nothing has placed yet. expect(flips(empty, open)).toEqual([ + // A source is read beside a target, so the item that opens one wakes with the target (S1). + // Close source is not beside it: no source is open, so there is still nothing to close. + { route: "menu", id: "file-open-source", disabled: false }, { route: "menu", id: "file-save-copy", disabled: false }, // Find and Replace need a document and nothing else, so both ungrey with the file (F2, F3). // Find next is absent from this list on purpose: it also needs a pattern, and nothing here diff --git a/e2e/specs/current-line-bands.spec.js b/e2e/specs/current-line-bands.spec.js index f2fa962..780b26d 100644 --- a/e2e/specs/current-line-bands.spec.js +++ b/e2e/specs/current-line-bands.spec.js @@ -54,6 +54,7 @@ const CONTROLS = [ ".currentline__actor-open", ".currentline__start", ".currentline__end", + ".currentline__colour-primary", ".currentline__subtitle-next-line", ".currentline__text", ]; @@ -93,6 +94,10 @@ const BARE_SHORTFALL = { 90: [], 110: [], 150: [".currentline__text"] }; * block's opening height again would clear it and would take that height from the grid at every * size, for one control in one configuration out of six. See edit-bar-tasks.md question 1, which is * what actually closes this. + * + * The colour beside them at 110 per cent is that same row and not a new shortfall: the entries on + * either side of it are the first and the last control of the button row, so the row was already + * behind the scroll there before the colours were drawn into it. B12. */ const SHORTFALL = { 90: { floor: [".currentline__text"], wide: [] }, @@ -100,6 +105,7 @@ const SHORTFALL = { floor: [ ".currentline__text", ".currentline__edit-style-bold", + ".currentline__colour-primary", ".currentline__subtitle-next-line", ], wide: [], @@ -109,6 +115,7 @@ const SHORTFALL = { ".currentline__end", ".currentline__text", ".currentline__edit-style-bold", + ".currentline__colour-primary", ".currentline__subtitle-next-line", ], wide: [".currentline__text"], @@ -1237,6 +1244,145 @@ describe("the current line's bands", () => { }); }); + it("writes the colour picked at the caret, and takes it off in one undo", async () => { + const lineText = () => + browser.execute(() => document.querySelector(".currentline__text")?.value ?? null); + const caretBefore = (word) => + browser.execute((wanted) => { + const box = document.querySelector(".currentline__text"); + const at = box.value.indexOf(wanted); + box.focus(); + box.setSelectionRange(at, at); + box.dispatchEvent(new Event("select", { bubbles: true })); + return at; + }, word); + const buttons = () => + browser.execute(() => + Array.from(document.querySelectorAll(".currentline__colour")).map((button) => ({ + name: button.getAttribute("aria-label"), + disabled: button.disabled, + })), + ); + + // An override tag is an ASS thing, so on a format that carries none all four are drawn and + // greyed rather than absent, and greyed is what keeps a refusal from being reachable (24 A2). + await openSubtitle(toplevel, workingCopy("srt/clean/basic-lf.srt")); + await goToRow(toplevel, 1); + expect(await buttons()).toEqual([ + { name: "Primary colour", disabled: true }, + { name: "Secondary colour", disabled: true }, + { name: "Outline colour", disabled: true }, + { name: "Shadow colour", disabled: true }, + ]); + + const copy = workingCopy("ass/clean/speakers.ass"); + await openSubtitle(toplevel, copy); + await goToRow(toplevel, 1); + const before = await lineText(); + expect(before).toContain("harbour"); + + await caretBefore("harbour"); + await waitFor(async () => ((await buttons()).every((one) => !one.disabled) ? 1 : null), { + timeout: 15000, + message: "a caret in the box to ungrey the four colours", + }); + + await clickElement(toplevel, ".currentline__colour-outline"); + await waitFor(() => present(".currentline__picker"), { + timeout: 15000, + message: "the picker to open under the button", + }); + await clickElement(toplevel, '.currentline__swatch[aria-label="#FF0000"]'); + // ASS writes a colour blue first, so red is `&H0000FF&`, and the outline is the third one. + await waitFor(async () => ((await lineText())?.includes("{\\3c&H0000FF&}harbour") ? 1 : null), { + timeout: 15000, + message: "the outline colour to be written where the caret was", + }); + expect(await lineText()).toBe(before.replace("harbour", "{\\3c&H0000FF&}harbour")); + expect(await present(".currentline__picker")).toBe(false); + + await clickElement(toplevel, ".toolbar__edit-undo"); + await waitFor(async () => ((await lineText()) === before ? 1 : null), { + timeout: 15000, + message: "one undo to take the colour back off", + }); + }); + + it("takes a colour typed into the picker, and writes nothing while it is half typed", async () => { + const lineText = () => + browser.execute(() => document.querySelector(".currentline__text")?.value ?? null); + const field = () => + browser.execute(() => { + const box = document.querySelector(".currentline__hex"); + return box === null + ? null + : { value: box.value, invalid: box.getAttribute("aria-invalid") }; + }); + const typeHex = async (toplevel, typed) => { + await clickElement(toplevel, ".currentline__hex"); + await waitFor( + () => + browser.execute( + () => document.activeElement?.classList.contains("currentline__hex") === true, + ), + { timeout: 15000, message: "the picker's field to take the keyboard" }, + ); + pressKey("ctrl+a"); + typeText(typed); + await waitFor(async () => ((await field())?.value === typed ? 1 : null), { + timeout: 15000, + message: `the picker's field to hold exactly ${typed}`, + }); + }; + + const copy = workingCopy("ass/clean/speakers.ass"); + await openSubtitle(toplevel, copy); + await goToRow(toplevel, 1); + const before = await lineText(); + await browser.execute(() => { + const box = document.querySelector(".currentline__text"); + box.focus(); + box.setSelectionRange(0, 0); + box.dispatchEvent(new Event("select", { bubbles: true })); + }); + await waitFor( + async () => + (await browser.execute( + () => document.querySelector(".currentline__colour-primary")?.disabled === false, + )) + ? 1 + : null, + { timeout: 15000, message: "a caret at the start of the box" }, + ); + + await clickElement(toplevel, ".currentline__colour-primary"); + await waitFor(() => present(".currentline__hex"), { + timeout: 15000, + message: "the picker's own field", + }); + + // Four digits is not a colour, so Enter writes nothing and the field says which it is. + await typeHex(toplevel, "#12AB"); + expect((await field())?.invalid).toBe("true"); + pressKey("Return"); + expect(await lineText()).toBe(before); + expect(await present(".currentline__hex")).toBe(true); + + await typeHex(toplevel, "#12AB34"); + pressKey("Return"); + // `#12AB34` is red 12, green AB, blue 34, and ASS writes the three the other way round. + await waitFor(async () => ((await lineText()) === `{\\c&H34AB12&}${before}` ? 1 : null), { + timeout: 15000, + message: "the typed colour to be written at the caret", + }); + + await clickElement(toplevel, ".toolbar__edit-undo"); + await waitFor(async () => ((await lineText()) === before ? 1 : null), { + timeout: 15000, + message: "one undo to take the typed colour back off", + }); + }); + it("turns a line into a comment and back, in one undo step each way", async () => { const flag = () => browser.execute(() => { diff --git a/e2e/specs/source-column.spec.js b/e2e/specs/source-column.spec.js new file mode 100644 index 0000000..f2f7da2 --- /dev/null +++ b/e2e/specs/source-column.spec.js @@ -0,0 +1,287 @@ +/* global describe, it, before, document, window */ +/** + * M2.6 S1: the document being read from, beside the one being written. + * + * The criteria are in side-by-side-tasks.md S1 and they are all about what a translator sees: a + * column that appears with the source and goes with it, aligned by index and by nothing else, and + * a source file that is the same bytes afterwards as it was before. + */ +import { copyFileSync, existsSync, mkdirSync, readFileSync, rmSync } from "node:fs"; +import path from "node:path"; +import process from "node:process"; + +import { browser, expect } from "@wdio/globals"; + +import { answerChooser, waitForChooser } from "../lib/chooser.js"; +import { clickAt, focusWindow, pressKey, typeText } from "../lib/input.js"; +import { repoRoot, windowHeight, windowWidth } from "../lib/paths.js"; +import { waitFor } from "../lib/proc.js"; +import { findToplevel } from "../lib/x11.js"; + +/** The target: three cues, and the third is the row the source cannot reach. */ +const TARGET = ["srt", "clean", "basic-lf.srt"]; +/** The source: two cues, whose text shares no word with the target's. */ +const SOURCE = ["srt", "clean", "starts-at-zero-short.srt"]; +const SOURCE_LINES = ["Bring the nets in.", "The gulls know before we do, they always have.", ""]; +const TARGET_FIRST = "The harbour was empty when we got there."; +/** What the bar says about the source once it is open, in the shape it says the target in. */ +const SOURCE_STATUS = "Source: SRT · 2 cues · LF"; +/** Typed over the target's first line, to prove an edit never reaches the file being read. */ +const EDITED = "Written into the target"; + +function dataHome() { + const home = process.env.SUBLORE_E2E_DATA_HOME; + if (typeof home !== "string" || home === "") { + throw new Error("SUBLORE_E2E_DATA_HOME is not set; e2e/wdio.conf.js sets it for every run."); + } + return home; +} + +/** Writes go to the harness temp dir. The committed fixtures are copied, never opened directly. */ +function workingCopy(parts, name) { + const from = path.join(repoRoot, "fixtures", "subtitles", ...parts); + if (!existsSync(from)) { + throw new Error( + `E2E prerequisite missing: ${from} does not exist. It is committed; restore it with ` + + "`git checkout fixtures/subtitles`.", + ); + } + const directory = path.join(dataHome(), "source-column"); + mkdirSync(directory, { recursive: true }); + const copy = path.join(directory, name); + copyFileSync(from, copy); + return copy; +} + +function centreOf(selector) { + return browser.execute((css) => { + const element = document.querySelector(css); + if (element === null) { + return null; + } + const rect = element.getBoundingClientRect(); + const dpr = window.devicePixelRatio; + return { x: (rect.x + rect.width / 2) * dpr, y: (rect.y + rect.height / 2) * dpr }; + }, selector); +} + +async function clickElement(toplevel, selector) { + const centre = await centreOf(selector); + if (centre === null) { + throw new Error(`${selector} is missing from the DOM, so there is nothing to click`); + } + // No window manager under Xvfb, so the toplevel origin is also the viewport origin. + clickAt(toplevel.absX + centre.x, toplevel.absY + centre.y); +} + +function present(selector) { + return browser.execute((css) => document.querySelector(css) !== null, selector); +} + +function textOf(selector) { + return browser.execute((css) => document.querySelector(css)?.textContent ?? null, selector); +} + +/** The source cell of every rendered row, in list order, so alignment can be read in one trip. */ +function sourceColumn() { + return browser.execute(() => + Array.from(document.querySelectorAll(".cuelist__row")).map((row) => ({ + position: row.querySelector(".cuelist__pos")?.textContent ?? null, + target: row.querySelector(".cuelist__text")?.textContent ?? null, + source: row.querySelector(".cuelist__source")?.textContent ?? null, + })), + ); +} + +/** Open the File menu and choose one of its items by command id. */ +async function fromFileMenu(toplevel, token) { + await clickElement(toplevel, ".menubar__title--file"); + await waitFor(() => present(`.menubar__item--${token}`), { + timeout: 15000, + message: `the File menu to open on ${token}`, + }); + await clickElement(toplevel, `.menubar__item--${token}`); +} + +/** Whether a File menu item is drawn and whether it is greyed, without choosing it. */ +async function fileItem(toplevel, token) { + await clickElement(toplevel, ".menubar__title--file"); + await waitFor(() => present(`.menubar__item--${token}`), { + timeout: 15000, + message: `the File menu to open on ${token}`, + }); + const state = await browser.execute((css) => { + const item = document.querySelector(css); + return item === null ? null : { drawn: true, disabled: item.disabled === true }; + }, `.menubar__item--${token}`); + // Close it again the way Escape does, so the next gesture is not answered by the open menu. + pressKey("Escape"); + await waitFor(async () => ((await present(`.menubar__item--${token}`)) ? null : 1), { + timeout: 15000, + message: "the File menu to close", + }); + return state; +} + +async function openTarget(toplevel, file) { + await clickElement(toplevel, ".toolbar__file-open-subtitle"); + const chooser = await waitForChooser("Choose a subtitle"); + await answerChooser(chooser, file, "subtitle"); + focusWindow(toplevel.id); +} + +async function openSource(toplevel, file) { + await fromFileMenu(toplevel, "file-open-source"); + const chooser = await waitForChooser("Choose a subtitle"); + await answerChooser(chooser, file, "subtitle"); + focusWindow(toplevel.id); +} + +describe("the document being read from", () => { + let toplevel = null; + let target = null; + let source = null; + let sourceBytes = null; + + before(async () => { + rmSync(path.join(dataHome(), "source-column"), { recursive: true, force: true }); + target = workingCopy(TARGET, "target.srt"); + source = workingCopy(SOURCE, "source.srt"); + sourceBytes = readFileSync(source); + toplevel = await waitFor(findToplevel, { + timeout: 30000, + message: `the ${windowWidth}x${windowHeight} "Sublore" toplevel to appear`, + }); + focusWindow(toplevel.id); + await waitFor(() => present(".toolbar__file-open-subtitle"), { + timeout: 30000, + message: "the app UI to render", + }); + }); + + it("draws no column until a source is open, and greys the two items that need one", async () => { + // Nothing open at all: a source is read beside a target, so there is nothing to read it beside. + expect(await fileItem(toplevel, "file-open-source")).toEqual({ drawn: true, disabled: true }); + expect(await fileItem(toplevel, "file-close-source")).toEqual({ drawn: true, disabled: true }); + + await openTarget(toplevel, target); + await waitFor( + async () => ((await textOf(".statusbar__document"))?.includes("3 cues") === true ? 1 : null), + { timeout: 20000, message: "the target to open" }, + ); + expect(await present(".cuelist__headcell--source")).toBe(false); + expect(await present(".cuelist__source")).toBe(false); + + // Now there is a target to read beside, and still nothing to close. + expect(await fileItem(toplevel, "file-open-source")).toEqual({ drawn: true, disabled: false }); + expect(await fileItem(toplevel, "file-close-source")).toEqual({ drawn: true, disabled: true }); + }); + + it("draws the source beside the target, row for row, and blank past its last line", async () => { + await openSource(toplevel, source); + await waitFor(() => present(".cuelist__headcell--source"), { + timeout: 20000, + message: "the source column to appear", + }); + + const rows = await waitFor( + async () => { + const drawn = await sourceColumn(); + return drawn.length === 3 ? drawn : null; + }, + { timeout: 15000, message: "three rows to be drawn" }, + ); + expect(rows.map((row) => row.position)).toEqual(["1", "2", "3"]); + expect(rows.map((row) => row.source)).toEqual(SOURCE_LINES); + // The target's own column is untouched by any of it. + expect(rows[0].target).toBe(TARGET_FIRST); + + // And the bar says which document is being read, in the shape it says the target in. + await waitFor( + async () => ((await textOf(".statusbar__document"))?.includes(SOURCE_STATUS) ? 1 : null), + { timeout: 15000, message: "the status bar to name the source" }, + ); + }); + + it("never writes the file it is reading, whatever is done to the one being written", async () => { + await clickElement(toplevel, ".currentline__text"); + await waitFor( + () => + browser.execute( + () => document.activeElement?.classList.contains("currentline__text") === true, + ), + { timeout: 15000, message: "the current line's box to take the keyboard" }, + ); + pressKey("ctrl+a"); + typeText(EDITED); + await waitFor( + async () => + (await browser.execute( + () => document.querySelector(".currentline__text")?.value ?? null, + )) === EDITED + ? 1 + : null, + { timeout: 15000, message: `the current line's box to hold exactly ${EDITED}` }, + ); + await clickElement(toplevel, ".toolbar__file-save"); + await waitFor( + async () => ((await textOf(".statusbar__message"))?.includes("aved") === true ? 1 : null), + { timeout: 20000, message: "the target to be saved" }, + ); + + // The target took the edit; the source is the bytes it was before any of this. + expect(readFileSync(target).toString()).toContain(EDITED); + expect(readFileSync(source).equals(sourceBytes)).toBe(true); + }); + + it("takes the column away when the source is closed, and leaves the target where it was", async () => { + expect(await fileItem(toplevel, "file-close-source")).toEqual({ drawn: true, disabled: false }); + await fromFileMenu(toplevel, "file-close-source"); + await waitFor(async () => ((await present(".cuelist__headcell--source")) ? null : 1), { + timeout: 15000, + message: "the source column to go with the source", + }); + expect(await present(".cuelist__source")).toBe(false); + + // The target is still the document on screen, with the line that was written into it. + const rows = await sourceColumn(); + expect(rows[0].target).toBe(EDITED); + expect(await textOf(".statusbar__document")).not.toContain("Source:"); + expect(readFileSync(source).equals(sourceBytes)).toBe(true); + }); + + it("refuses a source it cannot read, and leaves the column as it found it", async () => { + await openSource(toplevel, source); + await waitFor(() => present(".cuelist__headcell--source"), { + timeout: 20000, + message: "the source column to come back", + }); + + const broken = path.join( + repoRoot, + "fixtures", + "subtitles", + "srt", + "malformed", + "missing-arrow.srt", + ); + await openSource(toplevel, broken); + const message = await waitFor( + async () => { + const said = await textOf(".statusbar__error"); + return said !== null && said.trim() !== "" ? said : null; + }, + { timeout: 20000, message: "the refusal to be said on the status bar" }, + ); + expect(message.trim()).not.toBe(""); + + // A refused open leaves no half-read document behind, so the column goes rather than lying. + await waitFor(async () => ((await present(".cuelist__headcell--source")) ? null : 1), { + timeout: 15000, + message: "the column to go with the refused source", + }); + // The target is untouched by a source that never opened. + const rows = await sourceColumn(); + expect(rows[0].target).toBe(EDITED); + }); +}); diff --git a/e2e/specs/video-aspect.spec.js b/e2e/specs/video-aspect.spec.js index b373624..ab036aa 100644 --- a/e2e/specs/video-aspect.spec.js +++ b/e2e/specs/video-aspect.spec.js @@ -275,8 +275,13 @@ describe("how wide a box the picture would fill", () => { expect(countInLog(NO_PICTURE)).toBe(said + 1); expect(countInLog(A_SIZE)).toBe(sized); + // Each alert with the class it carries, so a failure names which line said it rather than + // leaving the reader to work it out from the sentence. See BACKLOG.md N40. const alarming = await browser.execute(() => - Array.from(document.querySelectorAll('[role="alert"]')).map((node) => node.textContent), + Array.from(document.querySelectorAll('[role="alert"]')).map((node) => ({ + said: node.textContent, + drawnAs: node.className, + })), ); // A sentence on screen and the refusals the app logged, read as one object: a refusal is what // puts a sentence there, and the log is the only place that says which command was refused. diff --git a/e2e/wdio.conf.js b/e2e/wdio.conf.js index 0306288..e17c573 100644 --- a/e2e/wdio.conf.js +++ b/e2e/wdio.conf.js @@ -13,7 +13,7 @@ import { passedTests, recordPassedTest, resetTally } from "./lib/tally.js"; * Every spec that exists must run. WebdriverIO does not reliably fail a run that executed nothing, * so the count is asserted here. Bump it when you add a test; see e2e/README.md. */ -const EXPECTED_TESTS = 276; +const EXPECTED_TESTS = 283; // Keeps a run out of the real data dir. Created once in the launcher; workers inherit the value. process.env.SUBLORE_E2E_DATA_HOME ??= mkdtempSync(path.join(os.tmpdir(), "sublore-e2e-")); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 3311890..b022b17 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -211,6 +211,9 @@ pub fn run() -> tauri::Result<()> { subtitle::subtitle_set_field, subtitle::subtitle_set_comment, subtitle::subtitle_toggle_style, + subtitle::subtitle_set_override_tag, + subtitle::subtitle_open_source, + subtitle::subtitle_close_source, subtitle::subtitle_set_times, subtitle::subtitle_insert, subtitle::subtitle_delete, diff --git a/src-tauri/src/subtitle/mod.rs b/src-tauri/src/subtitle/mod.rs index 4877a1c..7a7ace6 100644 --- a/src-tauri/src/subtitle/mod.rs +++ b/src-tauri/src/subtitle/mod.rs @@ -37,18 +37,29 @@ const BACKUP_DIR: &str = "backups"; /// so the guard is never held across an await. pub type SessionSlot = Mutex>; +/// The two documents a translator has open: the one being written, and the one being read from. +/// +/// They are two slots and not one with a flag, so that no edit can reach the source by taking the +/// wrong branch: every mutating command asks for `slot()` and there is no command anywhere that +/// asks for `source_slot()` and then writes. See side-by-side-tasks.md S1. #[derive(Default)] pub struct SubtitleState { session: Arc, + source: Arc, } impl SubtitleState { /// A handle the blocking half of a command can own, as `VideoState` hands out its player. // TODO(M2.6): narrow back to private. Public only so the close gate in `lib.rs` can read the - // session; M2.6 reshapes this signature for two documents anyway (owner ruling 2026-08-29). + // session. pub fn slot(&self) -> Arc { Arc::clone(&self.session) } + + /// The document being read from. Opened and closed and never written to. + fn source_slot(&self) -> Arc { + Arc::clone(&self.source) + } } #[derive(Clone, Debug, Serialize)] @@ -207,6 +218,26 @@ pub async fn subtitle_open( opened } +/// Open the document to read from, beside the one being written. It is never edited and never +/// saved, so it has no dirty state to guard and replacing it loses nothing. See S1. +#[tauri::command] +pub async fn subtitle_open_source( + state: State<'_, SubtitleState>, + path: String, +) -> Result { + let slot = state.source_slot(); + blocking(move || open_session(&slot, &path)).await +} + +/// Close the document being read from. The frame draws the target, so nothing on screen moves with +/// it, and the target is left exactly as it was. +#[tauri::command] +pub async fn subtitle_close_source(state: State<'_, SubtitleState>) -> Result<(), SubtitleError> { + let slot = state.source_slot(); + // Discarding is free here and not a choice made for the user: nothing ever wrote to it. + blocking(move || close_session(&slot, true)).await +} + #[tauri::command] pub async fn subtitle_close( app: AppHandle, @@ -383,6 +414,49 @@ pub async fn subtitle_toggle_style( .await } +/// One override tag as a caller names it: the tag, the value chosen for it, and the stretch of the +/// line it is written over. One value rather than four more arguments, which is the shape H5 +/// settled on when `clippy::too_many_arguments` was right about `module_invoke`. +#[derive(Debug, Deserialize)] +pub struct OverrideTagWrite { + pub tag: String, + pub value: String, + pub from: usize, + pub to: usize, +} + +/// One override tag with a value the caller chose, over the same stretch a style toggle works on. +/// The name and the value are checked by the planner, which refuses anything that could close the +/// block it is written into. See edit-bar-tasks.md B12. +#[tauri::command] +pub async fn subtitle_set_override_tag( + app: AppHandle, + state: State<'_, SubtitleState>, + revision: u64, + cue: usize, + write: OverrideTagWrite, +) -> Result { + let OverrideTagWrite { + tag, + value, + from, + to, + } = write; + edited( + &app, + state.slot(), + revision, + Edit::SetOverrideTag { + cue, + tag, + value, + from, + to, + }, + ) + .await +} + /// Whether one cue is a line a player draws. Refused on a format that has no descriptor to /// rewrite, and the panel draws that control greyed instead of asking. See edit-bar-tasks.md B8. #[tauri::command] diff --git a/src/App.tsx b/src/App.tsx index c152602..bdf283e 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -29,6 +29,7 @@ import { useModulePanels } from "./hooks/useModulePanels"; import { useModuleWork } from "./hooks/useModuleWork"; import { useModules, refusalLine } from "./hooks/useModules"; import { useSearch, type SearchOutcome } from "./hooks/useSearch"; +import { useSourceFile } from "./hooks/useSourceFile"; import { useProject } from "./hooks/useProject"; import { useStartupFiles } from "./hooks/useStartupFiles"; import { useSubtitleFile, type RowsMoved } from "./hooks/useSubtitleFile"; @@ -196,6 +197,9 @@ export default function App() { // Every HTML layer registers here while it is open, and the video surface hides for as long as // the set is not empty (decision 1, T8). const layers = useLayerRegistry(); + // The document being read from while translating, held apart from the one being written so that + // no edit can reach it. See side-by-side-tasks.md S1. + const source = useSourceFile(); // The user's own expression never runs on this thread: it runs where it can be killed (F4a). const search = useSearch(); // Read once at startup; the scan itself ran before this window existed (module-abi.md 3.5). @@ -436,6 +440,15 @@ export default function App() { * leaves it unmatched and the split greyed. */ const [caret, setCaret] = useState<{ index: number; offset: number; to: number } | null>(null); + /** + * Whether an override tag can be written now. Only ASS carries them, and one is written at a + * caret in the line's own editor, so it wants one on the row the cursor is on. + */ + const writesAtCaret = + subtitle.summary?.format === "ass" && + activeCue !== null && + caret !== null && + caret.index === selection.active; // The chooser is modal and answers on its own thread, so a second one asked for while it is up // would sit behind the first. Every chooser the chrome raises is raised here, so one flag covers // them all. @@ -867,6 +880,19 @@ export default function App() { enabled: !choosing, run: () => void pick("subtitle", undefined, (path) => void subtitle.open(path)), }, + { + id: "file.open-source", + label: en.menu.file.openSource, + // A source is read beside a target, so there has to be a target to read it beside. + enabled: !choosing && subtitle.summary !== null, + run: () => void pick("subtitle", undefined, (path) => void source.open(path)), + }, + { + id: "file.close-source", + label: en.menu.file.closeSource, + enabled: source.summary !== null, + run: () => void source.close(), + }, { id: "video.open", label: en.menu.file.openVideo, @@ -1083,8 +1109,7 @@ export default function App() { ...STYLE_FLAGS.map(({ id, flag, label }): Command => ({ id, label, - // A caret in the line's own editor is what it writes at, so it wants one on this row. - enabled: activeCue !== null && caret !== null && caret.index === selection.active, + enabled: writesAtCaret, run: () => { if (caret !== null && selection.active !== null) { void subtitle.toggleStyle(selection.active, flag, caret.offset, caret.to); @@ -1242,6 +1267,8 @@ export default function App() { title: en.menu.file.title, items: [ "file.open-subtitle", + "file.open-source", + "file.close-source", "video.open", "file.save", "file.save-copy", @@ -1538,6 +1565,20 @@ export default function App() { styles={subtitle.summary?.styles.map((style) => style.name) ?? []} canComment={subtitle.summary?.format === "ass"} onCommitComment={(cue, comment) => subtitle.setComment(cue, comment)} + canWriteTag={writesAtCaret} + onSetOverrideTag={async (tag, value) => { + // The same rule the button greys on, read again here: a greyed command must not + // run, and a picker left open on a row the cursor has left must not write to it. + if (writesAtCaret && caret !== null && selection.active !== null) { + await subtitle.setOverrideTag( + selection.active, + tag, + value, + caret.offset, + caret.to, + ); + } + }} /> @@ -1561,6 +1602,7 @@ export default function App() { ({ style: cues.some((cue) => cue.style !== ""), actor: cues.some((cue) => cue.actor !== ""), + // The source is a document and not a field, so an open one with nothing on this row still + // draws its column: what is missing there is a row the source does not reach. + source: sourceCues.length > 0, }), - [cues], + [cues, sourceCues], ); useEffect(() => { @@ -308,6 +317,11 @@ export default function CueList({ {en.subtitle.cueList.text} + {columns.source && ( + + {en.subtitle.cueList.source} + + )}
)} + {columns.source && ( + {sourceCues[index]?.text ?? ""} + )}
); })} diff --git a/src/components/CurrentLine.tsx b/src/components/CurrentLine.tsx index e9d54e9..d2f5175 100644 --- a/src/components/CurrentLine.tsx +++ b/src/components/CurrentLine.tsx @@ -50,8 +50,72 @@ type CurrentLineProps = { /** Whether the format has a descriptor at all: only ASS has one, so only ASS can be commented. */ canComment: boolean; onCommitComment: (cue: number, comment: boolean) => Promise; + /** + * One override tag written where the caret is. The shell binds the row and the caret, so the + * panel names only the tag and the value it chose. See edit-bar-tasks.md B12. + */ + onSetOverrideTag: (tag: string, value: string) => Promise; + /** Whether there is a caret on this row to write at. Without one the colour buttons grey. */ + canWriteTag: boolean; +}; + +/** The four colours a line can override, in the order row three of the reference draws them. */ +type ColourSlot = "primary" | "secondary" | "outline" | "shadow"; + +const COLOUR_SLOTS: ColourSlot[] = ["primary", "secondary", "outline", "shadow"]; + +/** The four style commands and the letter each is drawn as, in row three's order. */ +const STYLE_GLYPHS: { id: CommandId; glyph: string }[] = [ + { id: "edit.style-bold", glyph: "B" }, + { id: "edit.style-italic", glyph: "I" }, + { id: "edit.style-underline", glyph: "U" }, + { id: "edit.style-strikeout", glyph: "S" }, +]; + +/** + * The tag each of them writes. The first is `\\c` and not `\\1c` because that is the spelling the + * reference writes; a renderer reads the two as one colour. + */ +const COLOUR_TAGS: Record = { + primary: "\\c", + secondary: "\\2c", + outline: "\\3c", + shadow: "\\4c", }; +/** What the picker offers without typing: the sixteen a subtitle is actually coloured with. */ +const PALETTE = [ + "#FFFFFF", + "#C0C0C0", + "#808080", + "#000000", + "#FF0000", + "#800000", + "#FFFF00", + "#808000", + "#00FF00", + "#008000", + "#00FFFF", + "#008080", + "#0000FF", + "#000080", + "#FF00FF", + "#800080", +]; + +/** + * ASS writes a colour blue first, so `#RRGGBB` is written `&HBBGGRR&`. Null when the text is not + * six hexadecimal digits, which is what keeps a half-typed value out of the line. + */ +function assColour(hex: string): string | null { + const found = /^#?([0-9a-fA-F]{6})$/.exec(hex.trim()); + if (found === null) { + return null; + } + const digits = found[1].toUpperCase(); + return `&H${digits.slice(4, 6)}${digits.slice(2, 4)}${digits.slice(0, 2)}&`; +} + /** The ASS fields the panel holds as a number: the drawing order and the three margins. */ type NumberField = "layer" | "marginL" | "marginR" | "marginV"; @@ -157,6 +221,8 @@ export default function CurrentLine({ cues, onCommitField, commands, + onSetOverrideTag, + canWriteTag, styles, canComment, onCommitComment, @@ -203,6 +269,15 @@ export default function CurrentLine({ width: number; } | null>(null); const [highlight, setHighlight] = useState(0); + /** Which colour the picker is open on and where it is drawn. Null while it is closed. */ + const [colourAt, setColourAt] = useState<{ + slot: ColourSlot; + left: number; + top: number; + } | null>(null); + /** What the picker's own field holds, kept between openings so a colour is typed once. */ + const [hex, setHex] = useState(PALETTE[0]); + const pickerRef = useRef(null); const comboRefs = useRef>>({}); const values = useMemo( () => ({ actor: fieldValues(cues, "actor"), effect: fieldValues(cues, "effect") }), @@ -330,6 +405,40 @@ export default function CurrentLine({ }; }, [listAt]); + // The picker is anchored to a button on one row, so the cursor leaving that row closes it. + useEffect(() => { + setColourAt(null); + }, [index]); + + // Drawn at coordinates taken when it opened, so it closes rather than hanging over the panel it + // no longer belongs to. A press anywhere but inside it, or on the button that opened it, closes + // it too: the button's own click is what reopens it. + useEffect(() => { + if (colourAt === null) { + return; + } + const close = () => setColourAt(null); + const away = (event: PointerEvent) => { + const target = event.target; + if (!(target instanceof Node)) { + close(); + return; + } + const onOpener = target instanceof Element && target.closest(".currentline__colour") !== null; + if (pickerRef.current?.contains(target) !== true && !onOpener) { + close(); + } + }; + window.addEventListener("resize", close); + window.addEventListener("scroll", close, true); + window.addEventListener("pointerdown", away, true); + return () => { + window.removeEventListener("resize", close); + window.removeEventListener("scroll", close, true); + window.removeEventListener("pointerdown", away, true); + }; + }, [colourAt]); + const timesEdited = times.start !== timecode(startMs) || times.end !== timecode(endMs) || @@ -691,6 +800,67 @@ export default function CurrentLine({ ); } + /** Write one colour where the caret is, and close. A value short of six digits writes nothing. */ + async function pickColour(slot: ColourSlot, value: string) { + const written = assColour(value); + if (written === null) { + return; + } + const trimmed = value.trim().toUpperCase(); + setHex(trimmed.startsWith("#") ? trimmed : `#${trimmed}`); + setColourAt(null); + await onSetOverrideTag(COLOUR_TAGS[slot], written); + } + + /** One colour, drawn as the button that opens the picker over it. */ + function colourButton(slot: ColourSlot) { + const open = colourAt !== null && colourAt.slot === slot; + return ( + + ); + } + + /** + * One of the four style commands, drawn the way row three of the reference draws it: a letter in + * the style it writes, not the word. The command's own label stays as the button's spoken name. + */ + function styleButton(id: CommandId, glyph: string) { + const command = commands[id]; + if (command === undefined) { + return null; + } + return ( + + ); + } + /** One command from the registry, drawn as a button that greys and runs by the registry's rule. */ function commandButton(id: CommandId) { const command = commands[id]; @@ -807,10 +977,12 @@ export default function CurrentLine({ {/* Band 3, the commands the panel carries. Row three of the reference puts the style buttons first and Next line last, so it goes at the end and the others arrive before it. */}
- {commandButton("edit.style-bold")} - {commandButton("edit.style-italic")} - {commandButton("edit.style-underline")} - {commandButton("edit.style-strikeout")} + {/* Two groups of four, the way the reference draws them: buttons touching inside a group, + with the band's own gap between the groups rather than between the buttons. */} + + {STYLE_GLYPHS.map(({ id, glyph }) => styleButton(id, glyph))} + + {COLOUR_SLOTS.map((slot) => colourButton(slot))} {commandButton("subtitle.next-line")}