From fadda49ac6c9dd17e8a2291123d3f848b87e0978 Mon Sep 17 00:00:00 2001 From: Alcahest Date: Mon, 7 Sep 2026 00:59:57 +0200 Subject: [PATCH] feat: choose how see-through a colour is, beside the colour --- BACKLOG.md | 2 +- crates/sublore-edit/src/plan.rs | 73 ---------------------------- crates/sublore-edit/tests/session.rs | 18 +++---- e2e/specs/current-line-bands.spec.js | 25 ++++++++-- src-tauri/src/lib.rs | 1 - src-tauri/src/subtitle/mod.rs | 43 ---------------- src/App.tsx | 15 +----- src/components/CurrentLine.tsx | 71 ++++++++++++++++++++++++--- src/hooks/useSubtitleFile.ts | 15 ------ src/i18n/en.ts | 6 +++ 10 files changed, 101 insertions(+), 168 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 51102ed..77fada6 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -220,7 +220,7 @@ 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 two halves of the colour picker that are not built (filed 2026-09-06; the font half closed 2026-09-07).** The font button is in, over a font enumeration that reads the `name` table out of every installed font file, and over a planner that writes several tags as one step, which is what this entry said both halves wanted. What is left is the colour picker's own two: the alpha, `\1a` to `\4a`, written beside the colour and only when it changed, which the new planner can now carry; and the spectrum, where the picker offers sixteen colours and a field that takes `#RRGGBB` and the reference offers a saturation square, a hue slider, the three number notations and a screen eyedropper. What it said when it was filed: 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. +- [ ] **N39 The spectrum the colour picker does not have (filed 2026-09-06; the font half closed 2026-09-07, the alpha half the same day).** The font button is in, over a font enumeration that reads the `name` table out of every installed font file, and over a planner that writes several tags as one step, which is what this entry said both halves wanted. The transparency went in on that same planner: the picker takes a number from 0 to 255 and writes `\1a` to `\4a` beside the colour, as one undo step, and an empty field writes none at all, which is this product's answer to the reference writing it only when it changed. What is left is the spectrum, where the picker offers sixteen colours and a field that takes `#RRGGBB` and the reference offers a saturation square, a hue slider, the three number notations and a screen eyedropper. What it said when it was filed: 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 (the same defect as N36, filed twice by mistake on 2026-09-06; keep this entry, which carries what was learned) 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. diff --git a/crates/sublore-edit/src/plan.rs b/crates/sublore-edit/src/plan.rs index 57bb8b2..bd86121 100644 --- a/crates/sublore-edit/src/plan.rs +++ b/crates/sublore-edit/src/plan.rs @@ -74,16 +74,6 @@ 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. @@ -172,13 +162,6 @@ 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::ClearText { cue, keep_tags } => plan_clear_text(document, *cue, *keep_tags), Edit::SetOverrideTags { cue, tags, at } => { plan_set_override_tags(document, *cue, tags, *at) @@ -1288,62 +1271,6 @@ fn plan_clear_text( }) } -fn plan_set_override_tag( - document: &SubtitleDocument, - index: usize, - tag: &str, - value: &str, - from: usize, - to: usize, -) -> Result { - check_tag(tag, value)?; - 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/tests/session.rs b/crates/sublore-edit/tests/session.rs index 6a94286..cc08781 100644 --- a/crates/sublore-edit/tests/session.rs +++ b/crates/sublore-edit/tests/session.rs @@ -986,13 +986,13 @@ 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 { +/// One tag written at a caret, which is the list of one the pickers send when nothing else goes +/// with it: a colour with no transparency beside it, or a style flag's own value. +fn set_override_tag(cue: usize, tag: &str, value: &str, at: usize) -> Edit { + Edit::SetOverrideTags { cue, - tag: tag.to_owned(), - value: value.to_owned(), - from, - to, + tags: vec![(tag.to_owned(), value.to_owned())], + at, } } @@ -1003,7 +1003,7 @@ fn a_tag_with_a_chosen_value_is_written_where_the_caret_is() { let text = raw_text(&session, 0); session .apply( - &set_override_tag(0, "\\c", "&H0000FF&", 0, 0), + &set_override_tag(0, "\\c", "&H0000FF&", 0), Run::New, Instant::now(), ) @@ -1066,7 +1066,7 @@ fn a_numbered_colour_replaces_the_one_already_in_the_block_rather_than_joining_i for value in ["&H0000FF&", "&H00FF00&"] { session .apply( - &set_override_tag(0, "\\2c", value, 0, 0), + &set_override_tag(0, "\\2c", value, 0), Run::New, Instant::now(), ) @@ -1167,7 +1167,7 @@ fn a_tag_name_that_is_not_a_name_and_a_value_that_could_close_a_block_are_both_r for (tag, value) in [("c", "&H0&"), ("\\1c1", "&H0&"), ("\\c", "&H0&}x{\\b1")] { session .apply( - &set_override_tag(0, tag, value, 0, 0), + &set_override_tag(0, tag, value, 0), Run::New, Instant::now(), ) diff --git a/e2e/specs/current-line-bands.spec.js b/e2e/specs/current-line-bands.spec.js index f3f8c08..1e37ac1 100644 --- a/e2e/specs/current-line-bands.spec.js +++ b/e2e/specs/current-line-bands.spec.js @@ -1373,17 +1373,32 @@ describe("the current line's bands", () => { expect(await present(".currentline__hex")).toBe(true); await typeHex(toplevel, "#12AB34"); + // A transparency beside it, which ASS counts the other way from opacity: 128 is half see + // through and it is written in hexadecimal. The two go in as one step. See B12. + await clickElement(toplevel, ".currentline__alpha"); + pressKey("ctrl+a"); + typeText("128"); + await waitFor( + async () => + (await browser.execute( + () => document.querySelector(".currentline__alpha")?.value ?? null, + )) === "128" + ? 1 + : null, + { timeout: 15000, message: "the transparency field to hold exactly 128" }, + ); 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 waitFor( + async () => ((await lineText()) === `{\\c&H34AB12&\\1a&H80&}${before}` ? 1 : null), + { timeout: 15000, message: "the typed colour and its transparency written at the caret" }, + ); + // One step, not two: a colour and how see-through it is are one thing a translator chose. 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", + message: "one undo to take the colour and its transparency back off together", }); }); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index d1ecdbd..c545b56 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -213,7 +213,6 @@ pub fn run() -> tauri::Result<()> { subtitle::subtitle_set_field, subtitle::subtitle_set_comment, subtitle::subtitle_toggle_style, - subtitle::subtitle_set_override_tag, subtitle::subtitle_set_override_tags, subtitle::subtitle_clear_text, fonts::fonts_installed, diff --git a/src-tauri/src/subtitle/mod.rs b/src-tauri/src/subtitle/mod.rs index fb95568..27b8eed 100644 --- a/src-tauri/src/subtitle/mod.rs +++ b/src-tauri/src/subtitle/mod.rs @@ -446,17 +446,6 @@ 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, -} - /// Empty one cue's text. `keep_tags` is the reference's Clear Text: the braced runs stay where /// they are and only the words go. See edit-bar-tasks.md B13. #[tauri::command] @@ -503,38 +492,6 @@ pub async fn subtitle_set_override_tags( .await } -/// 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 5c1fbac..78482c3 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1676,23 +1676,12 @@ export default function App() { fontsLoading={fonts.loading} onLoadFonts={fonts.load} onSetOverrideTags={async (tags, at) => { + // The same rule the buttons grey on, read again here: a greyed control must not + // run, and a picker left open on a row the cursor has left must not write to it. if (writesAtCaret && selection.active !== null) { await subtitle.setOverrideTags(selection.active, tags, at); } }} - 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, - ); - } - }} /> diff --git a/src/components/CurrentLine.tsx b/src/components/CurrentLine.tsx index 05f68bb..722c059 100644 --- a/src/components/CurrentLine.tsx +++ b/src/components/CurrentLine.tsx @@ -50,11 +50,6 @@ 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; /** Several override tags at one caret, as one step: a font is a family and a size. See B12. */ @@ -95,6 +90,29 @@ const COLOUR_TAGS: Record = { shadow: "\\4c", }; +/** + * The tag each one's transparency is written with. Numbered from one even where the colour is not: + * the primary colour is `\\c` and its transparency is `\\1a`, which is the format's own spelling. + */ +const ALPHA_TAGS: Record = { + primary: "\\1a", + secondary: "\\2a", + outline: "\\3a", + shadow: "\\4a", +}; + +/** + * ASS writes transparency and not opacity: `&H00&` is solid and `&HFF&` is invisible. Null when the + * text is not a whole number in range, which is what keeps a half-typed one out of the line. + */ +function assAlpha(typed: string): string | null { + const wanted = Number(typed.trim()); + if (!Number.isInteger(wanted) || wanted < 0 || wanted > 255) { + return null; + } + return `&H${wanted.toString(16).toUpperCase().padStart(2, "0")}&`; +} + /** What the picker offers without typing: the sixteen a subtitle is actually coloured with. */ const PALETTE = [ "#FFFFFF", @@ -233,7 +251,6 @@ export default function CurrentLine({ cues, onCommitField, commands, - onSetOverrideTag, canWriteTag, onSetOverrideTags, caretAt, @@ -294,6 +311,8 @@ export default function CurrentLine({ } | null>(null); /** What the picker's own field holds, kept between openings so a colour is typed once. */ const [hex, setHex] = useState(PALETTE[0]); + /** The transparency beside it. Empty on purpose: an empty field writes no transparency at all. */ + const [alpha, setAlpha] = useState(""); const pickerRef = useRef(null); /** Where the font picker is drawn, and null while it is closed. */ const [fontAt, setFontAt] = useState<{ left: number; top: number } | null>(null); @@ -851,16 +870,34 @@ export default function CurrentLine({ ); } - /** Write one colour where the caret is, and close. A value short of six digits writes nothing. */ + /** + * Write one colour where the caret is, with its transparency when one was typed, and close. + * + * A colour short of six digits writes nothing, and so does a transparency that is not a whole + * number between 0 and 255: a half-typed field must not reach the line. The two go together as + * one step, because choosing a colour is one thing a translator did. See B12. + */ async function pickColour(slot: ColourSlot, value: string) { const written = assColour(value); if (written === null) { return; } + const typed = alpha.trim(); + const transparency = typed === "" ? null : assAlpha(typed); + if (typed !== "" && transparency === null) { + return; + } const trimmed = value.trim().toUpperCase(); setHex(trimmed.startsWith("#") ? trimmed : `#${trimmed}`); setColourAt(null); - await onSetOverrideTag(COLOUR_TAGS[slot], written); + const tags: [string, string][] = [[COLOUR_TAGS[slot], written]]; + if (transparency !== null) { + tags.push([ALPHA_TAGS[slot], transparency]); + } + if (caretAt === null) { + return; + } + await onSetOverrideTags(tags, caretAt); } /** @@ -1253,6 +1290,24 @@ export default function CurrentLine({ } }} /> + + {en.subtitle.currentLine.transparency} + setAlpha(event.target.value)} + onKeyDown={(event) => { + if (event.key === "Enter") { + event.preventDefault(); + void pickColour(colourAt.slot, hex); + } + }} + /> + )} diff --git a/src/hooks/useSubtitleFile.ts b/src/hooks/useSubtitleFile.ts index 6cd2988..f89ff10 100644 --- a/src/hooks/useSubtitleFile.ts +++ b/src/hooks/useSubtitleFile.ts @@ -111,14 +111,6 @@ export type SubtitleFile = { clearText: (cue: number, keepTags: boolean) => Promise; /** Begin a translation from the source: same cues, same timings, nothing written yet. See S2. */ newTranslation: () => Promise; - /** One override tag with a value the caller chose, over the stretch a flag is flipped on. B12. */ - setOverrideTag: ( - cue: number, - tag: string, - value: string, - from: number, - to: number, - ) => Promise; /** `before === cues.length` appends; the four below carry the backend's own argument names. */ insertCue: (before: number, startMs: number, endMs: number, text: string) => Promise; deleteCue: (cue: number) => Promise; @@ -420,12 +412,6 @@ export function useSubtitleFile(onRowsMoved: RowsMoved, onPanels: PanelSink): Su [command], ); - const setOverrideTag = useCallback( - (cue: number, tag: string, value: string, from: number, to: number) => - command("subtitle_set_override_tag", { cue, write: { tag, value, from, to } }), - [command], - ); - const setOverrideTags = useCallback( (cue: number, tags: [string, string][], at: number) => command("subtitle_set_override_tags", { cue, write: { tags, at } }), @@ -532,7 +518,6 @@ export function useSubtitleFile(onRowsMoved: RowsMoved, onPanels: PanelSink): Su setField, setComment, toggleStyle, - setOverrideTag, setOverrideTags, newTranslation, clearText, diff --git a/src/i18n/en.ts b/src/i18n/en.ts index 1cd0b7f..e06a4b9 100644 --- a/src/i18n/en.ts +++ b/src/i18n/en.ts @@ -457,6 +457,12 @@ export const en = { }, /** The picker's own field, which takes a colour written the way the web writes one. */ colourValue: "Colour, as #RRGGBB", + /** + * How see-through the colour is. ASS counts transparency and not opacity, so 0 is solid and + * 255 is invisible, and the field says the word rather than the number's direction. + */ + transparency: "Clear", + transparencyName: "Transparency, 0 solid to 255 invisible", /** The font this line is drawn in, over what the style says. See edit-bar-tasks.md B12. */ font: "Font", fontFamily: "Font family",