From 3d8fafa62573003b3533c35106962d6a36fac836 Mon Sep 17 00:00:00 2001 From: erklu Date: Sun, 18 Oct 2026 07:20:42 +0000 Subject: [PATCH 01/13] Add word highlights to the transcript editor, with optional RTF export (#131) Select text and press Ctrl+Shift+H, or use the Highlight button in the toolbar, to highlight it; do it again to take the highlight off. A note to self made while reviewing against the recording -- it never changes the text. - Anchored to whole words (SRTCaption.highlighted_words), kept apart from the "my edits" marks. carry_highlights() moves them with the words through typing, split, merge, delete across captions, word moves and Find & Replace; a word typed between two highlighted words joins the highlight, any other new word does not earn one. - Saved with the transcription ("highlighted" next to "edited"). Transcriptions only for now: a set of subtitles is plain SRT and has nowhere to keep them. - Drawn yellow with a line above and below so colour is not the only cue (1.4.1, 1.4.11), role="mark", distinguishable from flagged words, my edits and the word being played, in both themes. - RTF export can carry highlights and changed words; the options are unchecked by default and only shown when there is something to carry (a clean export is unchanged). - Shortcuts dialog gets a "Highlighting" group for transcriptions. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01LCjpD3ehigLiUGH1oVQQAW --- AGENTS.md | 11 + pages/srt.py | 24 + tests/test_highlights.py | 1027 +++++++++++++++++++++++++++++++ tests/test_my_edits.py | 5 +- tests/test_transcript_editor.py | 9 + utils/caption.py | 13 + utils/srt.py | 124 +++- utils/srt_export.py | 96 ++- utils/srt_render.py | 11 + utils/srt_review.py | 228 ++++++- utils/srt_search.py | 10 +- utils/styles.py | 65 ++ utils/transcript_editor.js | 128 +++- utils/transcript_editor.py | 78 +++ 14 files changed, 1800 insertions(+), 29 deletions(-) create mode 100644 tests/test_highlights.py diff --git a/AGENTS.md b/AGENTS.md index 3e22e04..9c02fe9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -124,6 +124,17 @@ There was a separate "Uncertain words" switch alongside a Low/Medium/High select - **The marking must not read as an error.** No red, and no wavy underline (that means spellcheck). It uses `--color-review-*`, a violet reserved for this and used nowhere else. - The flagged counter comes from `flagged_word_count()` over the whole word list, and reports 0 while the switch is off. Anything that changes the switch or the sensitivity must call `update_flagged_count()`. +### Highlights (SUNET/scribe-ui#131) + +The reader's own highlighter: a note to self made while reviewing against the recording, never a change to the text. Stored as `SRTCaption.highlighted_words`, word positions counted exactly as `edited_words` is, and **kept apart from it** -- the two mean different things and move differently when the text does. Anchored to whole words, not characters: a selection that takes any of a word takes all of it (`words_in_selection()`; nothing selected means the word the caret is in or against), so a highlight survives every edit the way an edit mark does, and cannot end up cutting a word. + +- **They follow the words.** `carry_highlights(previous, current, marked)` is the highlight's own `retag_edits`: words that stay move with their marks, words replaced one for one keep the mark, a longer or shorter replacement is highlighted when everything it replaced was, words typed *between two highlighted words* join the highlight and words typed anywhere else do **not** earn one (unlike an edit mark: typing a word is not highlighting it), removed words take theirs with them. `carry_marks(caption, new_text)` does both kinds in one call and is what every site that replaces a caption's text goes through (typing, split with unsent text, Find & Replace). Split/merge/`delete_range` reuse `split_edits`/`joined_edits` on the second set, and the one-word moves carry it across (they still do not carry edit marks). Any new place that rewrites `caption.text` has to carry both sets. +- **Toggling.** Ctrl+Shift+H (Ctrl only, by `event.code === "KeyH"`: Cmd+Shift+H is the browser's Home on a Mac; no Alt) reports `togglehighlight` with one `{id, start, end}` per block the selection reaches into (`highlightSpans()`, offsets in each block's own text), ahead of the block-scoped lists because those see only the caret's block. `SRTEditor.toggle_highlight()` takes everything touched: all already highlighted -> remove, otherwise add (a mixed selection becomes fully highlighted, a second press clears it). One undo step. `TranscriptEditor.toggle_highlight()` redraws and **puts the selection back** (`selectRange()`, offsets are unchanged because the text is), so the same key undoes what it did. Nothing selected and no word at the caret says so. **The toolbar's Highlight button** (`pages/srt.py`, where Validate sits in the subtitle editor, so transcriptions only) does the same for whoever does not know the key: it calls `TranscriptEditor.highlight_selection()` -> the component's `highlightSelection()`, which reads the selection as it is and emits the same `togglehighlight` (an empty list when nothing is selected, answered with the notice). It prevents the default on `mousedown`, as the caption row's note icon does, so pressing it does not take focus and the selection with it; `aria-keyshortcuts` names the key, the tooltip too. +- **Saved with the transcription -- and for now only there.** `to_dict()` writes `"highlighted"` (only when non-empty) next to `"edited"` and `parse_txt()` reads it back, ignoring anything that is not a list of ints. A set of subtitles is saved as plain SRT, which has no room for it, so **highlights are a transcription feature for now**: the key is not bound in `subtitleMode`, `SRTEditor.toggle_highlight()` refuses for `data_format == "srt"`, the shortcuts dialog has no "Highlighting" group there, and neither export option is offered or applied (`export_rtf()` ignores both for subtitles). The way to extend it is a place to keep marks beside an SRT result (positions plus a digest of each caption's text, restored only onto a caption with the same number and text) -- a backend endpoint; a working draft exists on the local branch `backup/highlight-words-with-subtitle-marks`, never pushed. +- **Drawing.** `review_runs()` gives a highlighted word `hl: true` (and makes every word a run of its own whenever a block has any highlight, since it is drawn with no switch on); the space between two highlighted words is a highlighted run too, so a passage reads as one stretch, except across a line break. The component draws `runClass(run)` (the review/edit class **and** `highlight-word`) with `role="mark"`. Yellow, and a 2px line above and below: yellow on white is 1.14:1 so colour cannot carry it (1.4.1/1.4.11) -- the lines are 4.9:1 against the page in light mode and the text on the yellow 13.6:1; dark mode uses a dark gold (8.0:1 text, 12.6:1 frame). No horizontal padding, so highlighting does not move the text. On a flagged or changed word the highlight takes the wash and the word keeps its own bar underneath; the playing word's tint wins the wash and the lines stay. +- **RTF export** (`export_rtf(..., highlights=False, edits=False)`, the two checkboxes in the export dialog, transcriptions only, **off by default** and **hidden when there is no mark of that kind** to carry, `rtf_marks_available()`, across every transcription of a bulk export): highlights as `\highlight1` (pure yellow, Word's own marker colour), changed words teal and underlined (`\cf2\ul`, underlined because a printout may have no colour). `marked_spans()` splits a caption into whole-word pieces; the same bridging rule as the editor. The colour table is only written when used, so a clean export is byte-for-byte what it was. No other format carries either. +- Tests: `tests/test_highlights.py`. + ## Undo history (`utils/undo_redo.py`) `save_state()` snapshots an `EditorState`: the captions **and** the speaker set, copied so a later edit to the live editor cannot reach back into a state already on a stack. Both halves have to travel together — the captions carry which speaker each block belongs to, the list carries which names exist at all, and the list cannot simply be recomputed from the captions on the way back, since a speaker can legitimately exist with no block using it ("Add new", or every block reassigned away). Restoring only the captions left a block naming a speaker the list no longer held: the menu highlighted nothing as current, `prompt_rename` refused to act on that name (it checks membership), and the name introduced by the undone edit lingered as unused forever. diff --git a/pages/srt.py b/pages/srt.py index 39ce6ba..5a3c87c 100644 --- a/pages/srt.py +++ b/pages/srt.py @@ -256,6 +256,30 @@ async def look_up_original() -> None: "click", lambda: editor.validate_captions(), ) + else: + # Where Validate is in the subtitle editor: the one thing + # about the text that is not history or finding. The same + # as Ctrl+Shift+H. Transcriptions only, for now. + # + # A mouse press must not take focus, or the selection it + # is meant to act on is the first thing it would lose -- + # the same reason the caption row's music-note icon + # prevents the default on mousedown. + with ui.button("Highlight", icon="highlight").props( + "flat aria-keyshortcuts=Control+Shift+H" + ).classes( + "editor-btn editor-toolbar-btn editor-quiet" + ) as highlight_button: + highlight_button.tooltip( + "Highlight the selection, or remove the highlight " + "(Ctrl + Shift + H)" + ) + highlight_button.on( + "mousedown", js_handler="(event) => event.preventDefault()" + ) + highlight_button.on( + "click", lambda: transcript.highlight_selection() + ) with ui.row().classes("editor-toolbar-group"): editor.show_keyboard_shortcuts() diff --git a/tests/test_highlights.py b/tests/test_highlights.py new file mode 100644 index 0000000..2029835 --- /dev/null +++ b/tests/test_highlights.py @@ -0,0 +1,1027 @@ +# Copyright (c) 2025-2026 Sunet. +# Contributor: Kristofer Hallin +# +# This file is part of Sunet Scribe. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +The reader's own highlights: kept per word, carried through every edit that +moves words, saved with a transcription, drawn, and optionally exported. +""" + +import json +import pathlib +import re + +import pytest + +from utils.caption import SRTCaption +from utils.srt import SRTEditor +from utils.transcript_editor import TranscriptEditor + + +WORDS = { + "version": 1, + "words": [ + {"t": "ett", "s": 0.0, "e": 0.5}, + {"t": "två", "s": 0.6, "e": 1.0}, + {"t": "tre", "s": 1.1, "e": 1.5}, + {"t": "fyra", "s": 2.0, "e": 2.4}, + {"t": "fem", "s": 2.5, "e": 2.9}, + {"t": "sex", "s": 3.0, "e": 3.5}, + ], +} + + +def make_editor(data_format="txt"): + editor = SRTEditor("job-uuid", data_format, "file") + + editor.changed = 0 + editor.snapshots = 0 + + def mark(): + editor.changed += 1 + + def snapshot(): + editor.snapshots += 1 + + editor.refresh_display = lambda *a, **k: None + editor.update_words_per_minute = lambda *a, **k: None + editor.update_flagged_count = lambda *a, **k: None + editor.mark_as_changed = mark + editor.save_state_for_undo = snapshot + + editor.load_words(WORDS) + editor.captions = [ + SRTCaption(1, "00:00:00,000", "00:00:01,500", "ett två tre", speaker="A"), + SRTCaption(2, "00:00:02,000", "00:00:03,500", "fyra fem sex", speaker="A"), + ] + editor.speakers = {"A"} + + return editor + + +@pytest.fixture +def editor(monkeypatch): + messages = [] + monkeypatch.setattr( + "utils.srt.ui.notify", lambda message, **k: messages.append(message) + ) + built = make_editor() + built.messages = messages + + return built + + +class TestWordsInSelection: + def test_a_selection_takes_the_words_it_touches(self, editor): + assert editor.words_in_selection("ett två tre", 4, 7) == [1] + + def test_a_selection_ending_inside_a_word_takes_all_of_it(self, editor): + assert editor.words_in_selection("ett två tre", 1, 5) == [0, 1] + + def test_a_selection_only_brushing_an_edge_does_not_take_that_word(self, editor): + # Ends where "två" begins; starts where "ett" ends. + assert editor.words_in_selection("ett två tre", 0, 4) == [0] + assert editor.words_in_selection("ett två tre", 3, 8) == [1] + + def test_the_whole_text(self, editor): + assert editor.words_in_selection("ett två tre", 0, 11) == [0, 1, 2] + + def test_a_backwards_selection_is_the_same(self, editor): + assert editor.words_in_selection("ett två tre", 7, 4) == [1] + + def test_a_caret_in_a_word_takes_that_word(self, editor): + assert editor.words_in_selection("ett två tre", 5, 5) == [1] + + def test_a_caret_against_either_edge_of_a_word_takes_that_word(self, editor): + assert editor.words_in_selection("ett två tre", 3, 3) == [0] + assert editor.words_in_selection("ett två tre", 4, 4) == [1] + + def test_a_caret_in_the_space_between_words_takes_none(self, editor): + assert editor.words_in_selection("ett två", 4, 4) == [] + + def test_line_breaks_separate_words_too(self, editor): + assert editor.words_in_selection("ett\ntvå", 4, 6) == [1] + + def test_empty_text(self, editor): + assert editor.words_in_selection("", 0, 0) == [] + + +class TestCarryHighlights: + def carry(self, editor, before, after, marked): + return editor.carry_highlights(before, after, set(marked)) + + def test_words_that_stay_move_with_their_marks(self, editor): + assert self.carry(editor, "b c", "a b c", {0}) == {1} + + def test_words_removed_take_their_marks_with_them(self, editor): + assert self.carry(editor, "a b c", "a c", {1}) == set() + assert self.carry(editor, "a b c", "a c", {0, 1, 2}) == {0, 1} + + def test_a_word_corrected_inside_a_highlight_stays_highlighted(self, editor): + assert self.carry(editor, "teh cat sat", "the cat sat", {0, 1}) == {0, 1} + + def test_a_word_corrected_elsewhere_does_not_gain_one(self, editor): + assert self.carry(editor, "teh cat sat", "the cat sat", {1}) == {1} + + def test_a_new_word_does_not_earn_a_highlight(self, editor): + assert self.carry(editor, "a b", "a new b", set()) == set() + assert self.carry(editor, "a b", "a b new", {0, 1}) == {0, 1} + + def test_a_word_typed_inside_a_highlight_joins_it(self, editor): + assert self.carry(editor, "a b", "a new b", {0, 1}) == {0, 1, 2} + + def test_a_word_typed_next_to_a_highlight_does_not(self, editor): + assert self.carry(editor, "a b c", "a b new c", {0, 1}) == {0, 1} + assert self.carry(editor, "a b c", "new a b c", {0, 1}) == {1, 2} + + def test_a_longer_replacement_of_highlighted_words_is_highlighted(self, editor): + assert self.carry(editor, "a x y d", "a p q r d", {1, 2}) == {1, 2, 3} + + def test_a_replacement_of_part_of_a_highlight_is_not(self, editor): + assert self.carry(editor, "a x y d", "a p q r d", {1}) == set() + + def test_a_shorter_replacement_of_highlighted_words_is_highlighted(self, editor): + assert self.carry(editor, "a x y d", "a p d", {1, 2}) == {1} + + def test_the_input_is_not_changed(self, editor): + marked = {0} + editor.carry_highlights("a b", "x a b", marked) + + assert marked == {0} + + def test_no_marks_stay_none(self, editor): + assert self.carry(editor, "a b c", "a x c d", set()) == set() + + +class TestToggling: + def test_it_highlights_the_words_selected(self, editor): + first = editor.captions[0] + + assert editor.toggle_highlight([(first, 4, 7)]) == "added" + + assert first.highlighted_words == {1} + + def test_the_same_selection_again_takes_it_off(self, editor): + first = editor.captions[0] + editor.toggle_highlight([(first, 4, 7)]) + + assert editor.toggle_highlight([(first, 4, 7)]) == "removed" + + assert first.highlighted_words == set() + + def test_a_mixed_selection_is_highlighted_throughout(self, editor): + first = editor.captions[0] + first.highlighted_words = {0} + + assert editor.toggle_highlight([(first, 0, 11)]) == "added" + + assert first.highlighted_words == {0, 1, 2} + + def test_a_selection_inside_a_highlight_takes_off_only_what_it_holds(self, editor): + first = editor.captions[0] + first.highlighted_words = {0, 1, 2} + + assert editor.toggle_highlight([(first, 4, 7)]) == "removed" + + assert first.highlighted_words == {0, 2} + + def test_a_selection_across_captions_is_one_gesture(self, editor): + first, second = editor.captions + + editor.toggle_highlight([(first, 4, 11), (second, 0, 4)]) + + assert first.highlighted_words == {1, 2} + assert second.highlighted_words == {0} + assert editor.snapshots == 1 + + def test_across_captions_it_comes_off_only_when_all_of_it_is_highlighted( + self, editor + ): + first, second = editor.captions + first.highlighted_words = {1, 2} + + # Not all highlighted yet: the second caption's word is not. + editor.toggle_highlight([(first, 4, 11), (second, 0, 4)]) + assert second.highlighted_words == {0} + + editor.toggle_highlight([(first, 4, 11), (second, 0, 4)]) + assert first.highlighted_words == set() + assert second.highlighted_words == set() + + def test_nothing_selected_says_so_and_changes_nothing(self, editor): + first = editor.captions[0] + + assert editor.toggle_highlight([(first, 3, 4)]) == "" + assert editor.toggle_highlight([]) == "" + + assert first.highlighted_words == set() + assert editor.snapshots == 0 + assert editor.changed == 0 + assert editor.messages == ["Select the words to highlight first"] * 2 + + def test_it_is_one_undo_step(self, editor): + editor.toggle_highlight([(editor.captions[0], 0, 11)]) + + assert editor.snapshots == 1 + + def test_the_text_is_untouched(self, editor): + editor.toggle_highlight([(editor.captions[0], 0, 11)]) + + assert [caption.text for caption in editor.captions] == [ + "ett två tre", + "fyra fem sex", + ] + assert editor.captions[0].edited_words == set() + + def test_a_transcription_counts_it_as_a_change_to_save(self, editor): + editor.toggle_highlight([(editor.captions[0], 0, 11)]) + + assert editor.changed == 1 + + def test_subtitles_get_none_for_now(self): + """ + Plain SRT has nowhere to keep a highlight, so a set of subtitles is + refused outright -- no change, no undo step, no notice. + """ + + editor = make_editor("srt") + + assert editor.toggle_highlight([(editor.captions[0], 0, 11)]) == "" + + assert editor.captions[0].highlighted_words == set() + assert editor.snapshots == 0 + assert editor.changed == 0 + + +class TestSavedWithTheTranscription: + def reopen(self, editor): + saved = json.dumps(editor.export_json()) + + reopened = make_editor() + reopened.captions = [] + reopened.speakers = set() + reopened.parse_txt(saved) + + return reopened + + def test_a_copy_does_not_share_the_marks(self): + caption = SRTCaption(1, "00:00:00,000", "00:00:01,000", "a b") + caption.highlighted_words = {0} + + copy = caption.copy() + copy.highlighted_words.add(1) + + assert caption.highlighted_words == {0} + + def test_they_are_in_the_saved_data_only_when_there_are_any(self): + caption = SRTCaption(1, "00:00:00,000", "00:00:01,000", "a b") + + assert "highlighted" not in caption.to_dict() + + caption.highlighted_words = {1, 0} + + assert caption.to_dict()["highlighted"] == [0, 1] + + def test_they_come_back_when_the_file_is_opened_again(self, editor): + editor.captions[0].highlighted_words = {0, 2} + editor.captions[1].highlighted_words = {1} + + reopened = self.reopen(editor) + + assert [caption.highlighted_words for caption in reopened.captions] == [ + {0, 2}, + {1}, + ] + + def test_edit_marks_and_highlights_are_kept_apart(self, editor): + editor.captions[0].edited_words = {0} + editor.captions[0].highlighted_words = {2} + + reopened = self.reopen(editor) + + assert reopened.captions[0].edited_words == {0} + assert reopened.captions[0].highlighted_words == {2} + + def test_a_file_saved_before_this_existed_opens_without_any(self, editor): + reopened = self.reopen(editor) + + assert all(not caption.highlighted_words for caption in reopened.captions) + + def test_junk_in_the_file_is_ignored(self, editor): + data = editor.export_json() + data["segments"][0]["highlighted"] = [0, "x", None, True, 2.5, 1] + data["segments"][1]["highlighted"] = "all of it" + + reopened = make_editor() + reopened.captions = [] + reopened.parse_txt(json.dumps(data)) + + assert reopened.captions[0].highlighted_words == {0, 1} + assert reopened.captions[1].highlighted_words == set() + + +class TestFollowingTheText: + """ + A highlight is on words, so it has to stay on them whatever the reader does + to the text around them. + """ + + def test_typing_before_it_moves_it_along(self, editor): + first = editor.captions[0] + first.highlighted_words = {1} + + editor.update_caption_text(first, "nytt ett två tre") + + assert first.highlighted_words == {2} + + def test_correcting_a_word_inside_it_keeps_it(self, editor): + first = editor.captions[0] + first.highlighted_words = {0, 1, 2} + + editor.update_caption_text(first, "ett tvo tre") + + assert first.highlighted_words == {0, 1, 2} + + def test_typing_a_word_gives_it_no_highlight_of_its_own(self, editor): + first = editor.captions[0] + + editor.update_caption_text(first, "ett två tre fyra") + + assert first.highlighted_words == set() + + def test_splitting_divides_them_between_the_halves(self, editor): + first = editor.captions[0] + first.highlighted_words = {0, 2} + + editor.split_caption(first, cursor_position=7) + + assert editor.captions[0].text == "ett två" + assert editor.captions[0].highlighted_words == {0} + assert editor.captions[1].text == "tre" + assert editor.captions[1].highlighted_words == {0} + + def test_splitting_with_unsent_text_keeps_them(self, editor): + first = editor.captions[0] + first.highlighted_words = {2} + + editor.split_caption(first, cursor_position=8, text="nytt ett två tre") + + assert editor.captions[0].text == "nytt ett" + assert editor.captions[0].highlighted_words == set() + assert editor.captions[1].text == "två tre" + assert editor.captions[1].highlighted_words == {1} + + def test_merging_with_the_next_joins_them(self, editor): + first, second = editor.captions + first.highlighted_words = {2} + second.highlighted_words = {0} + + editor.merge_with_next(first) + + assert editor.captions[0].highlighted_words == {2, 3} + + def test_merging_with_the_previous_joins_them(self, editor): + first, second = editor.captions + first.highlighted_words = {2} + second.highlighted_words = {0} + + editor.merge_with_previous(second) + + assert editor.captions[0].highlighted_words == {2, 3} + + def test_a_word_moved_to_the_previous_caption_takes_its_highlight(self, editor): + first, second = editor.captions + second.highlighted_words = {0, 2} + + editor.move_first_word_to_previous(second) + + assert first.highlighted_words == {3} + assert second.highlighted_words == {1} + + def test_a_word_moved_to_the_next_caption_takes_its_highlight(self, editor): + first, second = editor.captions + first.highlighted_words = {0, 2} + second.highlighted_words = {1} + + editor.move_last_word_to_next(first) + + assert first.highlighted_words == {0} + assert second.highlighted_words == {0, 2} + + def test_a_word_moved_without_one_leaves_the_rest_alone(self, editor): + first, second = editor.captions + second.highlighted_words = {2} + + editor.move_first_word_to_previous(second) + + assert first.highlighted_words == set() + assert second.highlighted_words == {1} + + def test_deleting_across_captions_keeps_what_is_left(self, editor): + first, second = editor.captions + first.highlighted_words = {0, 2} + second.highlighted_words = {0, 2} + + # From the end of "två" in the first to just after "fyra" in the + # second: "ett två" and " fem sex" are what is left. + editor.delete_range(first, second, 7, 4, "") + + assert editor.captions[0].text == "ett två fem sex" + # "ett" and "sex" were highlighted; "två" and "fem" were not. + assert editor.captions[0].highlighted_words == {0, 3} + + def test_replacing_text_keeps_them_following_the_words(self, editor): + first = editor.captions[0] + first.highlighted_words = {2} + editor.search_term = "ett" + editor.selected_caption = first + editor.case_sensitive = True + + editor.replace_in_current_caption("ett nytt") + + assert first.text == "ett nytt två tre" + assert first.highlighted_words == {3} + + +class TestDrawing: + def runs(self, editor, caption): + return editor.review_runs(caption) + + def test_nothing_is_drawn_when_nothing_is_highlighted(self, editor): + assert self.runs(editor, editor.captions[0]) == [ + {"t": "ett två tre", "flag": False} + ] + + def test_a_highlight_is_drawn_without_any_switch_on(self, editor): + first = editor.captions[0] + first.highlighted_words = {1} + + runs = self.runs(editor, first) + + assert [run["t"] for run in runs if run.get("hl")] == ["två"] + assert "".join(run["t"] for run in runs) == "ett två tre" + + def test_the_space_between_highlighted_words_is_highlighted_too(self, editor): + first = editor.captions[0] + first.highlighted_words = {0, 1} + + runs = self.runs(editor, first) + + assert [run["t"] for run in runs if run.get("hl")] == ["ett", " ", "två"] + + def test_the_space_around_a_lone_highlighted_word_is_not(self, editor): + first = editor.captions[0] + first.highlighted_words = {1} + + runs = self.runs(editor, first) + + assert [run["t"] for run in runs if run.get("hl")] == ["två"] + + def test_a_line_break_between_highlighted_words_is_left_out(self, editor): + first = editor.captions[0] + first.text = "ett\ntvå" + first.highlighted_words = {0, 1} + + runs = self.runs(editor, first) + + assert [run["t"] for run in runs if run.get("hl")] == ["ett", "två"] + assert "".join(run["t"] for run in runs) == "ett\ntvå" + + def test_highlights_are_not_runs_with_a_flag(self, editor): + first = editor.captions[0] + first.highlighted_words = {0} + + assert all(run["flag"] is False for run in self.runs(editor, first)) + + def test_a_changed_word_can_be_highlighted_too(self, editor): + editor.show_my_edits = True + first = editor.captions[0] + first.highlighted_words = {1} + first.edited_words = {1} + + run = [run for run in self.runs(editor, first) if run["t"] == "två"][0] + + assert run["hl"] is True + assert run["edit"] is True + + def test_it_carries_the_word_timing_when_following_the_audio(self, editor): + first = editor.captions[0] + first.highlighted_words = {1} + + runs = editor.review_runs(first, per_word=True) + run = [run for run in runs if run["t"] == "två"][0] + + assert run["hl"] is True + assert (run["s"], run["e"]) == (0.6, 1.0) + + def test_other_words_read_as_they_always_did(self, editor): + editor.show_my_edits = True + first = editor.captions[0] + first.highlighted_words = {1} + + runs = self.runs(editor, first) + + assert "hl" not in [run for run in runs if run["t"] == "ett"][0] + + +class TestRtfExport: + def export(self, editor, **kwargs): + return editor.export_rtf(False, False, False, **kwargs) + + def test_a_clean_export_has_no_marking_and_no_colour_table(self, editor): + editor.captions[0].highlighted_words = {1} + editor.captions[0].edited_words = {0} + + clean = self.export(editor) + + assert "highlight" not in clean + assert "colortbl" not in clean + assert "\\ul" not in clean + + def test_a_clean_export_is_the_same_as_before_highlights_existed(self, editor): + before = self.export(editor) + + editor.captions[0].highlighted_words = {0, 1} + editor.captions[1].edited_words = {2} + + assert self.export(editor) == before + + def test_highlights_are_a_yellow_marker_when_asked_for(self, editor): + editor.captions[0].highlighted_words = {1} + + rtf = self.export(editor, highlights=True) + + assert r"{\colortbl;\red255\green255\blue0;" in rtf + assert r"ett {\highlight1 tv\u229?} tre" in rtf + + def test_a_passage_is_one_marked_stretch(self, editor): + editor.captions[0].highlighted_words = {0, 1} + + rtf = self.export(editor, highlights=True) + + assert rtf.count(r"\highlight1") == 1 + assert r"{\highlight1 ett tv\u229?} tre" in rtf + + def test_a_piece_starting_with_a_space_keeps_it(self, editor): + # The one space after a control word is its delimiter; any more are + # the text. Two highlighted words with two spaces between them are + # bridged, so the piece starts at the first word; the piece after a + # lone highlighted word starts with the spaces. + editor.captions[0].text = "ett två" + editor.captions[0].highlighted_words = {0} + + rtf = self.export(editor, highlights=True) + + assert r"{\highlight1 ett} tv\u229?" in rtf + + def test_changed_words_are_underlined_and_teal_when_asked_for(self, editor): + editor.captions[0].edited_words = {2} + + rtf = self.export(editor, edits=True) + + assert r"\red18\green121\blue106;" in rtf + assert r"{\cf2\ul tre}" in rtf + assert "highlight1" not in rtf + + def test_both_on_the_same_word(self, editor): + editor.captions[0].edited_words = {2} + editor.captions[0].highlighted_words = {2} + + rtf = self.export(editor, highlights=True, edits=True) + + assert r"{\highlight1\cf2\ul tre}" in rtf + + def test_each_is_independent_of_the_other(self, editor): + editor.captions[0].edited_words = {2} + editor.captions[0].highlighted_words = {0} + + highlights_only = self.export(editor, highlights=True) + edits_only = self.export(editor, edits=True) + + assert r"\ul" not in highlights_only + assert "highlight1" not in edits_only + + def test_the_text_is_the_same_with_or_without(self, editor): + editor.captions[0].highlighted_words = {0, 1, 2} + editor.captions[0].edited_words = {1} + editor.captions[1].highlighted_words = {1} + + def body(rtf): + return rtf.split(r"\fs20 ", 1)[1] + + marked = body(self.export(editor, highlights=True, edits=True)) + clean = body(self.export(editor)) + + # Take the groups back out: what is left must be the clean text. + stripped = re.sub( + r"\{(?:\\highlight1)?(?:\\cf2\\ul)? ", "", marked + ) + # Every group's closing brace goes, but the document's own stays. + stripped = stripped[:-1].replace("}", "") + "}" + + assert stripped == clean + + def test_subtitles_never_carry_them_for_now(self): + subtitles = make_editor("srt") + subtitles.captions[0].edited_words = {0} + subtitles.captions[0].highlighted_words = {1} + + rtf = self.export(subtitles, highlights=True, edits=True) + + assert "highlight" not in rtf + assert "colortbl" not in rtf + assert "\\ul" not in rtf + + def test_a_line_break_is_still_a_line_break(self, editor): + editor.captions[0].text = "ett\ntvå" + editor.captions[0].highlighted_words = {0, 1} + + rtf = self.export(editor, highlights=True) + + assert r"{\highlight1 ett}\line {\highlight1 tv\u229?}" in rtf + + def test_special_characters_in_a_marked_word_are_escaped(self, editor): + editor.captions[0].text = "a {b} c" + editor.captions[0].highlighted_words = {1} + + rtf = self.export(editor, highlights=True) + + assert r"{\highlight1 \{b\}}" in rtf + + def test_the_braces_balance(self, editor): + editor.captions[0].highlighted_words = {0, 2} + editor.captions[0].edited_words = {1, 2} + editor.captions[1].highlighted_words = {0, 1, 2} + + rtf = self.export(editor, highlights=True, edits=True) + unescaped = rtf.replace("\\{", "").replace("\\}", "") + + assert unescaped.count("{") == unescaped.count("}") + + +class TestOfferingTheMarks: + """ + The export dialog offers a kind of mark only when there is one to carry, + and only from a transcription, for now. + """ + + def test_nothing_marked_offers_nothing(self, editor): + assert editor.rtf_marks_available() == (False, False) + + def test_each_kind_is_offered_on_its_own(self, editor): + editor.captions[1].highlighted_words = {0} + assert editor.rtf_marks_available() == (True, False) + + editor.captions[1].highlighted_words = set() + editor.captions[0].edited_words = {2} + assert editor.rtf_marks_available() == (False, True) + + def test_both(self, editor): + editor.captions[0].highlighted_words = {0} + editor.captions[1].edited_words = {0} + + assert editor.rtf_marks_available() == (True, True) + + def test_a_bulk_export_offers_a_kind_if_any_file_has_it(self, editor): + other = make_editor() + other.captions[0].highlighted_words = {1} + + assert editor.rtf_marks_available([other]) == (True, False) + + def test_subtitles_offer_nothing_for_now(self): + subtitles = make_editor("srt") + subtitles.captions[0].edited_words = {0} + + assert subtitles.rtf_marks_available() == (False, False) + + def test_subtitles_in_a_bulk_export_are_not_counted(self, editor): + subtitles = make_editor("srt") + subtitles.captions[0].edited_words = {0} + + assert editor.rtf_marks_available([subtitles]) == (False, False) + + def test_the_checkboxes_follow_it_and_start_unchecked(self): + source = pathlib.Path("utils/srt_export.py").read_text() + + assert "rtf_hl_incl.set_visibility(has_highlights)" in source + assert "rtf_edits_incl.set_visibility(has_edits)" in source + assert '"Include highlights", value=False' in source + assert '"Include my edits (underlined)", value=False' in source + + +class TestMarkedSpans: + def test_the_pieces_are_the_text_again(self, editor): + caption = editor.captions[0] + caption.text = "ett två\ntre" + caption.highlighted_words = {0, 2} + caption.edited_words = {1} + + spans = editor.marked_spans(caption, True, True) + + assert "".join(text for text, _, _ in spans) == "ett två\ntre" + + def test_neighbouring_words_with_the_same_marks_are_one_piece(self, editor): + caption = editor.captions[0] + caption.highlighted_words = {0, 1} + + assert editor.marked_spans(caption, True, False) == [ + ("ett två", True, False), + (" tre", False, False), + ] + + def test_with_both_off_it_is_one_unmarked_piece(self, editor): + caption = editor.captions[0] + caption.highlighted_words = {0, 1} + caption.edited_words = {2} + + assert editor.marked_spans(caption, False, False) == [ + ("ett två tre", False, False) + ] + + def test_no_text_no_pieces(self, editor): + caption = editor.captions[0] + caption.text = "" + + assert editor.marked_spans(caption, True, True) == [] + + +class FakeBody: + def __init__(self): + self.selected = [] + self.asked = 0 + + def highlight_selection(self): + self.asked += 1 + + def select_range(self, *args): + self.selected.append(args) + + +@pytest.fixture +def view(editor, monkeypatch): + view = TranscriptEditor(editor) + view.body = FakeBody() + view.refreshed = 0 + view.refresh = lambda *a, **k: setattr(view, "refreshed", view.refreshed + 1) + view.changed = lambda *a, **k: None + + return view + + +class TestTheShortcutsDialog: + def groups(self, data_format): + return dict(make_editor(data_format).keyboard_shortcut_groups()) + + def test_a_transcription_lists_it(self): + assert self.groups("txt")["Highlighting"] == [ + ("Highlight selection, or remove the highlight", "Ctrl + Shift + H") + ] + + def test_subtitles_do_not_for_now(self): + assert "Highlighting" not in self.groups("srt") + + +class TestTheHandler: + def test_the_event_reaches_the_view(self): + source = pathlib.Path("utils/transcript_editor.py").read_text() + + assert ( + 'self.body.on(\n "togglehighlight", ' + "lambda event: self.toggle_highlight(event.args)\n )" + ) in source + + def test_it_highlights_what_the_component_measured(self, view, editor): + view.toggle_highlight({"spans": [{"id": 1, "start": 4, "end": 7}]}) + + assert editor.captions[0].highlighted_words == {1} + assert view.refreshed == 1 + + def test_the_selection_is_put_back_after_the_redraw(self, view, editor): + view.toggle_highlight( + { + "spans": [ + {"id": 1, "start": 4, "end": 11}, + {"id": 2, "start": 0, "end": 4}, + ] + } + ) + + assert view.body.selected == [(1, 4, 2, 4)] + + def test_nothing_selected_changes_nothing_and_redraws_nothing(self, view, editor): + view.toggle_highlight({"spans": [{"id": 1, "start": 3, "end": 4}]}) + + assert view.refreshed == 0 + assert view.body.selected == [] + + @pytest.mark.parametrize( + "args", + [ + None, + "x", + {}, + {"spans": None}, + {"spans": "all"}, + {"spans": []}, + {"spans": [None, 3, "x"]}, + {"spans": [{"id": 99, "start": 0, "end": 3}]}, + {"spans": [{"id": 1, "start": "0", "end": 3}]}, + {"spans": [{"id": 1, "start": True, "end": 3}]}, + {"spans": [{"id": 1, "start": 0}]}, + ], + ) + def test_bad_input_is_harmless(self, view, editor, args): + view.toggle_highlight(args) + + assert all(not caption.highlighted_words for caption in editor.captions) + assert view.refreshed == 0 + + def test_a_bad_span_does_not_stop_the_good_ones(self, view, editor): + view.toggle_highlight( + { + "spans": [ + {"id": 99, "start": 0, "end": 3}, + {"id": 2, "start": 0, "end": 4}, + ] + } + ) + + assert editor.captions[1].highlighted_words == {0} + + +class TestTheButton: + """ + The toolbar's Highlight button, where Validate is in the subtitle editor: + the same as the key, for whoever does not know the key. + """ + + def page(self): + return pathlib.Path("pages/srt.py").read_text() + + def test_it_asks_the_component_for_the_selection(self, view): + view.highlight_selection() + + assert view.body.asked == 1 + + def test_it_is_harmless_before_the_editor_is_built(self, editor): + TranscriptEditor(editor).highlight_selection() + + def test_nothing_selected_says_so_and_changes_nothing(self, view, editor): + view.toggle_highlight({"spans": []}) + + assert editor.messages == ["Select the words to highlight first"] + assert view.refreshed == 0 + assert all(not caption.highlighted_words for caption in editor.captions) + + def test_the_component_reports_the_selection_as_the_key_does(self): + source = pathlib.Path("utils/transcript_editor.js").read_text() + body = source[source.index("highlightSelection() {") :] + body = body[: body.index("\n },")] + + assert "this.flush();" in body + assert 'this.$emit("togglehighlight", { spans: this.highlightSpans() });' in body + + def test_it_is_in_the_toolbar_for_transcriptions_only(self): + page = self.page() + validate = page.index('with ui.button("Validate", icon="check")') + highlight = page.index('with ui.button("Highlight", icon="highlight")') + + # After Validate's own branch, in the else of the same test. + between = page[validate:highlight] + + assert between.count("\n else:\n") == 1 + + def test_it_is_a_labelled_quiet_toolbar_button_naming_its_key(self): + page = self.page() + start = page.index('with ui.button("Highlight", icon="highlight")') + button = page[start : start + 1400] + + assert "aria-keyshortcuts=Control+Shift+H" in button + assert "editor-btn editor-toolbar-btn editor-quiet" in button + assert "(Ctrl + Shift + H)" in button + + def test_a_mouse_press_does_not_take_the_selection_with_it(self): + page = self.page() + start = page.index('with ui.button("Highlight", icon="highlight")') + button = page[start : start + 1400] + + assert '"mousedown", js_handler="(event) => event.preventDefault()"' in button + + def test_a_click_is_the_same_as_the_key(self): + page = self.page() + start = page.index('with ui.button("Highlight", icon="highlight")') + button = page[start : start + 1400] + + assert '"click", lambda: transcript.highlight_selection()' in button + + +class TestTheComponent: + def source(self): + return pathlib.Path("utils/transcript_editor.js").read_text() + + def test_the_key_is_ctrl_shift_h_by_physical_key(self): + source = self.source() + start = source.index("event.ctrlKey &&\n event.shiftKey") + block = source[start : start + 260] + + assert "!event.altKey" in block + assert "!event.metaKey" in block + assert 'event.code === "KeyH"' in block + + assert '"togglehighlight"' in source + + def test_it_is_for_transcriptions_only_for_now(self): + source = self.source() + start = source.index("!this.subtitleMode &&\n event.ctrlKey &&") + + assert 'event.code === "KeyH"' in source[start : start + 260] + + def test_it_is_ahead_of_the_block_scoped_lists(self): + """ + A selection can reach across blocks, which the lists -- working from + the caret's own block -- cannot see. + """ + + source = self.source() + + assert source.index('"togglehighlight"') < source.index( + "this.subtitleShortcut(event, at)" + ) + + def test_both_marking_classes_come_from_one_function(self): + source = self.source() + + assert source.count(':class="runClass(run)"') == 2 + assert 'run.hl ? "highlight-word" : null' in source + + def test_a_highlight_is_exposed_as_marked_text(self): + assert self.source().count(""":role="run.hl ? 'mark' : null\"""") == 2 + + def test_the_selection_is_sent_one_range_per_block(self): + source = self.source() + body = source[source.index("highlightSpans() {") :] + body = body[: body.index("\n },")] + + assert "first === last" in body + assert "this.plainText(block.textContent).length" in body + + def test_the_selection_can_be_put_back(self): + source = self.source() + + assert "selectRange(startId, startOffset, endId, endOffset) {" in source + assert "pointAt(block, offset) {" in source + + +class TestTheStyles: + def css(self): + return pathlib.Path("utils/styles.py").read_text() + + def rule(self, selector): + css = self.css() + rule = css[css.index(selector) :] + + return rule[: rule.index("}")] + + def test_it_has_its_own_colours_in_both_themes(self): + css = self.css() + + assert css.count("--color-highlight-bg:") == 2 + assert css.count("--color-highlight-frame:") == 2 + assert css.count("--color-highlight-text:") == 2 + + def test_it_does_not_pad_the_text_sideways(self): + assert "padding: 1px 0;" in self.rule(" .highlight-word {") + + def test_it_has_a_line_above_and_below_that_does_not_depend_on_colour(self): + rule = self.rule(" .highlight-word {") + + assert "inset 0 2px 0 var(--color-highlight-frame)" in rule + assert "inset 0 -2px 0 var(--color-highlight-frame)" in rule + + def test_a_flagged_or_changed_word_keeps_its_own_bar(self): + review = self.rule(" .highlight-word.review-word {") + edit = self.rule(" .highlight-word.edit-word {") + + assert "inset 0 2px 0 var(--color-highlight-frame)" in review + assert "inset 0 -2px 0 var(--color-review-accent)" in review + assert "inset 0 2px 0 var(--color-highlight-frame)" in edit + assert "inset 0 -2px 0 var(--color-edit-accent)" in edit + + def test_the_word_being_played_keeps_the_lines(self): + rule = self.rule(" .transcript-text .highlight-word[data-current] {") + + assert "background-color: var(--color-playing-bg);" in rule + assert "inset 0 2px 0 var(--color-highlight-frame)" in rule diff --git a/tests/test_my_edits.py b/tests/test_my_edits.py index 7505b78..40b9417 100644 --- a/tests/test_my_edits.py +++ b/tests/test_my_edits.py @@ -716,7 +716,10 @@ def test_a_space_already_gone_just_clears_the_reference(self): def test_the_template_renders_the_edit_marking(self): source = self.source() - assert "'edit-word'" in source + # Chosen in runClass, which the template calls for every run, so the + # highlight can be added to the same list without a second binding. + assert '"edit-word"' in source + assert ':class="runClass(run)"' in source # Bound on every word, not only the ones the server marked, so the # stylesheet can reach the message on a word the browser marked too. assert ':data-edit="editLabel"' in source diff --git a/tests/test_transcript_editor.py b/tests/test_transcript_editor.py index 6890104..084ded8 100644 --- a/tests/test_transcript_editor.py +++ b/tests/test_transcript_editor.py @@ -2819,6 +2819,15 @@ def test_the_transcription_list_is_exactly_these_and_nothing_else(self): ("Delete paragraph", "Ctrl + D"), ], ), + ( + "Highlighting", + [ + ( + "Highlight selection, or remove the highlight", + "Ctrl + Shift + H", + ), + ], + ), ( "File operations", [ diff --git a/utils/caption.py b/utils/caption.py index 7c621ae..0f610b4 100644 --- a/utils/caption.py +++ b/utils/caption.py @@ -52,6 +52,13 @@ def __init__( # and marked untouched words as edited. self.edited_words: Set[int] = set() + # Word positions the reader has highlighted for themselves, counted the + # same way. A note to self made while reviewing against the recording: + # it never changes the text, and it is kept apart from edited_words + # because the two mean different things and move differently when the + # text changes (see carry_highlights). + self.highlighted_words: Set[int] = set() + def copy(self) -> "SRTCaption": """ Create a deep copy of the caption. @@ -71,6 +78,7 @@ def copy(self) -> "SRTCaption": # Copied rather than shared: the undo stack holds these snapshots, so # undoing a change has to take its marks back with it. new_caption.edited_words = set(self.edited_words) + new_caption.highlighted_words = set(self.highlighted_words) return new_caption def to_dict(self) -> Dict[str, Any]: @@ -83,6 +91,11 @@ def to_dict(self) -> Dict[str, Any]: # Only when there are any, so a transcription nobody has touched # exports exactly as it did before this was recorded. **({"edited": sorted(self.edited_words)} if self.edited_words else {}), + **( + {"highlighted": sorted(self.highlighted_words)} + if self.highlighted_words + else {} + ), } def to_srt_format(self) -> str: diff --git a/utils/srt.py b/utils/srt.py index 7f2a08f..63ed358 100644 --- a/utils/srt.py +++ b/utils/srt.py @@ -683,6 +683,16 @@ def parse_txt(self, data: dict) -> None: position for position in marks if isinstance(position, int) } + # The same for the reader's highlights. + highlights = segment.get("highlighted") + + if isinstance(highlights, list): + restored.highlighted_words = { + position + for position in highlights + if isinstance(position, int) and not isinstance(position, bool) + } + self.captions.append(restored) self.speakers.add(segment.get("speaker", "")) @@ -956,9 +966,7 @@ def split_caption( if text is not None and text != caption.text: # The text area holds edits that have not been committed yet; # splitting must not discard them, nor the marks they earned. - caption.edited_words = self.retag_edits( - caption.text, text, caption.edited_words - ) + self.carry_marks(caption, text) caption.text = text self.mark_as_changed() @@ -983,10 +991,14 @@ def split_caption( # Marks follow their words across the break. kept, moved = self.split_edits(caption.edited_words, len(first_part.split())) + kept_highlights, moved_highlights = self.split_edits( + caption.highlighted_words, len(first_part.split()) + ) # Update first caption caption.text = first_part caption.edited_words = kept + caption.highlighted_words = kept_highlights caption.end_time = self.seconds_to_timestamp(mid_seconds) # Create second caption @@ -998,6 +1010,7 @@ def split_caption( speaker=caption.speaker, ) new_caption.edited_words = moved + new_caption.highlighted_words = moved_highlights # Insert new caption caption_index = self.captions.index(caption) @@ -1079,9 +1092,21 @@ def move_first_word_to_previous(self, caption: SRTCaption) -> None: self.save_state_for_undo() previous = self.captions[position - 1] + + # A highlight on the word follows it across. Read before either text + # changes, since both positions are counted against the old words. + first_was_highlighted = 0 in caption.highlighted_words + previous_count = len(previous.text.split()) + previous.text = f"{previous.text.rstrip()} {moved_word}" caption.text = remaining + caption.highlighted_words = { + index - 1 for index in caption.highlighted_words if index > 0 + } + if first_was_highlighted: + previous.highlighted_words.add(previous_count) + if self.word_is_timed(moved_timing) and self.word_is_timed(next_timing): previous.end_time = self.seconds_to_timestamp(moved_timing["e"]) caption.start_time = self.seconds_to_timestamp(next_timing["s"]) @@ -1136,9 +1161,21 @@ def move_last_word_to_next(self, caption: SRTCaption) -> None: self.save_state_for_undo() following = self.captions[position + 1] + + # See move_first_word_to_previous: the highlight goes with the word. + last_index = len(caption.text.split()) - 1 + last_was_highlighted = last_index in caption.highlighted_words + following.text = f"{moved_word} {following.text.lstrip()}" caption.text = remaining + caption.highlighted_words.discard(last_index) + following.highlighted_words = { + index + 1 for index in following.highlighted_words + } + if last_was_highlighted: + following.highlighted_words.add(0) + if self.word_is_timed(moved_timing) and self.word_is_timed(previous_timing): following.start_time = self.seconds_to_timestamp(moved_timing["s"]) caption.end_time = self.seconds_to_timestamp(previous_timing["e"]) @@ -1161,6 +1198,66 @@ def finish_word_move(self) -> None: self.update_words_per_minute() self.refresh_display(force_full_refresh=True) + def toggle_highlight(self, selections: list) -> str: + """ + Highlight the words a selection touches, or take the highlight off. + + `selections` is a list of (caption, start, end) with the offsets in the + caption's text, one entry per caption the selection reaches into. A + selection that reaches across blocks is one gesture and one undo step. + + One key does both, so which it is depends on what is there: when every + word taken is already highlighted the highlight comes off, otherwise + everything taken gets one. A mixed selection therefore becomes fully + highlighted first -- the reader can press again to clear it. + + Returns "added", "removed", or "" when nothing was selected (a notice + says so). + + Transcriptions only, for now. A set of subtitles is saved as plain + SRT, which has nowhere to keep a highlight, so it gets none: nothing + in the editor asks for one there, and this refuses if something does. + """ + + if self.data_format == "srt": + return "" + + touched = [] + + for caption, start, end in selections: + positions = self.words_in_selection(caption.text, start, end) + + if positions: + touched.append((caption, positions)) + + if not touched: + ui.notify( + "Select the words to highlight first", + type="warning", + timeout=None, + close_button="Close", + ) + return "" + + remove = all( + position in caption.highlighted_words + for caption, positions in touched + for position in positions + ) + + self.save_state_for_undo() + + for caption, positions in touched: + if remove: + caption.highlighted_words.difference_update(positions) + else: + caption.highlighted_words.update(positions) + + # Kept with the transcription's own data, so it is a change to save. + self.mark_as_changed() + + return "removed" if remove else "added" + def add_caption_after(self, caption: SRTCaption) -> None: """ Add a new caption after the selected one. @@ -1270,9 +1367,7 @@ def update_caption_text( self.save_state_for_undo() # Worked out before the text is replaced, while there is still # something to compare it against. - caption.edited_words = self.retag_edits( - caption.text, new_text, caption.edited_words - ) + self.carry_marks(caption, new_text) caption.text = new_text # Editing a word can take it off the count, or put one on it. self.update_flagged_count() @@ -1327,14 +1422,21 @@ def delete_range( # retag_edits does with words a change brought in. old_text = first.text old_marks = set(first.edited_words) + old_highlights = set(first.highlighted_words) for caption in self.captions[start + 1 : end + 1]: old_marks = self.joined_edits( old_marks, caption.edited_words, len(old_text.split()) ) + old_highlights = self.joined_edits( + old_highlights, caption.highlighted_words, len(old_text.split()) + ) old_text = "\n".join(part for part in (old_text, caption.text) if part) first.edited_words = self.retag_edits(old_text, merged, old_marks) + first.highlighted_words = self.carry_highlights( + old_text, merged, old_highlights + ) first.text = merged first.end_time = last.end_time @@ -1378,6 +1480,11 @@ def merge_with_next(self, caption: SRTCaption) -> None: next_caption.edited_words, len(caption.text.split()), ) + caption.highlighted_words = self.joined_edits( + caption.highlighted_words, + next_caption.highlighted_words, + len(caption.text.split()), + ) # Only actually joined with a newline when both sides have text -- # an empty caption (freshly added, then merged straight back away # with Backspace) has nothing to separate from the other, and a @@ -1426,6 +1533,11 @@ def merge_with_previous(self, caption: SRTCaption) -> None: caption.edited_words, len(previous_caption.text.split()), ) + previous_caption.highlighted_words = self.joined_edits( + previous_caption.highlighted_words, + caption.highlighted_words, + len(previous_caption.text.split()), + ) # See the equivalent join in merge_with_next for why this is not # always a bare "\n" join. previous_caption.text = "\n".join( diff --git a/utils/srt_export.py b/utils/srt_export.py index d995c8b..8bf590d 100644 --- a/utils/srt_export.py +++ b/utils/srt_export.py @@ -160,6 +160,29 @@ class ExportMixin: Export formats and the export dialog. """ + def rtf_marks_available(self, others: list | None = None) -> tuple: + """ + Whether there is anything for the RTF export to carry: (highlights, + changed words). Looked at across this editor and any others exported + along with it, so a bulk export offers a choice if any file has one. + + Transcriptions only, for now: a set of subtitles does not carry its + marks into an export. + """ + + editors = [self] + list(others or []) + captions = [ + caption + for editor in editors + if editor.data_format != "srt" + for caption in editor.captions + ] + + return ( + any(caption.highlighted_words for caption in captions), + any(caption.edited_words for caption in captions), + ) + def export_rtf( self, speakers: bool, @@ -167,11 +190,24 @@ def export_rtf( block_nr: bool, ts_which: str = "both", ts_fmt: str = "srt", + highlights: bool = False, + edits: bool = False, ) -> str: """ Export captions to RTF format with proper Unicode handling. + + `highlights` carries the reader's highlights as a yellow marker and + `edits` carries the words they changed, underlined and in the teal of + the editor's own marking -- underlined as well because a printout + may have no colour. Both are off by default, which is the clean + transcript: nothing about the text itself changes either way. + + Transcriptions only, for now: subtitles never carry them. """ + if self.data_format == "srt": + highlights = edits = False + def fmt_ts(ts: str, f: str) -> str: """ Format timestamp according to selected format. @@ -210,8 +246,20 @@ def to_rtf_unicode(text: str) -> str: ) return "".join(result) + # Colour 1 is the marker (pure yellow: it is what Word's own highlighter + # palette holds, so it is shown as drawn and not matched to the nearest + # colour), colour 2 the editor's teal for changed words. The table is + # only written when it is used, so a clean export is the document it + # always was. + colours = ( + r"{\colortbl;\red255\green255\blue0;\red18\green121\blue106;}" + if highlights or edits + else "" + ) rtf_content = ( - r"{\rtf1\ansi\deff0{\fonttbl{\f0 Arial;}}" r"\viewkind4\uc1\pard\f0\fs20 " + r"{\rtf1\ansi\deff0{\fonttbl{\f0 Arial;}}" + + colours + + r"\viewkind4\uc1\pard\f0\fs20 " ) parts = [] @@ -238,9 +286,24 @@ def to_rtf_unicode(text: str) -> str: if header_parts: rtf_data = r"\b " + " ".join(header_parts) + r"\b0\line " - rtf_data += ( - to_rtf_unicode(caption.text).replace("\n", r"\line ") + r"\line\line " - ) + for text, highlighted, edited in self.marked_spans( + caption, highlights, edits + ): + body = to_rtf_unicode(text).replace("\n", r"\line ") + + if highlighted or edited: + # A group, so the formatting ends with the piece. The one + # space after a control word is its delimiter and is not + # part of the text, so a piece that starts with a space + # keeps it. + style = (r"\highlight1" if highlighted else "") + ( + r"\cf2\ul" if edited else "" + ) + body = "{" + style + " " + body + "}" + + rtf_data += body + + rtf_data += r"\line\line " parts.append(rtf_data) @@ -542,6 +605,23 @@ def _close_and_delete() -> None: rtf_idx_incl = ui.checkbox( "Include paragraph numbers", value=False ) + # Off by default: an export is clean unless + # it is asked to carry the marks. And only + # offered for a kind of mark that exists in + # what is being exported -- a choice about + # nothing is just clutter. Transcriptions + # only, for now (see rtf_marks_available). + has_highlights, has_edits = self.rtf_marks_available( + [editor for _, editor in bulk_editors or []] + ) + rtf_hl_incl = ui.checkbox( + "Include highlights", value=False + ) + rtf_hl_incl.set_visibility(has_highlights) + rtf_edits_incl = ui.checkbox( + "Include my edits (underlined)", value=False + ) + rtf_edits_incl.set_visibility(has_edits) ui.separator() # CSV options (for csv only) @@ -1007,6 +1087,14 @@ def export_one(editor): rtf_idx_incl.value, ts_which.value, ts_fmt.value, + highlights=bool( + rtf_hl_incl.visible + and rtf_hl_incl.value + ), + edits=bool( + rtf_edits_incl.visible + and rtf_edits_incl.value + ), ) elif fmt.value == "txt": parts = [] diff --git a/utils/srt_render.py b/utils/srt_render.py index 34afae6..a91cf4a 100644 --- a/utils/srt_render.py +++ b/utils/srt_render.py @@ -712,8 +712,19 @@ def keyboard_shortcut_groups(self) -> list: ] ) + # The selection's own highlight. A transcription's alone, for now: a + # set of subtitles is plain SRT, which has nowhere to keep it. + highlighting = ( + [] + if subtitles + else [ + ("Highlight selection, or remove the highlight", "Ctrl + Shift + H"), + ] + ) + shortcut_groups = [ ("Editing", editing), + *([("Highlighting", highlighting)] if highlighting else []), ( "File operations", [ diff --git a/utils/srt_review.py b/utils/srt_review.py index e224be6..1e0600d 100644 --- a/utils/srt_review.py +++ b/utils/srt_review.py @@ -347,6 +347,178 @@ def joined_edits(first: set, second: set, offset: int) -> set: return set(first) | {index + offset for index in second} + @staticmethod + def carry_highlights(previous: str, current: str, marked: set) -> set: + """ + Carry a caption's highlights across a change to its text. + + Unlike an edit mark, a highlight is the reader's note about a passage + and not about who wrote it, so a change does not earn one: typing a + new word somewhere does not highlight it. The reader's marks do have + to survive the corrections made inside a highlighted passage, though, + or fixing a typo would quietly take the highlight away. + + - Words that stay move with their marks, however far they shift. + - Words replaced one for one keep the mark of the word they replace. + - A longer or shorter replacement is highlighted when everything it + replaced was. + - Words inserted between two highlighted words are inside the + highlight and take it; anywhere else they do not. + - Words that are gone take their highlight with them. + """ + + before = previous.split() + after = current.split() + carried: set = set() + + matcher = SequenceMatcher(None, before, after, autojunk=False) + + for tag, before_start, before_end, after_start, after_end in ( + matcher.get_opcodes() + ): + if tag == "equal": + for offset in range(before_end - before_start): + if before_start + offset in marked: + carried.add(after_start + offset) + elif tag == "replace": + old = range(before_start, before_end) + new = range(after_start, after_end) + + if len(old) == len(new): + carried.update( + after_start + offset + for offset, index in enumerate(old) + if index in marked + ) + elif all(index in marked for index in old): + carried.update(new) + elif tag == "insert": + if before_start - 1 in marked and before_start in marked: + carried.update(range(after_start, after_end)) + + return carried + + def marked_spans( + self, caption: SRTCaption, highlights: bool, edits: bool + ) -> list: + """ + A caption's text as (text, highlighted, edited) pieces, for a format + that draws the marks itself -- the RTF export. + + Not the editor's own runs (review_runs): those follow the switches + under the video, while an export is told what to carry. A piece's + text is whole words and the space between them, so concatenating the + pieces gives the caption's text back exactly. + + The space between two marked words is marked with them, so a passage + is one stretch and not a row of words, as it is drawn in the editor. + A line break is left outside the marking, for the same reason as + there. + """ + + tokens = [token for token in re.split(r"(\s+)", caption.text) if token] + position = -1 + positions: list = [] + + for token in tokens: + if token.strip(): + position += 1 + positions.append(position) + else: + positions.append(None) + + def flags(at: int) -> tuple: + word = positions[at] + + if word is not None: + return ( + highlights and self.token_is_highlight(caption, word), + edits and self.token_is_edit(caption, word), + ) + + before = positions[at - 1] if at > 0 else None + after = positions[at + 1] if at + 1 < len(tokens) else None + + if before is None or after is None or "\n" in tokens[at]: + return (False, False) + + return ( + highlights + and self.token_is_highlight(caption, before) + and self.token_is_highlight(caption, after), + edits + and self.token_is_edit(caption, before) + and self.token_is_edit(caption, after), + ) + + pieces: list = [] + + for at, token in enumerate(tokens): + highlighted, edited = flags(at) + + if pieces and pieces[-1][1:] == (highlighted, edited): + pieces[-1] = (pieces[-1][0] + token, highlighted, edited) + else: + pieces.append((token, highlighted, edited)) + + return pieces + + def carry_marks(self, caption: SRTCaption, new_text: str) -> None: + """ + Carry both kinds of mark on a caption across a change to its text. + + Called before the text itself is replaced, while there is still + something to compare the new text against. + """ + + caption.edited_words = self.retag_edits( + caption.text, new_text, caption.edited_words + ) + caption.highlighted_words = self.carry_highlights( + caption.text, new_text, caption.highlighted_words + ) + + @staticmethod + def words_in_selection(text: str, start: int, end: int) -> list: + """ + The positions of the words a selection touches, counted in + whitespace separated words from the start of the text. + + A word is touched when the selection takes any of it, so a selection + that ends part way through a word takes the whole word: marking a + passage never cuts a word in two. A selection that only brushes a + word's edge, ending where it begins or starting where it ends, does + not touch it. + + Nothing selected means the word the caret is in or against: either + edge counts, since the caret is as often just after a word as in it, + and no two words share an edge because whitespace separates them. A + caret in the space between words touches neither. + """ + + if end < start: + start, end = end, start + + touched = [] + + for position, match in enumerate(re.finditer(r"\S+", text)): + if start == end: + if match.start() <= start <= match.end(): + touched.append(position) + elif match.start() < end and match.end() > start: + touched.append(position) + + return touched + + @staticmethod + def token_is_highlight(caption: SRTCaption, index: int) -> bool: + """ + Whether the reader has highlighted the word at this position. + """ + + return index in caption.highlighted_words + + def speech_runs(self, gap: float = 0.3) -> List[list]: """ Where speech actually is, as [start, end] pairs. @@ -668,17 +840,21 @@ def review_runs( if not source: return [] + highlighted = caption.highlighted_words + if ( not self.show_uncertain_words and not self.show_my_edits and not per_word + and not highlighted ): return [{"t": source, "flag": False}] # A word has to be a run of its own before it can be marked at all, so # marking the reader's words needs the same split that following the - # audio does. - one_run_per_word = per_word or self.show_my_edits + # audio does. So does a highlight, which is always drawn: it is not one + # of the switches under the video. + one_run_per_word = per_word or self.show_my_edits or bool(highlighted) words = self.aligned_words(caption, source) runs: list = [] @@ -690,12 +866,42 @@ def flush() -> None: runs.append({"t": "".join(plain), "flag": False}) plain.clear() - for token in re.split(r"(\s+)", source): - if not token: - continue + tokens = [token for token in re.split(r"(\s+)", source) if token] + + def is_highlighted(position: int) -> bool: + return self.token_is_highlight(caption, position) + + # Which word each token is, so that the space between two highlighted + # words can be drawn highlighted too: a passage reads as one marked + # stretch, not a row of separate words. + word_position = -1 + positions: list = [] + for token in tokens: + if token.strip(): + word_position += 1 + positions.append(word_position) + else: + positions.append(None) + + for at, token in enumerate(tokens): if not token.strip(): - plain.append(token) + before = positions[at - 1] if at > 0 else None + after = positions[at + 1] if at + 1 < len(tokens) else None + + # A line break is left unmarked: a highlight running out to the + # edge of the block would draw a stub at the end of the line. + if ( + "\n" not in token + and before is not None + and after is not None + and is_highlighted(before) + and is_highlighted(after) + ): + flush() + runs.append({"t": token, "flag": False, "hl": True}) + else: + plain.append(token) continue word = words[index] if index < len(words) else None @@ -704,14 +910,19 @@ def flush() -> None: # be honoured here too or words stay marked with them switched off. flagged = self.show_uncertain_words and self.word_needs_review(word) edited = self.show_my_edits and self.token_is_edit(caption, index - 1) + marked = is_highlighted(index - 1) - if not one_run_per_word and not flagged and not edited: + if not one_run_per_word and not flagged and not edited and not marked: plain.append(token) continue flush() run = {"t": token, "flag": flagged} + # Only carried when true, like "edit". + if marked: + run["hl"] = True + # Only carried when true, so a run reads the same as it always did # wherever nothing has been edited. if edited: @@ -719,7 +930,8 @@ def flush() -> None: # Only a word still matching what the model transcribed has a # timing we can attribute to it; an edited word has none, and nor - # has a word transcribed without one. Neither is ever highlighted. + # has a word transcribed without one. Neither is ever lit up as the + # word being played. if per_word and self.word_is_timed(word): run["s"] = word["s"] run["e"] = word["e"] diff --git a/utils/srt_search.py b/utils/srt_search.py index 95a27b7..f029ba0 100644 --- a/utils/srt_search.py +++ b/utils/srt_search.py @@ -127,11 +127,7 @@ def replace_in_current_caption(self, replacement: str) -> None: # every mark after it along. Worked out here rather than through # update_caption_text, which would take a second undo snapshot on # top of the one already taken above. - self.selected_caption.edited_words = self.retag_edits( - self.selected_caption.text, - new_text, - self.selected_caption.edited_words, - ) + self.carry_marks(self.selected_caption, new_text) self.selected_caption.text = new_text self.update_flagged_count() self.refresh_display() @@ -171,9 +167,7 @@ def replace_all(self, replacement: str) -> None: # of the reader's, and a replacement of a different length # moves every mark after it. One undo snapshot covers the whole # Replace All, so this cannot go through update_caption_text. - caption.edited_words = self.retag_edits( - caption.text, new_text, caption.edited_words - ) + self.carry_marks(caption, new_text) caption.text = new_text count += 1 diff --git a/utils/styles.py b/utils/styles.py index 04fadfe..857a83c 100644 --- a/utils/styles.py +++ b/utils/styles.py @@ -127,6 +127,15 @@ --color-edit-accent: #12796a; --color-edit-text: #10453d; + /* Words the reader has highlighted for themselves. The one marking + that is yellow, as a highlighter is. Yellow on white is barely + brighter than the page (1.14:1) so the colour cannot carry it + alone: the frame lines carry it, 4.9:1 against the page (1.4.1, + 1.4.11), and the text on the yellow measures 13.6:1. */ + --color-highlight-bg: #fff1a3; + --color-highlight-frame: #8a6d00; + --color-highlight-text: #2b2400; + /* The word currently being spoken. A pale tint of the brand navy, so it reads as "you are here" rather than as a status. */ --color-playing-bg: #dbe4f0; @@ -252,6 +261,12 @@ --color-edit-accent: #6fd3c1; --color-edit-text: #dff5f0; + /* Dark mode keeps the yellow as a dark gold. Text on it 8.0:1, the + frame 12.6:1 against the surface. */ + --color-highlight-bg: #5a4a00; + --color-highlight-frame: #ffd54f; + --color-highlight-text: #fff6cc; + --color-playing-bg: #1c3252; --color-header-bg: #16181d; @@ -1372,6 +1387,56 @@ box-shadow: inset 0 -2px 0 var(--color-edit-accent); } + /* Words the reader has highlighted. Yellow, like a highlighter, with a + line above and below so it does not depend on the colour: it is a + different shape from the other two markings, which are a wash with a + bar underneath. No horizontal padding, unlike them, so highlighting + a passage does not move the text -- and a run of highlighted words + with the spaces between them reads as one stretch. + + On a flagged or changed word the highlight takes the wash and the text + colour, and the word keeps its own bar underneath, so both can be seen. + While the word is being played the playing tint wins the wash but the + lines stay, so the passage can still be seen under the playback. */ + .highlight-word { + padding: 1px 0; + color: var(--color-highlight-text); + background-color: var(--color-highlight-bg); + box-shadow: + inset 0 2px 0 var(--color-highlight-frame), + inset 0 -2px 0 var(--color-highlight-frame); + box-decoration-break: clone; + -webkit-box-decoration-break: clone; + } + .highlight-word.review-word { + box-shadow: + inset 0 2px 0 var(--color-highlight-frame), + inset 0 -2px 0 var(--color-review-accent); + } + .highlight-word.edit-word { + box-shadow: + inset 0 2px 0 var(--color-highlight-frame), + inset 0 -2px 0 var(--color-edit-accent); + } + .transcript-text .highlight-word[data-current] { + background-color: var(--color-playing-bg); + color: inherit; + border-radius: 0; + box-shadow: + inset 0 2px 0 var(--color-highlight-frame), + inset 0 -2px 0 var(--color-highlight-frame); + } + .transcript-text .highlight-word.review-word[data-current] { + box-shadow: + inset 0 2px 0 var(--color-highlight-frame), + inset 0 -2px 0 var(--color-review-accent); + } + .transcript-text .highlight-word.edit-word[data-current] { + box-shadow: + inset 0 2px 0 var(--color-highlight-frame), + inset 0 -2px 0 var(--color-edit-accent); + } + /* Shared hover message. A CSS box rather than title=, which the browser draws itself and stylesheets cannot reach. diff --git a/utils/transcript_editor.js b/utils/transcript_editor.js index 814e30f..6e4a8e9 100644 --- a/utils/transcript_editor.js +++ b/utils/transcript_editor.js @@ -205,11 +205,11 @@ export default { >Delete caption
{{ run.t }}
{{ run.t }}
{{ run.t }}
+ >{{ run.t }}
+ `, props: { @@ -452,6 +497,23 @@ export default { stamps: {}, timed: [], menu: { open: false, x: 0, y: 0, id: null, speaker: null }, + // The comment box. peek is a box opened by a click on commented text: + // it shows the comment without taking focus from the text, and goes + // away at the next key or click. Opened from the key or the button it + // takes focus, and keeps what is typed until it is saved or cancelled. + comment: { + open: false, + x: 0, + y: 0, + id: null, + start: 0, + end: 0, + text: "", + original: "", + quote: "", + existing: false, + peek: false, + }, // The block the caret is in, so the caption being edited can look // like it. Purely presentational, and separate from activeId, which // is the caption the recording is playing -- the reader is very @@ -493,6 +555,7 @@ export default { this.onTime = () => this.markCurrentWord(); this.onOutside = (event) => { if (!event.target.closest(".speaker-menu")) this.closeMenu(); + if (!event.target.closest(".comment-box")) this.leaveComment(); }; this.onEscape = (event) => { if (event.key === "Escape") this.closeMenu(true); @@ -616,6 +679,8 @@ export default { return [ run.flag ? "review-word" : run.edit ? "edit-word" : null, run.hl ? "highlight-word" : null, + run.cm !== undefined ? "comment-word" : null, + run.cl ? "comment-end" : null, ]; }, @@ -668,6 +733,163 @@ export default { this.$emit("togglehighlight", { spans: this.highlightSpans() }); }, + // The toolbar's Comment button, and the key: ask the server to open the + // comment on the selected words. The server decides what that means -- + // a new comment, or the one the words already have -- and answers with + // showComment. + commentSelection() { + this.flush(); + this.$emit("opencomment", { spans: this.highlightSpans() }); + }, + + // Open the comment box beside a stretch of a block's text. `payload` is + // what the server worked out: the block, the character range the comment + // is on, its text so far, whether it already exists, and whether the box + // should take focus (not when it was only clicked on). + showComment(payload) { + this.$nextTick(() => { + const block = this.$refs.body?.querySelector( + `.transcript-text[data-id="${payload.id}"]` + ); + if (!block) return; + + const from = this.pointAt(block, payload.start); + const to = this.pointAt(block, payload.end); + if (!from || !to) return; + + const range = document.createRange(); + range.setStart(from.node, from.offset); + range.setEnd(to.node, to.offset); + + // Under the last line of the words, so the box does not cover them. + const rects = range.getClientRects(); + const box = rects.length ? rects[rects.length - 1] : range.getBoundingClientRect(); + const root = this.$el.getBoundingClientRect(); + + // Estimates, only used to keep the box on screen: its width follows + // the stylesheet, and its height is a few lines of text and a row of + // buttons. + const width = 304; + const height = 190; + const gap = 6; + const edge = 8; + + const x = Math.max(edge, Math.min(box.left, window.innerWidth - edge - width)); + let y = box.bottom + gap; + if (y + height > window.innerHeight - edge) { + y = Math.max(edge, box.top - gap - height); + } + + const quote = this.plainText(block.textContent).slice(payload.start, payload.end); + + this.comment = { + open: true, + x: x - root.left, + y: y - root.top, + id: payload.id, + start: payload.start, + end: payload.end, + text: payload.text, + original: payload.text, + quote: quote.length > 60 ? quote.slice(0, 59) + "\u2026" : quote, + existing: !!payload.existing, + peek: !payload.focus, + }; + + if (payload.focus) { + this.$nextTick(() => { + const area = this.$refs.commentText; + if (!area) return; + area.focus(); + area.setSelectionRange(area.value.length, area.value.length); + }); + } + }); + }, + + closeComment() { + this.comment = { ...this.comment, open: false, peek: false }; + }, + + // Put the reader's selection back on the words, the way it was when the + // box opened, so they carry on from where they were. + returnToComment() { + const { id, start, end } = this.comment; + this.selectRange(id, start, id, end); + }, + + saveComment() { + const text = this.comment.text.trim(); + if (!this.comment.open || !text) return; + + const { id, start, end } = this.comment; + this.returnToComment(); + this.closeComment(); + this.$emit("savecomment", { id, start, end, text }); + }, + + removeComment() { + if (!this.comment.open) return; + + const { id, start, end } = this.comment; + this.returnToComment(); + this.closeComment(); + this.$emit("removecomment", { id, start, end }); + }, + + cancelComment() { + if (!this.comment.open) return; + + this.returnToComment(); + this.closeComment(); + }, + + // The reader moved on without choosing: a click elsewhere, or tabbing out + // of the box. What they typed is not thrown away -- it is saved, if there + // is something to save and it is not what was there. Escape, and Cancel, + // are how to abandon it. A peek has nothing typed in it to save. + leaveComment() { + if (!this.comment.open) return; + + const text = this.comment.text.trim(); + + if (!this.comment.peek && text && text !== this.comment.original.trim()) { + const { id, start, end } = this.comment; + this.closeComment(); + // stay: the reader has already put the caret somewhere else, and the + // redraw must not take it back to these words. + this.$emit("savecomment", { id, start, end, text, stay: true }); + return; + } + + this.closeComment(); + }, + + // Every key typed in the box stays in it: the page has shortcuts of its + // own (undo, save, play) that would otherwise act on the transcript while + // a sentence is being written. Escape abandons the comment and + // Ctrl/Cmd+Enter saves it. + onCommentKeydown(event) { + event.stopPropagation(); + + if (event.key === "Escape") { + event.preventDefault(); + this.cancelComment(); + } else if (event.key === "Enter" && (event.ctrlKey || event.metaKey)) { + event.preventDefault(); + this.saveComment(); + } + }, + + onCommentFocusOut(event) { + const next = event.relatedTarget; + if (next && event.currentTarget.contains(next)) return; + // Focus leaving the box for nowhere in particular (the window losing + // focus, say) is not the reader choosing anything. + if (!next) return; + this.leaveComment(); + }, + // The text node and offset a block's character offset falls at. An // offset past the end gives null, for the caller to treat as "the end". pointAt(block, offset) { @@ -1844,6 +2066,27 @@ export default { } } + // Comment on the selection, or open the comment the words already + // have. Ctrl only and by physical key, for the reasons given for the + // highlight above. Transcriptions only, for now. + if ( + !this.subtitleMode && + event.ctrlKey && + event.shiftKey && + !event.altKey && + !event.metaKey && + event.code === "KeyK" + ) { + const spans = this.highlightSpans(); + if (spans.length > 0) { + return this.report(event, "opencomment", { spans }); + } + } + + // A box opened by a click was only there to be read; the reader typing + // on has done with it. + if (this.comment.open && this.comment.peek) this.closeComment(); + const at = this.caret(); if (!at) return; @@ -2066,6 +2309,22 @@ export default { const block = this.blockOf(event.target); if (!block) return; + // A click on commented words shows the comment beside them. The server + // knows what it says and answers with showComment; nothing here changes + // where the caret is. + // Not at the end of a drag: someone selecting words to copy them is not + // asking to read the note. + if ( + event.target.closest("[data-comment]") && + !this.subtitleMode && + window.getSelection()?.isCollapsed + ) { + const where = this.caret(); + if (where && where.id === Number(block.dataset.id)) { + this.$emit("peekcomment", { id: where.id, offset: where.offset }); + } + } + // Where in the block the caret landed, so the recording can be moved to // that word rather than to the start of the block. The browser places // the caret on mousedown, so it is already correct by the time click diff --git a/utils/transcript_editor.py b/utils/transcript_editor.py index 3a1e51b..507618a 100644 --- a/utils/transcript_editor.py +++ b/utils/transcript_editor.py @@ -191,6 +191,12 @@ def select_range( ) -> None: self.run_method("selectRange", start_id, start_offset, end_id, end_offset) + def comment_selection(self) -> None: + self.run_method("commentSelection") + + def show_comment(self, payload: dict) -> None: + self.run_method("showComment", payload) + def focus_caret(self) -> None: self.run_method("focusCaret") @@ -593,6 +599,10 @@ def build(self) -> None: self.body.on( "togglehighlight", lambda event: self.toggle_highlight(event.args) ) + self.body.on("opencomment", lambda event: self.open_comment(event.args)) + self.body.on("peekcomment", lambda event: self.peek_comment(event.args)) + self.body.on("savecomment", lambda event: self.save_comment(event.args)) + self.body.on("removecomment", lambda event: self.remove_comment(event.args)) self.body.on("addblock", lambda event: self.add_after(event.args)) self.body.on("deleteblock", lambda event: self.delete(event.args)) self.body.on("blockclick", lambda event: self.seek(event.args)) @@ -1001,6 +1011,199 @@ def toggle_highlight(self, args) -> None: first, last = spans[0], spans[-1] self.body.select_range(first[0], first[1], last[0], last[2]) + def comment_selection(self) -> None: + """ + What the toolbar's Comment button does: ask the component for the + selection, which it reports as it does for the key. + """ + + if self.body is not None: + self.body.comment_selection() + + def tell_to_select_words(self) -> None: + ui.notify( + "Select the words to comment on first", + type="warning", + timeout=None, + close_button="Close", + ) + + def show_comment_on( + self, caption: SRTCaption, start: int, end: int, focus: bool + ) -> bool: + """ + Open the comment box for the words around a selection. Says whether + there was anything to open it for. + """ + + target = self.editor.comment_target(caption, start, end) + + if target is None or self.body is None: + return False + + self.body.show_comment( + { + "id": caption.index, + "start": target["start"], + "end": target["end"], + "text": target["text"], + "existing": target["index"] is not None, + "focus": focus, + } + ) + + return True + + def open_comment(self, args) -> None: + """ + The key or the button: comment on the selected words, or open the + comment they already have. + + A comment belongs to one block, so a selection reaching across blocks + is refused. A selection that merely ends at the very edge of the next + block, taking none of it, is still within one. + """ + + if self.editor.data_format == "srt": + return + + raw = args.get("spans") if isinstance(args, dict) else None + + if not isinstance(raw, list): + return + + if not raw: + self.tell_to_select_words() + return + + taken = [] + + for item in raw: + if not isinstance(item, dict): + continue + + caption = self.caption(self.block_id(item)) + start, end = item.get("start"), item.get("end") + + if ( + caption is None + or isinstance(start, bool) + or isinstance(end, bool) + or not isinstance(start, int) + or not isinstance(end, int) + ): + continue + + taken.append((caption, start, end)) + + # Blocks the selection only brushes, with nothing taken from them, do + # not count -- unless that is all there is, which is a caret. + reaching = [entry for entry in taken if entry[1] != entry[2]] + + if len(reaching) > 1: + ui.notify( + "A comment has to stay within one paragraph", + type="warning", + timeout=None, + close_button="Close", + ) + return + + if not taken: + return + + caption, start, end = (reaching or taken)[0] + + if not self.show_comment_on(caption, start, end, focus=True): + self.tell_to_select_words() + + def peek_comment(self, args) -> None: + """ + A click on commented words: show the comment beside them, without + taking focus from the text. Nothing for a click anywhere else. + """ + + if self.editor.data_format == "srt": + return + + caption = self.caption(self.block_id(args)) + offset = args.get("offset") if isinstance(args, dict) else None + + if caption is None or isinstance(offset, bool) or not isinstance(offset, int): + return + + target = self.editor.comment_target(caption, offset, offset) + + if target is None or target["index"] is None: + return + + self.show_comment_on(caption, offset, offset, focus=False) + + def comment_args(self, args): + """ + A comment event's block and range, checked, or None. + """ + + caption = self.caption(self.block_id(args)) + start = args.get("start") if isinstance(args, dict) else None + end = args.get("end") if isinstance(args, dict) else None + + if ( + caption is None + or isinstance(start, bool) + or isinstance(end, bool) + or not isinstance(start, int) + or not isinstance(end, int) + ): + return None + + return caption, start, end + + def save_comment(self, args) -> None: + """ + Keep the text the reader wrote as the comment on those words. + """ + + checked = self.comment_args(args) + text = args.get("text") if isinstance(args, dict) else None + + if checked is None or not isinstance(text, str): + return + + caption, start, end = checked + + if not self.editor.set_comment(caption, start, end, text): + return + + self.refresh() + self.changed() + + # Offsets are in the text, which a comment does not change, so the + # selection can be put back as it was. + if self.body is not None and not args.get("stay"): + self.body.select_range(caption.index, start, caption.index, end) + + def remove_comment(self, args) -> None: + """ + Take the comment off the words. + """ + + checked = self.comment_args(args) + + if checked is None: + return + + caption, start, end = checked + + if not self.editor.remove_comment(caption, start, end): + return + + self.refresh() + self.changed() + + if self.body is not None: + self.body.select_range(caption.index, start, caption.index, end) + def move_word(self, args) -> None: """ Hand one word across the boundary to the caption either side. From 4320c4256e7516f27e7f06dfc421d7ae5a6d53f9 Mon Sep 17 00:00:00 2001 From: erklu Date: Sun, 18 Oct 2026 08:26:46 +0000 Subject: [PATCH 04/13] Test that comments travel with the undo history Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01LCjpD3ehigLiUGH1oVQQAW --- tests/test_comments.py | 50 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/test_comments.py b/tests/test_comments.py index 81fd7df..b68bc0c 100644 --- a/tests/test_comments.py +++ b/tests/test_comments.py @@ -505,6 +505,56 @@ def test_the_marks_do_not_get_mixed_up(self, editor): assert shape(first) == [([3], "c")] +class TestUndo: + """ + With the real history, not the stand-in the other tests use: a comment + has to travel with the snapshots. + """ + + @pytest.fixture + def real(self, editor): + del editor.save_state_for_undo + editor._update_undo_redo_buttons = lambda: None + editor.update_beforeunload_state = lambda: None + + return editor + + def state(self, editor): + first = editor.captions[0] + + return first.text, shape(first) + + def test_adding_one_can_be_undone_and_redone(self, real): + real.set_comment(real.captions[0], 0, 7, "note") + + real.undo() + assert self.state(real) == ("ett två tre", []) + + real.redo() + assert self.state(real) == ("ett två tre", [([0, 1], "note")]) + + def test_deleting_the_words_and_undoing_brings_both_back(self, real): + real.set_comment(real.captions[0], 0, 7, "note") + real.update_caption_text(real.captions[0], "tre") + assert self.state(real) == ("tre", []) + + real.undo() + + assert self.state(real) == ("ett två tre", [([0, 1], "note")]) + + def test_changing_the_text_and_removing_can_be_undone(self, real): + first = real.captions[0] + real.set_comment(first, 0, 3, "one") + real.set_comment(first, 0, 3, "two") + real.remove_comment(first, 0, 3) + + real.undo() + assert shape(real.captions[0]) == [([0], "two")] + + real.undo() + assert shape(real.captions[0]) == [([0], "one")] + + class TestDrawing: def runs(self, editor, caption): return editor.review_runs(caption) From f87fdee6f1237b21a4642afde6ff7a7df9fee313 Mon Sep 17 00:00:00 2001 From: erklu Date: Sun, 18 Oct 2026 08:54:48 +0000 Subject: [PATCH 05/13] Show comments in a card under the video instead of a box beside the words (#131) The popover is gone. A comment is now written, read and changed in a card in the right column, built like the validation panel: one comment at a time, shut until needed, with Previous/Next and Close, and in the colour of the underline on the commented words (magenta, because blue is the progress tint). The card follows the caret: put the caret in commented words, or click them, and it shows that comment. An open card follows; a shut one opens only for a click. What is typed is kept when the reader moves on, and abandoned only by Escape or Cancel. The underline is now a dotted 3px line 4px below the text, because a thinner one closer in was lost against the highlight's frame lines on words that are both highlighted and commented. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01LCjpD3ehigLiUGH1oVQQAW --- AGENTS.md | 14 +- pages/srt.py | 10 + tests/test_comments.py | 879 +++++++++++++++++++++++++++---------- utils/comments_panel.py | 602 +++++++++++++++++++++++++ utils/styles.py | 85 ++-- utils/transcript_editor.js | 252 ++--------- utils/transcript_editor.py | 132 +----- 7 files changed, 1361 insertions(+), 613 deletions(-) create mode 100644 utils/comments_panel.py diff --git a/AGENTS.md b/AGENTS.md index 338efa3..9537cb6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -139,13 +139,15 @@ The reader's own highlighter: a note to self made while reviewing against the re A note on a stretch of words, with a text of its own: `SRTCaption.comments`, a list of `WordComment(words, text)` (`utils/caption.py`), the words counted exactly as the highlight's are. **Stacked on the highlights branch**, and it reuses its rules: `CommentsMixin` (`utils/srt_comments.py`) carries each comment's words with `carry_highlights()` -- so the two kinds of note cannot drift apart -- and `carry_marks()` carries comments too, which covers typing, split with unsent text and Find & Replace. Split divides the list (`split_comments`; a comment across the split is on both halves with the same text, as separate copies), merge and `delete_range` join it (`join_comments`; neighbouring comments stay separate notes), and the one-word moves take the word *out of* the comment (`drop_word_from_comments`, `shift_comments`): the comment stays behind. A comment with no words left is gone, with its words. Any new place that rewrites `caption.text` has to go through `carry_marks`. -- **Rules.** A comment belongs to **one caption**: a selection reaching across blocks is refused with a notice (a selection that merely brushes the edge of the next block is not across). A word has **at most one comment**: asking to comment on words that already have one opens that comment (`comment_target()`), never a second over it, so no drawing, navigation or export has to deal with overlap; `restore_comments()` enforces it on read too. Text is stripped, never empty (Save is disabled, `set_comment()` returns "" for it) and at most `COMMENT_MAX_LENGTH` (1000) characters. Transcriptions only for now: the key is not bound in `subtitleMode`, `open_comment`/`peek_comment` and `set_comment`/`remove_comment` refuse for `data_format == "srt"`, the dialog group is absent and the export option is not offered -- the same reason as highlights (SRT has nowhere to keep it). +- **Rules.** A comment belongs to **one caption**: a selection reaching across blocks is refused with a notice (a selection that merely brushes the edge of the next block is not across). A word has **at most one comment**: asking to comment on words that already have one opens that comment (`comment_target()`), never a second over it, so no drawing, navigation or export has to deal with overlap; `restore_comments()` enforces it on read too. Text is stripped, never empty (Save is disabled, `set_comment()` returns "" for it) and at most `COMMENT_MAX_LENGTH` (1000) characters. Transcriptions only for now: the key is not bound in `subtitleMode`, there is no panel for `data_format == "srt"`, `set_comment`/`remove_comment` refuse, the dialog group is absent and the export option is not offered -- the same reason as highlights (SRT has nowhere to keep it). - **Saved** as `"comments": [{"words": [...], "text": "..."}]` per segment, only when there are any. `restore_comments(saved, text)` ignores anything unusable (junk shapes, empty text, non-int or bool or negative or out-of-range positions, a word already taken by an earlier comment) and cuts an over-long text. -- **Opening it.** Ctrl+Shift+K (Ctrl only, by `event.code === "KeyK"`, no Alt/Meta; Chrome and macOS leave it alone) or the toolbar's **Comment** button (`sym_o_add_comment`, beside Highlight, `mousedown` prevented so the selection survives) -> `commentSelection()` -> `opencomment {spans}` -> `TranscriptEditor.open_comment()` works out what it is about (`comment_target`) and answers with `showComment({id, start, end, text, existing, focus})`. A **click on commented words** emits `peekcomment {id, offset}`, answered the same way with `focus: false`: the box is shown without taking focus from the text and goes at the next key or click. -- **The box** is a component of its own inside `transcript_editor.js` (`.comment-box`, `role="dialog"` named "Comment", the text area named "Comment on "), placed against the component's root like the speaker menu. Save (disabled while empty) / Cancel / Remove (only for an existing comment). Ctrl/Cmd+Enter saves, Escape cancels, **every key typed in it is stopped from reaching the page** (otherwise Ctrl+Z would undo the transcript while a sentence is being written, and Ctrl+S would save). Leaving it by a click elsewhere or by Tab **saves what was typed** if it is non-empty and different (`leaveComment()`; sent with `stay: true` so the redraw does not take the caret the reader just put elsewhere); Escape and Cancel abandon. A peek never saves anything. After Save/Cancel/Remove the selection goes back on the words. -- **Drawing.** `review_runs()` gives a commented word `cm` (the comment's index in the caption); the first word also `cf` and `ct` (the text), the last `cl`. The space between two words of the same comment is in it (not across a line break, not between two comments). A word is drawn with `comment-word`: a **dotted 2px underline** in `--color-comment-line` (`#a1136a` light, `#ff9fd2` dark: 6.3-9.4:1 on every wash a word can have, including the highlight, flagged and changed washes and the playing tint). A pattern and not colour alone (1.4.1), drawn with `text-decoration` so it takes no room, touches no box-shadow (the other markings keep theirs) and needs no `::after` -- which the review/edit words already use for a hover message that the editor turns off to protect the caret, and a marker after the last word would sit where the caret goes. The first word has `aria-description="Comment: "`; **not tested with a screen reader**. -- **RTF export** (`export_rtf(..., comments=False)`, an "Include comments" checkbox, **off by default and hidden when there is none**, `rtf_comments_available()`, bulk exports included): each comment is written as a Word comment -- `{\*\atrfstart N}` before its words, `{\*\atrfend N}{\*\atnid SC}{\*\atnauthor Scribe}\chatn{\*\annotation{\*\atnref N}{\*\atndate D}\pard\plain\f0\fs20 text}` after (`rtf_date()` packs the date as RTF wants it). The pieces of `marked_spans()` are cut where a comment begins or ends so a marker never lands inside a piece; the text between the markers is the caption's own text throughout. Author is the constant "Scribe" (the editor does not know the reader's name). Verified by converting an export with LibreOffice (both comments, their ranges, author and date came out right); **Word not yet tried**. A clean export is byte-for-byte unchanged. -- Tests: `tests/test_comments.py`. The component's behaviour (key, box, peek, save/cancel/remove, leaving it, keys not reaching the page) was driven in jsdom against the real component; there is no layout there, so the box's position is checked only as arithmetic. +- **The panel** (`utils/comments_panel.py`, `CommentsPanel`) is where a comment is written, read and changed. It is a card in the right column under the video, built like `ValidationPanel` (same `validation-panel*` classes, `.is-comment` sets `--vp-tone` to the comment colour): a head with Previous / position / Next and Close, then the words (a button that goes to them), "Paragraph N", a text area and Remove / Cancel / Save. **One comment at a time**, shut until needed. `pages/srt.py` builds it for transcriptions only (`transcript.comments_panel`), and `TranscriptEditor.refresh()` ends with `comments_panel.sync()` so a comment that an edit or an undo replaced is found again (same caption index, same words) and one that is gone hides the card. +- **Opening it.** Ctrl+Shift+K (Ctrl only, by `event.code === "KeyK"`, no Alt/Meta) or the toolbar's **Comment** button (`sym_o_add_comment`, beside Highlight, `mousedown` prevented so the selection survives) -> `commentSelection()` -> `opencomment {spans}` -> `TranscriptEditor.open_comment()` -> `CommentsPanel.open_for()`, which shows the existing comment on those words or an empty card for a new one, with focus in the text area. A file that has comments opens with the first one on the card (`open_first()`, no focus, text not moved). +- **The caret drives the card** (the same idea as the validation cards). The component reports `caretcomment {id, cm, click}` **once per comment** when the caret enters one (`reportCommentCaret()`, called from `updateCaretBlock()` and from `onClick` with `click: true`), and `CommentsPanel.follow()` shows it. An open card follows the caret; a **shut** card opens only for a *click*, so an arrow key crossing commented words does not throw a card up under someone typing; the comment already shown is left alone. A selection that is not collapsed reports nothing (a drag to copy words does not open it). +- **What is typed is kept when the reader moves on** (`commit()`): to another comment, into the text, with Previous/Next, or shutting the card -- if it is non-empty and different from what was there. Escape (in the text area) and Cancel are the ways to abandon; Ctrl/Cmd+Enter and Save keep it. Previous/Next are disabled while a new comment is pending. **Keys typed in the text area do not reach the page** (a `keydown` handler with `stopPropagation`; NiceGUI's `ui.keyboard(ignore=[])` is global and listens on the document, so otherwise Ctrl+Z would undo the transcript while a sentence is being written and Ctrl+S would save). After Save, Cancel and Remove the selection goes back on the words (`select_words`). A `sr-only` live label (`spoken`) says "Comment 2 of 5" and the like. +- **Drawing.** `review_runs()` gives a commented word `cm` (the comment's index in the caption); the first word also `cf` and `ct` (the text), the last `cl`. The space between two words of the same comment is in it (not across a line break, not between two comments). A word is drawn with `comment-word`: a **dotted 3px underline, 4px below the text**, in `--color-comment-line` (`#a1136a` light, `#ff9fd2` dark: 6.3-9.4:1 on every wash a word can have, except 4.6:1 on the dark highlight, all above 3:1). A pattern and not colour alone (1.4.1), drawn with `text-decoration` so it takes no room, touches no box-shadow (the other markings keep theirs) and needs no `::after` -- which the review/edit words already use for a hover message that the editor turns off to protect the caret, and a marker after the last word would sit where the caret goes. It is thick and offset because a thin line close to the text was lost against the highlight's own frame lines on words that are both highlighted and commented (rendered in Chromium, both themes, wrapped and not). The comment colour is magenta, not blue: blue is the progress/playing tint. The first word has `aria-description="Comment: "`; **not tested with a screen reader**. +- **RTF export** (`export_rtf(..., comments=False)`, an "Include comments" checkbox, **off by default and hidden when there is none**, `rtf_comments_available()`, bulk exports included): each comment is written as a Word comment -- `{\*\atrfstart N}` before its words, `{\*\atrfend N}{\*\atnid SC}{\*\atnauthor Scribe}\chatn{\*\annotation{\*\atnref N}{\*\atndate D}\pard\plain\f0\fs20 text}` after (`rtf_date()` packs the date as RTF wants it). The pieces of `marked_spans()` are cut where a comment begins or ends so a marker never lands inside a piece; the text between the markers is the caption's own text throughout. Author is the constant "Scribe" (the editor does not know the reader's name). Checked by converting an export with LibreOffice and by opening it in Word (comments by "Scribe" with the right ranges). Highlights show in the export only with "Include highlights" ticked, which is separate. A clean export is byte-for-byte unchanged. +- Tests: `tests/test_comments.py`. The panel's behaviour (type/Save/Remove/Esc/Ctrl+Enter/Cmd+Enter/Close, follow rules, sync) was driven with NiceGUI's `User` simulation, and the component (key, caret reports, attributes) in jsdom against the real component; jsdom has no layout, so nothing about position or focus order is checked there. **Not yet tried by hand in a browser**: focus moving into and out of the card, the caret following in a real Selection, Ctrl+Shift+K being free in the reader's browser, tab order (the card is after the video in the DOM; no skip link was added), and screen readers. ## Undo history (`utils/undo_redo.py`) diff --git a/pages/srt.py b/pages/srt.py index 88730e0..c192199 100644 --- a/pages/srt.py +++ b/pages/srt.py @@ -37,6 +37,7 @@ TIMELINE_SHOW_KEY, ) from utils.speech_timeline import SpeechTimeline +from utils.comments_panel import CommentsPanel from utils.validation_panel import ValidationPanel from utils.transcript_editor import TranscriptEditor from utils.video import create_video_proxy @@ -974,3 +975,12 @@ def save_sensitivity(event) -> None: transcript.on_caret = ( editor.validation_panel.follow_caret ) + else: + # Its counterpart in a transcription: the card + # where comments are written and read, drawn like + # the validation card and following the caret the + # same way. Shut until needed -- except in a file + # that already has comments, which opens on the + # first. + transcript.comments_panel = CommentsPanel(transcript) + transcript.comments_panel.open_first() diff --git a/tests/test_comments.py b/tests/test_comments.py index b68bc0c..df70f64 100644 --- a/tests/test_comments.py +++ b/tests/test_comments.py @@ -820,18 +820,54 @@ def test_the_export_asks_for_comments_only_when_the_box_is_shown_and_ticked(self class FakeBody: def __init__(self): self.selected = [] - self.shown = [] + self.scrolled = [] self.asked = 0 + self.caret_asked = 0 def comment_selection(self): self.asked += 1 - def show_comment(self, payload): - self.shown.append(payload) - def select_range(self, *args): self.selected.append(args) + def scroll_to_block(self, block_id): + self.scrolled.append(block_id) + + def focus_caret(self): + self.caret_asked += 1 + + +class Element: + """ + Just enough of a NiceGUI element to see what the panel did to it. + """ + + def __init__(self, value=""): + self.value = value + self.text = "" + self.enabled = True + self.visible = True + self.properties = [] + + def set_value(self, value): + self.value = value + + def set_text(self, text): + self.text = text + + def set_enabled(self, enabled): + self.enabled = enabled + + def set_visibility(self, visible): + self.visible = visible + + def props(self, props): + self.properties.append(props) + + +class FakePanel: + pass + @pytest.fixture def view(editor, monkeypatch): @@ -844,81 +880,504 @@ def view(editor, monkeypatch): return view -class TestTheShortcutsDialog: - def groups(self, data_format): - return dict(make_editor(data_format).keyboard_shortcut_groups()) +@pytest.fixture +def panel(view, editor, monkeypatch): + from utils.comments_panel import CommentsPanel + + built = CommentsPanel.__new__(CommentsPanel) + built.transcript = view + built.current = None + built.pending = None + + for name in ( + "container", + "heading", + "position_label", + "spoken", + "previous", + "next", + "words_button", + "detail", + "remove_button", + "cancel_button", + "save_button", + ): + setattr(built, name, Element()) - def test_a_transcription_lists_it(self): - assert self.groups("txt")["Comments"] == [ - ("Comment on the selection, or open its comment", "Ctrl + Shift + K"), - ("Save the comment you are writing", "Ctrl/⌘ + Enter"), - ("Close the comment without saving", "Esc"), - ] + built.container.visible = False + built.text = Element("") + built.focused = 0 + built.focus_text = lambda: setattr(built, "focused", built.focused + 1) + monkeypatch.setattr( + CommentsPanel, "focus", staticmethod(lambda element: None) + ) + view.comments_panel = built - def test_subtitles_do_not_for_now(self): - assert "Comments" not in self.groups("srt") + return built -class TestOpeningTheBox: - def test_the_events_reach_the_view(self): - source = pathlib.Path("utils/transcript_editor.py").read_text() +class TestThePanel: + """ + The card under the video where a comment is written, read and changed. + """ - for event, handler in ( - ("opencomment", "open_comment"), - ("peekcomment", "peek_comment"), - ("savecomment", "save_comment"), - ("removecomment", "remove_comment"), - ): - assert ( - f'self.body.on("{event}", lambda event: self.{handler}(event.args))' - in source - ) + def type(self, panel, text): + panel.text.value = text - def test_a_new_comment_opens_empty_with_focus_on_the_words(self, view): - view.open_comment({"spans": [{"id": 1, "start": 5, "end": 9}]}) + def test_a_new_comment_opens_empty_on_the_words_with_focus(self, panel, editor): + assert panel.open_for(editor.captions[0], 5, 9) is True - assert view.body.shown == [ - { - "id": 1, - "start": 4, - "end": 11, - "text": "", - "existing": False, - "focus": True, - } - ] + assert panel.container.visible is True + assert panel.pending == (editor.captions[0], 4, 11) + assert panel.text.value == "" + assert panel.heading.text == "New comment" + assert panel.words_button.text == "\u201ctv\u00e5 tre\u201d" + assert panel.detail.text == "Paragraph 1" + assert panel.focused == 1 - def test_it_changes_nothing_by_itself(self, view, editor): - view.open_comment({"spans": [{"id": 1, "start": 5, "end": 9}]}) + def test_opening_changes_nothing_by_itself(self, panel, editor): + panel.open_for(editor.captions[0], 5, 9) assert editor.captions[0].comments == [] assert editor.snapshots == 0 - assert view.refreshed == 0 + assert panel.transcript.refreshed == 0 - def test_words_with_a_comment_open_it_with_its_text_and_range(self, view, editor): + def test_the_words_of_an_existing_comment_open_it_with_its_text( + self, panel, editor + ): editor.captions[0].comments = [comment({1, 2}, "there")] - view.open_comment({"spans": [{"id": 1, "start": 5, "end": 5}]}) + panel.open_for(editor.captions[0], 5, 5) + + assert panel.pending is None + assert panel.current[1] is editor.captions[0].comments[0] + assert panel.text.value == "there" + assert panel.heading.text == "Comment 1 of 1" + assert panel.focused == 1 + + def test_no_word_means_nothing_opens(self, panel, editor): + editor.captions[0].text = "ett tre" + + assert panel.open_for(editor.captions[0], 4, 4) is False + assert panel.container.visible is False + + def test_the_heading_says_where_it_is_among_all_the_comments(self, panel, editor): + editor.captions[0].comments = [comment({0}, "a"), comment({2}, "b")] + editor.captions[1].comments = [comment({1}, "c")] + + panel.show(editor.captions[1], editor.captions[1].comments[0]) + + assert panel.heading.text == "Comment 3 of 3" + assert panel.position_label.text == "3 of 3" + assert panel.spoken.text == "Comment 3 of 3." + assert panel.previous.enabled is True + assert panel.next.enabled is False + + def test_drawing_it_scrolls_the_text_to_the_words_unless_told_not_to( + self, panel, editor + ): + editor.captions[1].comments = [comment({1}, "c")] + + panel.show(editor.captions[1], editor.captions[1].comments[0]) + panel.show(editor.captions[1], editor.captions[1].comments[0], scroll=False) + + assert panel.transcript.body.scrolled == [2] + + def test_saving_a_new_comment_puts_it_on_the_words_and_goes_back_to_them( + self, panel, editor + ): + panel.open_for(editor.captions[0], 5, 9) + self.type(panel, " check this ") + + panel.save() + + assert shape(editor.captions[0]) == [([1, 2], "check this")] + assert panel.pending is None + assert panel.current[1] is editor.captions[0].comments[0] + assert panel.transcript.refreshed == 1 + assert panel.transcript.body.selected == [(1, 4, 1, 11)] + assert panel.heading.text == "Comment 1 of 1" + + def test_saving_nothing_does_nothing(self, panel, editor): + panel.open_for(editor.captions[0], 5, 9) + self.type(panel, " ") + + panel.save() + + assert editor.captions[0].comments == [] + assert panel.pending is not None + assert panel.transcript.refreshed == 0 + + def test_save_cannot_be_pressed_with_nothing_typed(self, panel, editor): + panel.open_for(editor.captions[0], 5, 9) + + assert panel.save_button.enabled is False + + self.type(panel, "x") + panel.value_changed() + + assert panel.save_button.enabled is True + + def test_saving_a_changed_text_changes_that_comment(self, panel, editor): + editor.captions[0].comments = [comment({1}, "old")] + panel.open_for(editor.captions[0], 5, 5) + self.type(panel, "new") + + panel.save() + + assert shape(editor.captions[0]) == [([1], "new")] + assert panel.transcript.body.selected == [(1, 4, 1, 7)] + + def test_cancelling_a_new_comment_makes_none_and_goes_back_to_the_words( + self, panel, editor + ): + panel.open_for(editor.captions[0], 5, 9) + self.type(panel, "abandoned") + + panel.cancel() + + assert editor.captions[0].comments == [] + assert panel.pending is None + assert panel.container.visible is False + assert panel.transcript.body.selected == [(1, 4, 1, 11)] + + def test_cancelling_a_new_comment_goes_back_to_the_one_that_was_on_show( + self, panel, editor + ): + editor.captions[0].comments = [comment({0}, "there")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + panel.open_for(editor.captions[1], 0, 4) + + panel.cancel() + + assert panel.container.visible is True + assert panel.text.value == "there" + assert panel.heading.text == "Comment 1 of 1" + + def test_cancelling_a_change_puts_the_old_text_back(self, panel, editor): + editor.captions[0].comments = [comment({1}, "old")] + panel.open_for(editor.captions[0], 5, 5) + self.type(panel, "typed over") + + panel.cancel() + + assert panel.text.value == "old" + assert shape(editor.captions[0]) == [([1], "old")] + assert panel.transcript.body.selected == [(1, 4, 1, 7)] + + def test_removing_takes_the_comment_off_and_shows_the_next(self, panel, editor): + editor.captions[0].comments = [comment({0}, "a"), comment({2}, "b")] + panel.open_for(editor.captions[0], 0, 3) + + panel.remove() + + assert shape(editor.captions[0]) == [([2], "b")] + assert panel.current[1].text == "b" + assert panel.heading.text == "Comment 1 of 1" + assert panel.transcript.body.selected == [(1, 0, 1, 3)] + + def test_removing_the_last_one_puts_the_panel_away(self, panel, editor): + editor.captions[0].comments = [comment({0}, "a")] + panel.open_for(editor.captions[0], 0, 3) + + panel.remove() + + assert editor.captions[0].comments == [] + assert panel.container.visible is False + assert panel.current is None + + def test_a_comment_not_yet_made_cannot_be_removed(self, panel, editor): + panel.open_for(editor.captions[0], 5, 9) + + panel.remove() + + assert editor.snapshots == 0 + assert panel.pending is not None + + def test_closing_keeps_what_was_typed_and_goes_back_to_the_words( + self, panel, editor + ): + panel.open_for(editor.captions[0], 5, 9) + self.type(panel, "kept") + + panel.close() + + assert shape(editor.captions[0]) == [([1, 2], "kept")] + assert panel.container.visible is False + assert panel.transcript.body.selected[-1] == (1, 4, 1, 11) + + def test_closing_with_nothing_typed_makes_nothing(self, panel, editor): + panel.open_for(editor.captions[0], 5, 9) + + panel.close() + + assert editor.captions[0].comments == [] + assert panel.container.visible is False + + def test_the_arrows_step_through_every_comment_in_the_order_of_the_text( + self, panel, editor + ): + editor.captions[0].comments = [comment({0}, "a"), comment({2}, "b")] + editor.captions[1].comments = [comment({1}, "c")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + + panel.go_next() + assert panel.text.value == "b" + panel.go_next() + assert panel.text.value == "c" + assert panel.heading.text == "Comment 3 of 3" + panel.go_next() + assert panel.text.value == "c" + panel.go_previous() + assert panel.text.value == "b" + + def test_stepping_keeps_what_was_typed(self, panel, editor): + editor.captions[0].comments = [comment({0}, "a"), comment({2}, "b")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + self.type(panel, "a, rewritten") + + panel.go_next() + + assert shape(editor.captions[0]) == [([0], "a, rewritten"), ([2], "b")] + + def test_a_comment_being_written_cannot_be_stepped_away_from(self, panel, editor): + editor.captions[0].comments = [comment({0}, "a")] + panel.open_for(editor.captions[1], 0, 4) + self.type(panel, "half written") + + panel.go_next() + panel.go_previous() + + assert panel.pending is not None + assert panel.text.value == "half written" + assert panel.previous.enabled is False and panel.next.enabled is False + + def test_the_words_button_selects_them_and_scrolls_to_them(self, panel, editor): + editor.captions[1].comments = [comment({1}, "c")] + panel.show(editor.captions[1], editor.captions[1].comments[0], scroll=False) + + panel.go_to_words() + + assert panel.transcript.body.scrolled == [2] + assert panel.transcript.body.selected == [(2, 5, 2, 8)] + + def test_a_file_with_comments_opens_on_the_first_without_taking_focus( + self, panel, editor + ): + editor.captions[1].comments = [comment({1}, "c")] + + panel.open_first() + + assert panel.container.visible is True + assert panel.text.value == "c" + assert panel.focused == 0 + assert panel.transcript.body.scrolled == [] + + def test_a_file_without_comments_leaves_it_shut(self, panel, editor): + panel.open_first() + + assert panel.container.visible is False + + +class TestFollowingTheCaret: + def one(self, editor): + editor.captions[0].comments = [comment({0}, "a")] + editor.captions[1].comments = [comment({1}, "b")] + + return editor.captions[0].comments[0], editor.captions[1].comments[0] + + def test_a_click_on_commented_words_opens_a_shut_panel_on_the_comment( + self, panel, editor + ): + first, _ = self.one(editor) + + panel.follow(editor.captions[0], first, click=True) + + assert panel.container.visible is True + assert panel.text.value == "a" + assert panel.focused == 0 + + def test_a_key_that_crosses_commented_words_does_not_open_a_shut_panel( + self, panel, editor + ): + first, _ = self.one(editor) + + panel.follow(editor.captions[0], first, click=False) + + assert panel.container.visible is False + + def test_an_open_panel_follows_the_caret_from_comment_to_comment( + self, panel, editor + ): + first, second = self.one(editor) + panel.show(editor.captions[0], first) + + panel.follow(editor.captions[1], second, click=False) + + assert panel.text.value == "b" + assert panel.heading.text == "Comment 2 of 2" + + def test_following_does_not_scroll_the_text_under_the_caret(self, panel, editor): + first, second = self.one(editor) + panel.show(editor.captions[0], first, scroll=False) + + panel.follow(editor.captions[1], second, click=True) + + assert panel.transcript.body.scrolled == [] + + def test_the_comment_already_on_show_is_left_alone(self, panel, editor): + first, _ = self.one(editor) + panel.show(editor.captions[0], first) + self.draws = 0 + panel.draw = lambda **k: setattr(self, "draws", self.draws + 1) + panel.text.value = "typing in it" + + panel.follow(editor.captions[0], first, click=True) + + assert self.draws == 0 + assert panel.text.value == "typing in it" + + def test_what_was_typed_is_kept_when_the_caret_moves_on(self, panel, editor): + first, second = self.one(editor) + panel.show(editor.captions[0], first) + panel.text.value = "a, changed" + + panel.follow(editor.captions[1], second, click=True) + + assert first.text == "a, changed" + assert panel.text.value == "b" - assert view.body.shown[0]["text"] == "there" - assert view.body.shown[0]["existing"] is True - assert (view.body.shown[0]["start"], view.body.shown[0]["end"]) == (4, 11) + def test_a_comment_being_written_is_kept_when_the_caret_moves_on( + self, panel, editor + ): + _, second = self.one(editor) + panel.open_for(editor.captions[0], 4, 7) + panel.text.value = "new one" + + panel.follow(editor.captions[1], second, click=True) + + assert [c.text for c in editor.captions[0].comments] == ["a", "new one"] + assert panel.pending is None + + def test_an_empty_comment_being_written_is_dropped_when_the_caret_moves_on( + self, panel, editor + ): + _, second = self.one(editor) + panel.open_for(editor.captions[0], 4, 7) + + panel.follow(editor.captions[1], second, click=True) + + assert [c.text for c in editor.captions[0].comments] == ["a"] + assert panel.pending is None + + +class TestKeepingUpWithTheText: + def test_a_card_that_is_still_right_is_not_touched(self, panel, editor): + editor.captions[0].comments = [comment({0}, "a")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + panel.text.value = "being typed" + calls = [] + panel.draw = lambda **k: calls.append(k) + + panel.sync() + + assert calls == [] + assert panel.text.value == "being typed" + + def test_a_comment_that_an_undo_replaced_is_found_again_by_its_words( + self, panel, editor + ): + editor.captions[0].comments = [comment({0, 1}, "a")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + + replacement = editor.captions[0].copy() + replacement.comments[0].text = "as it was" + editor.captions[0] = replacement + + panel.sync() + + assert panel.current[0] is replacement + assert panel.current[1] is replacement.comments[0] + assert panel.text.value == "as it was" + assert panel.container.visible is True + + def test_a_comment_that_is_gone_puts_the_panel_away(self, panel, editor): + editor.captions[0].comments = [comment({0}, "a")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + + editor.captions[0].comments = [] + panel.sync() + + assert panel.container.visible is False + assert panel.current is None + + def test_a_comment_whose_words_were_deleted_goes_with_the_panel( + self, panel, editor + ): + editor.captions[0].comments = [comment({1}, "a")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + + editor.update_caption_text(editor.captions[0], "ett tre") + panel.sync() + + assert panel.container.visible is False + + def test_a_new_comment_whose_paragraph_is_gone_is_dropped(self, panel, editor): + panel.open_for(editor.captions[1], 0, 4) + + editor.captions = [editor.captions[0]] + panel.sync() + + assert panel.pending is None + assert panel.container.visible is False + + def test_a_shut_panel_is_left_alone(self, panel, editor): + panel.sync() + + assert panel.container.visible is False + + def test_the_view_tells_the_panel_whenever_it_redraws(self, view, panel): + calls = [] + panel.sync = lambda: calls.append(1) + view.body.set_blocks = lambda blocks: None + view.body.set_speakers = lambda *a: None + view.refresh_overlay = lambda: None + view.refresh_timeline = lambda: None + del view.refresh + view.blocks = lambda: [] + + view.refresh() + + assert calls == [1] + + +class TestAskingForTheComment: + def test_the_events_reach_the_view(self): + source = pathlib.Path("utils/transcript_editor.py").read_text() - def test_nothing_selected_says_so(self, view, editor): + assert 'self.body.on("opencomment", lambda event: self.open_comment(event.args))' in source + assert 'self.body.on("caretcomment", lambda event: self.caret_comment(event.args))' in source + + def test_the_selected_words_open_the_panel_on_them(self, view, panel, editor): + view.open_comment({"spans": [{"id": 1, "start": 5, "end": 9}]}) + + assert panel.pending == (editor.captions[0], 4, 11) + assert panel.container.visible is True + + def test_nothing_selected_says_so(self, view, panel, editor): view.open_comment({"spans": []}) assert editor.messages == ["Select the words to comment on first"] - assert view.body.shown == [] + assert panel.container.visible is False - def test_a_caret_in_the_space_between_words_says_so(self, view, editor): + def test_a_caret_between_words_says_so(self, view, panel, editor): editor.captions[0].text = "ett tre" view.open_comment({"spans": [{"id": 1, "start": 4, "end": 4}]}) assert editor.messages == ["Select the words to comment on first"] - assert view.body.shown == [] - def test_a_selection_across_paragraphs_is_refused(self, view, editor): + def test_a_selection_across_paragraphs_is_refused(self, view, panel, editor): view.open_comment( { "spans": [ @@ -929,10 +1388,10 @@ def test_a_selection_across_paragraphs_is_refused(self, view, editor): ) assert editor.messages == ["A comment has to stay within one paragraph"] - assert view.body.shown == [] + assert panel.container.visible is False def test_a_selection_that_only_brushes_the_next_paragraph_is_within_one( - self, view, editor + self, view, panel, editor ): view.open_comment( { @@ -944,18 +1403,23 @@ def test_a_selection_that_only_brushes_the_next_paragraph_is_within_one( ) assert editor.messages == [] - assert view.body.shown[0]["id"] == 1 + assert panel.pending[0] is editor.captions[0] def test_subtitles_open_nothing(self, monkeypatch): monkeypatch.setattr("utils.srt.ui.notify", lambda *a, **k: None) subtitles = make_editor("srt") view = TranscriptEditor(subtitles) view.body = FakeBody() + view.comments_panel = FakePanel() view.open_comment({"spans": [{"id": 1, "start": 0, "end": 3}]}) - view.peek_comment({"id": 1, "offset": 1}) + view.caret_comment({"id": 1, "cm": 0, "click": True}) - assert view.body.shown == [] + def test_without_a_panel_nothing_happens(self, view, editor): + view.comments_panel = None + + view.open_comment({"spans": [{"id": 1, "start": 0, "end": 3}]}) + view.caret_comment({"id": 1, "cm": 0}) @pytest.mark.parametrize( "args", @@ -972,123 +1436,47 @@ def test_subtitles_open_nothing(self, monkeypatch): {"spans": [{"id": 1, "start": 0}]}, ], ) - def test_bad_input_is_harmless(self, view, editor, args): + def test_bad_input_is_harmless(self, view, panel, editor, args): view.open_comment(args) - assert view.body.shown == [] + assert panel.container.visible is False assert all(not caption.comments for caption in editor.captions) + def test_the_caret_in_a_comment_shows_it(self, view, panel, editor): + editor.captions[1].comments = [comment({1}, "b")] -class TestPeeking: - def test_a_click_on_a_commented_word_shows_its_comment_without_focus( - self, view, editor - ): - editor.captions[0].comments = [comment({1}, "there")] + view.caret_comment({"id": 2, "cm": 0, "click": True}) - view.peek_comment({"id": 1, "offset": 5}) + assert panel.text.value == "b" - assert view.body.shown == [ - { - "id": 1, - "start": 4, - "end": 7, - "text": "there", - "existing": True, - "focus": False, - } - ] - - def test_a_click_anywhere_else_shows_nothing(self, view, editor): - editor.captions[0].comments = [comment({1}, "there")] + def test_a_click_is_told_from_a_key(self, view, panel, editor): + editor.captions[1].comments = [comment({1}, "b")] - view.peek_comment({"id": 1, "offset": 0}) - view.peek_comment({"id": 2, "offset": 1}) + view.caret_comment({"id": 2, "cm": 0, "click": False}) + view.caret_comment({"id": 2, "cm": 0}) + view.caret_comment({"id": 2, "cm": 0, "click": "yes"}) - assert view.body.shown == [] - - @pytest.mark.parametrize( - "args", - [None, {}, {"id": 1}, {"id": 1, "offset": "5"}, {"id": 1, "offset": True}, - {"id": 9, "offset": 1}], - ) - def test_bad_input_is_harmless(self, view, editor, args): - editor.captions[0].comments = [comment({1}, "there")] - - view.peek_comment(args) - - assert view.body.shown == [] - - -class TestSavingAndRemoving: - def test_saving_puts_the_comment_on_the_words_and_redraws(self, view, editor): - view.save_comment({"id": 1, "start": 4, "end": 11, "text": "check"}) - - assert shape(editor.captions[0]) == [([1, 2], "check")] - assert view.refreshed == 1 - - def test_the_selection_goes_back_to_the_words(self, view): - view.save_comment({"id": 1, "start": 4, "end": 11, "text": "check"}) - - assert view.body.selected == [(1, 4, 1, 11)] - - def test_saving_from_a_click_elsewhere_leaves_the_caret_where_it_went(self, view): - view.save_comment( - {"id": 1, "start": 4, "end": 11, "text": "check", "stay": True} - ) - - assert view.refreshed == 1 - assert view.body.selected == [] - - def test_saving_what_is_already_there_redraws_nothing(self, view, editor): - editor.captions[0].comments = [comment({1}, "same")] - - view.save_comment({"id": 1, "start": 4, "end": 7, "text": "same"}) - - assert view.refreshed == 0 - assert view.body.selected == [] - - def test_saving_a_changed_text_changes_that_comment(self, view, editor): - editor.captions[0].comments = [comment({1}, "old")] - - view.save_comment({"id": 1, "start": 4, "end": 7, "text": "new"}) - - assert shape(editor.captions[0]) == [([1], "new")] - - def test_removing_takes_the_comment_off(self, view, editor): - editor.captions[0].comments = [comment({1}, "old")] - - view.remove_comment({"id": 1, "start": 4, "end": 7}) - - assert editor.captions[0].comments == [] - assert view.refreshed == 1 - assert view.body.selected == [(1, 4, 1, 7)] - - def test_removing_where_there_is_none_redraws_nothing(self, view): - view.remove_comment({"id": 1, "start": 4, "end": 7}) - - assert view.refreshed == 0 + assert panel.container.visible is False @pytest.mark.parametrize( "args", [ None, - "x", {}, - {"id": 1, "start": 0, "end": 3}, - {"id": 1, "start": 0, "end": 3, "text": 5}, - {"id": 1, "start": 0, "end": 3, "text": ""}, - {"id": 99, "start": 0, "end": 3, "text": "x"}, - {"id": 1, "start": "0", "end": 3, "text": "x"}, - {"id": 1, "start": True, "end": 3, "text": "x"}, - {"id": 1, "start": 0, "text": "x"}, + {"id": 2}, + {"id": 2, "cm": "0"}, + {"id": 2, "cm": True}, + {"id": 2, "cm": 5}, + {"id": 2, "cm": -1}, + {"id": 9, "cm": 0}, ], ) - def test_bad_input_is_harmless(self, view, editor, args): - view.save_comment(args) - view.remove_comment(args) + def test_a_bad_caret_report_is_harmless(self, view, panel, editor, args): + editor.captions[1].comments = [comment({1}, "b")] - assert all(not caption.comments for caption in editor.captions) - assert view.refreshed == 0 + view.caret_comment(args) + + assert panel.container.visible is False class TestTheButton: @@ -1141,6 +1529,90 @@ def test_the_icon_is_a_material_symbol_that_exists(self): assert 'icon="sym_o_add_comment"' in self.page() +class TestThePanelSource: + def source(self): + return pathlib.Path("utils/comments_panel.py").read_text() + + def test_it_is_drawn_like_the_validation_card_in_its_own_colour(self): + source = self.source() + + assert '"validation-panel is-comment w-full"' in source + assert "validation-panel-head" in source and "validation-panel-step" in source + + def test_it_has_previous_next_position_and_close_with_names(self): + source = self.source() + + for label in ( + 'aria-label="Previous comment"', + 'aria-label="Next comment"', + 'aria-label="Close comments panel"', + ): + assert label in source + + def test_the_heading_names_the_section_and_can_take_focus(self): + source = self.source() + + assert 'aria-labelledby="comments-panel-heading"' in source + assert "id=comments-panel-heading tabindex=-1" in source + + def test_the_position_is_spoken_in_a_live_region(self): + assert '"role=status aria-live=polite aria-atomic=true"' in self.source() + + def test_the_text_is_limited_as_the_server_limits_it(self): + assert "maxlength={COMMENT_MAX_LENGTH}" in self.source() + + def test_keys_typed_in_the_text_stay_in_it(self): + source = self.source() + + assert '"keydown", js_handler="(event) => event.stopPropagation()"' in source + + def test_escape_abandons_and_ctrl_or_cmd_enter_saves(self): + source = self.source() + + assert 'self.text.on("keydown.escape", self.cancel)' in source + assert 'self.text.on("keydown.ctrl.enter", self.save)' in source + assert 'self.text.on("keydown.meta.enter", self.save)' in source + + def test_it_starts_shut(self): + assert "self.container.set_visibility(False)" in self.source() + + def test_the_page_builds_it_for_transcriptions_and_opens_a_file_that_has_comments(self): + page = pathlib.Path("pages/srt.py").read_text() + start = page.index("transcript.comments_panel = CommentsPanel(transcript)") + block = page[start - 900 : start] + + assert "else:" in block + assert "transcript.comments_panel.open_first()" in page[start : start + 200] + assert "data_format" not in page[start : start + 200] + + def test_the_validation_panel_is_still_subtitles_only(self): + page = pathlib.Path("pages/srt.py").read_text() + start = page.index("editor.validation_panel = ValidationPanel(") + + assert 'if data_format == "srt":' in page[start - 200 : start] + + +class TestPanelHelpers: + def test_the_quote_is_one_line(self): + from utils.comments_panel import quote_of + + assert quote_of("two\n lines here") == "two lines here" + + def test_a_long_quote_is_shortened(self): + from utils.comments_panel import QUOTE_LENGTH, quote_of + + quoted = quote_of("word " * 50) + + assert len(quoted) == QUOTE_LENGTH + assert quoted.endswith("\u2026") + + def test_the_heading(self): + from utils.comments_panel import heading_text + + assert heading_text(True, 0, 3) == "New comment" + assert heading_text(False, 1, 3) == "Comment 2 of 3" + + class TestTheComponent: def source(self): return pathlib.Path("utils/transcript_editor.js").read_text() @@ -1174,12 +1646,24 @@ def test_the_button_reports_the_selection_as_the_key_does(self): assert "this.flush();" in body assert '"opencomment", { spans: this.highlightSpans() }' in body - def test_a_click_on_commented_words_asks_for_a_peek(self): - source = self.source() + def test_the_caret_in_a_comment_is_reported_once_per_comment(self): + body = self.method("reportCommentCaret") + + assert "key === this.lastCommentKey && !clicked" in body + assert '"caretcomment", { ...at, click: clicked }' in body + + def test_only_a_collapsed_caret_in_commented_words_counts(self): + body = self.method("commentAtCaret") - assert 'event.target.closest("[data-comment]") &&' in source - assert "!this.subtitleMode &&\n window.getSelection()?.isCollapsed" in source - assert '"peekcomment"' in source + assert "!selection.isCollapsed" in body + assert 'closest?.("[data-comment]")' in body + + def test_keys_report_it_and_a_click_reports_it_as_a_click(self): + update = self.method("updateCaretBlock") + click = self.method("onClick") + + assert "this.reportCommentCaret();" in update + assert "this.reportCommentCaret(true);" in click def test_commented_words_are_marked_up_with_what_a_screen_reader_needs(self): source = self.source() @@ -1199,62 +1683,11 @@ def test_the_classes_come_from_the_one_function(self): assert 'run.cm !== undefined ? "comment-word" : null' in source assert 'run.cl ? "comment-end" : null' in source - def test_the_box_is_a_labelled_dialog_with_a_labelled_text_area(self): - source = self.source() - - assert 'class="comment-box"\n role="dialog"\n aria-label="Comment"' in source - assert ":aria-label=\"'Comment on ' + comment.quote\"" in source - - def test_the_text_is_limited_as_the_server_limits_it(self): - assert 'maxlength="1000"' in self.source() - - def test_save_cannot_save_nothing(self): - assert ':disable="!comment.text.trim()"' in self.source() - - def test_keys_typed_in_the_box_stay_in_it(self): - body = self.method("onCommentKeydown") - - assert "event.stopPropagation();" in body - - def test_escape_abandons_and_ctrl_enter_saves(self): - body = self.method("onCommentKeydown") - - assert 'event.key === "Escape"' in body - assert "this.cancelComment();" in body - assert 'event.key === "Enter" && (event.ctrlKey || event.metaKey)' in body - assert "this.saveComment();" in body - - def test_leaving_the_box_keeps_what_was_typed_but_not_a_peek(self): - body = self.method("leaveComment") - - assert "!this.comment.peek" in body - assert "text !== this.comment.original.trim()" in body - assert "stay: true" in body - - def test_saving_and_cancelling_put_the_selection_back_on_the_words(self): - for name in ("saveComment", "removeComment", "cancelComment"): - assert "this.returnToComment();" in self.method(name) - - def test_a_peek_goes_at_the_next_key(self): + def test_nothing_of_the_old_box_is_left(self): source = self.source() - assert "if (this.comment.open && this.comment.peek) this.closeComment();" in source - - def test_a_click_outside_leaves_the_box(self): - assert 'if (!event.target.closest(".comment-box")) this.leaveComment();' in self.source() - - def test_the_box_takes_focus_unless_it_is_only_a_peek(self): - body = self.method("showComment") - - assert "peek: !payload.focus" in body - assert "if (payload.focus) {" in body - assert "area.focus();" in body - - def test_it_is_placed_against_the_components_own_root(self): - body = self.method("showComment") - - assert "this.$el.getBoundingClientRect()" in body - assert "y: y - root.top" in body + for gone in ("comment-box", "showComment", "peekcomment", "leaveComment"): + assert gone not in source class TestTheStyles: @@ -1275,7 +1708,8 @@ def test_commented_words_have_a_dotted_underline_that_is_not_only_colour(self): assert "text-decoration-line: underline;" in rule assert "text-decoration-style: dotted;" in rule - assert "text-decoration-thickness: 2px;" in rule + assert "text-decoration-thickness: 3px;" in rule + assert "text-underline-offset: 4px;" in rule assert "var(--color-comment-line)" in rule def test_the_underline_takes_no_room_and_no_box_shadow(self): @@ -1292,12 +1726,13 @@ def test_nothing_is_added_after_the_last_word(self): assert ".comment-end::after" not in css assert ".comment-word::after" not in css - def test_the_box_is_placed_from_its_own_root_like_the_menu(self): - rule = self.rule(" .comment-box {") + def test_the_panel_has_the_colour_of_the_underline(self): + rule = self.rule(" .validation-panel.is-comment {") + + assert "--vp-tone: var(--color-comment-line);" in rule - assert "position: absolute;" in rule - assert "z-index: 9500;" in rule - assert "var(--color-bg-surface)" in rule + def test_nothing_of_the_old_box_is_left(self): + assert ".comment-box" not in self.css() @pytest.mark.parametrize( "line,backgrounds", diff --git a/utils/comments_panel.py b/utils/comments_panel.py new file mode 100644 index 0000000..6a6be9a --- /dev/null +++ b/utils/comments_panel.py @@ -0,0 +1,602 @@ +# Copyright (c) 2025-2026 Sunet. +# Contributor: Kristofer Hallin +# +# This file is part of Sunet Scribe. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +The comments panel: where a comment is written, read and changed. + +A card under the video, built like the validation panel's and drawn like it -- +a stripe down the left and a tinted header, with Previous, the position and +Next in the header -- in the colour of the underline on the commented words. +It shows one comment at a time and follows the caret: put the caret in +commented words, or click them, and the panel shows that comment. + +It is shut until it is needed: it opens when a comment is written or opened +(Ctrl+Shift+K, the Comment button), when commented words are clicked, and when +a file with comments is opened. What is typed in it is kept when the reader +moves on -- to another comment, to the text, or shutting the panel -- and +abandoned only by Escape or Cancel. +""" + +from typing import Optional, Tuple + +from nicegui import ui + +from utils.caption import SRTCaption, WordComment +from utils.srt_comments import COMMENT_MAX_LENGTH + +# The words a comment is on, as shown in the panel and read out. +QUOTE_LENGTH = 80 + + +def quote_of(text: str) -> str: + """ + The words a comment is on, flattened to one line and shortened. + """ + + flat = " ".join(text.split()) + + return flat if len(flat) <= QUOTE_LENGTH else flat[: QUOTE_LENGTH - 1] + "…" + + +def position_text(position: int, total: int) -> str: + return f"{position + 1} of {total}" + + +def heading_text(pending: bool, position: int, total: int) -> str: + if pending: + return "New comment" + + return f"Comment {position_text(position, total)}" + + +class CommentsPanel: + """ + Built once, hidden, under the video. + + Parameters: + transcript: the TranscriptEditor, to redraw after a change and to put + the selection back on the words. + """ + + def __init__(self, transcript) -> None: + self.transcript = transcript + + # The comment on screen: (caption, comment). While a new comment is + # being written it is the last one that was on screen, and `pending` + # -- (caption, start, end) -- is what the card is about instead. + self.current: Optional[Tuple[SRTCaption, WordComment]] = None + self.pending: Optional[Tuple[SRTCaption, int, int]] = None + + with ui.element("div").classes("validation-panel-wrap w-full") as self.container: + with ui.element("section").classes( + "validation-panel is-comment w-full" + ).props('aria-labelledby="comments-panel-heading"') as self.panel: + with ui.element("div").classes("validation-panel-head"): + self.heading = ( + ui.label("") + .classes("validation-panel-heading") + .props("id=comments-panel-heading tabindex=-1") + ) + + with ui.element("div").classes("validation-panel-nav"): + self.previous = ui.button( + icon="chevron_left", on_click=self.go_previous, color=None + ).props('flat aria-label="Previous comment"').classes( + "validation-panel-step" + ) + self.position_label = ui.label("").classes( + "validation-panel-position" + ) + self.next = ui.button( + icon="chevron_right", on_click=self.go_next, color=None + ).props('flat aria-label="Next comment"').classes( + "validation-panel-step" + ) + + ui.button(icon="close", on_click=self.close).props( + 'flat aria-label="Close comments panel"' + ).classes("editor-btn editor-icon validation-panel-close").tooltip( + "Close" + ) + + # Said as the panel moves between comments, while focus stays + # where it is. + self.spoken = ui.label("").classes("sr-only").props( + "role=status aria-live=polite aria-atomic=true" + ) + + with ui.element("div").classes("validation-panel-body"): + with ui.element("div").classes("validation-panel-issue"): + with ui.element("div").classes("validation-panel-kind"): + ui.icon("comment").props("aria-hidden=true") + ui.label("Comment") + + with ui.element("div").classes("validation-panel-detail"): + self.words_button = ( + ui.button("", on_click=self.go_to_words, color=None) + .props("flat dense no-caps") + .classes("validation-panel-caption comments-panel-words") + ) + self.separator = ui.label("·").props("aria-hidden=true") + self.detail = ui.label("") + + self.text = ( + ui.textarea(label="Comment") + .props(f"outlined autogrow maxlength={COMMENT_MAX_LENGTH}") + .classes("w-full comments-panel-text") + ) + # Every key typed here stays here: the page has + # shortcuts of its own (undo, save, play) that would + # otherwise act on the transcript while a sentence is + # being written. Escape abandons and Ctrl/Cmd+Enter + # saves. + self.text.on( + "keydown", js_handler="(event) => event.stopPropagation()" + ) + self.text.on("keydown.escape", self.cancel) + self.text.on("keydown.ctrl.enter", self.save) + self.text.on("keydown.meta.enter", self.save) + self.text.on_value_change(self.value_changed) + + with ui.element("div").classes("comments-panel-actions"): + self.remove_button = ( + ui.button("Remove", on_click=self.remove) + .props("flat dense no-caps") + .classes("comments-panel-remove") + ) + self.cancel_button = ui.button( + "Cancel", on_click=self.cancel + ).props("flat dense no-caps") + self.save_button = ui.button( + "Save", on_click=self.save + ).props("dense no-caps") + + self.container.set_visibility(False) + + # -- What there is ------------------------------------------------------ + + @property + def editor(self): + return self.transcript.editor + + def entries(self) -> list: + """ + Every comment in the transcription, in the order of the text. + """ + + return [ + (caption, comment) + for caption in self.editor.captions + for comment in caption.comments + ] + + def position(self) -> int: + """ + Where the comment on screen is among all of them, or -1. + """ + + if self.current is None: + return -1 + + for at, (caption, comment) in enumerate(self.entries()): + if comment is self.current[1]: + return at + + return -1 + + def span_of(self, caption: SRTCaption, comment: WordComment) -> Optional[tuple]: + """ + The character range of the words a comment is on. + """ + + return self.editor.word_range(caption.text, min(comment.words), max(comment.words)) + + def shown_range(self) -> Optional[tuple]: + """ + (caption, start, end) of what the card is about: the words of the + comment on screen, or of the one being written. + """ + + if self.pending is not None: + return self.pending + + if self.current is not None: + span = self.span_of(*self.current) + + if span is not None: + return self.current[0], span[0], span[1] + + return None + + # -- What was typed ------------------------------------------------------- + + def draft(self) -> str: + return self.editor.clean_comment(self.text.value) + + def dirty(self) -> bool: + """ + Whether there is something typed that is not yet the comment. + """ + + if not self.draft(): + return False + + if self.pending is not None: + return True + + return self.current is not None and self.draft() != self.current[1].text + + def save_draft(self) -> Optional[tuple]: + """ + Keep what is typed as the comment on the words it is about, redraw + the text, and say where: (caption, start, end), or None when nothing + was kept (nothing typed, the same as there, refused). + """ + + shown = self.shown_range() + + if shown is None or not self.draft(): + return None + + caption, start, end = shown + + if not self.editor.set_comment(caption, start, end, self.draft()): + return None + + self.transcript.refresh() + self.transcript.changed() + + return shown + + def commit(self) -> bool: + """ + Keep what is typed, if there is anything: the reader is moving on, and + nothing they wrote is thrown away by it. + """ + + if not self.dirty(): + return False + + return self.save_draft() is not None + + def value_changed(self) -> None: + self.save_button.set_enabled(bool(self.draft())) + + # -- Showing ---------------------------------------------------------------- + + def show( + self, + caption: SRTCaption, + comment: Optional[WordComment] = None, + words: Optional[tuple] = None, + focus: bool = False, + scroll: bool = True, + ) -> None: + """ + Put a comment on the card -- an existing one, or a new one to be + written on `words` (start, end) -- and open the panel. + """ + + if comment is not None: + self.current = (caption, comment) + self.pending = None + self.text.set_value(comment.text) + else: + self.pending = (caption, words[0], words[1]) + self.text.set_value("") + + self.container.set_visibility(True) + self.draw(scroll=scroll) + + if focus: + self.focus_text() + + def open_for(self, caption: SRTCaption, start: int, end: int) -> bool: + """ + The key or the button: write a comment on the words around a + selection, or open the one they already have. False when the + selection takes no word. + """ + + target = self.editor.comment_target(caption, start, end) + + if target is None: + return False + + self.commit() + + if target["index"] is not None: + self.show(caption, caption.comments[target["index"]], focus=True) + else: + self.show(caption, words=(target["start"], target["end"]), focus=True) + + return True + + def open_first(self) -> None: + """ + A file with comments has just been opened: show the first, without + taking focus or moving the text. + """ + + entries = self.entries() + + if entries: + caption, comment = entries[0] + self.show(caption, comment, scroll=False) + + def follow(self, caption: SRTCaption, comment: WordComment, click: bool) -> None: + """ + The caret is in a comment's words: show that comment. + + An open panel follows the caret; a shut one opens only for a click, + so an arrow key that happens to cross commented words does not throw + a card up under someone typing. A comment already on screen is left + alone. + """ + + if not (self.container.visible or click): + return + + if ( + self.container.visible + and self.pending is None + and self.current is not None + and self.current[1] is comment + ): + return + + self.commit() + self.show(caption, comment, scroll=False) + + def draw(self, scroll: bool = True) -> None: + """ + Redraw the card on what it is about. + """ + + shown = self.shown_range() + entries = self.entries() + total = len(entries) + at = self.position() + pending = self.pending is not None + + self.heading.set_text(heading_text(pending, max(at, 0), total)) + self.position_label.set_text("" if pending else position_text(at, total)) + self.spoken.set_text( + "New comment." if pending else f"Comment {position_text(at, total)}." + ) + + self.previous.set_enabled(not pending and at > 0) + self.next.set_enabled(not pending and 0 <= at < total - 1) + + if shown is not None: + caption, start, end = shown + self.words_button.set_text(f"“{quote_of(caption.text[start:end])}”") + spoken = quote_of(caption.text[start:end]).replace('"', "'") + self.words_button.props(f'aria-label="Go to the words: {spoken}"') + self.detail.set_text(f"Paragraph {caption.index}") + + if scroll and self.transcript.body is not None: + self.transcript.body.scroll_to_block(caption.index) + + self.remove_button.set_visibility(not pending) + self.cancel_button.set_visibility(True) + self.save_button.set_enabled(bool(self.draft())) + + # -- Acting ------------------------------------------------------------------- + + def select_words(self, caption: SRTCaption, start: int, end: int) -> None: + """ + Put the selection, and with it focus, back on the words. + """ + + if self.transcript.body is not None: + self.transcript.body.select_range(caption.index, start, caption.index, end) + + def save(self) -> None: + """ + Save and go back to the text, on the words. + """ + + shown = self.save_draft() + + if shown is None: + return + + caption, start, end = shown + touched = self.editor.words_in_selection(caption.text, start, end) + at = self.editor.comment_index_at(caption, touched[0]) if touched else None + + if at is None: + return + + self.pending = None + self.current = (caption, caption.comments[at]) + self.draw(scroll=False) + self.select_words(caption, start, end) + + def remove(self) -> None: + """ + Take the comment off its words and carry on with the next one. + """ + + shown = self.shown_range() + + if self.pending is not None or shown is None: + return + + caption, start, end = shown + before = self.position() + + if not self.editor.remove_comment(caption, start, end): + return + + self.transcript.refresh() + self.transcript.changed() + + entries = self.entries() + self.current = None + + if entries: + caption_next, comment_next = entries[min(max(before, 0), len(entries) - 1)] + self.show(caption_next, comment_next, scroll=False) + else: + self.container.set_visibility(False) + + self.select_words(caption, start, end) + + def cancel(self) -> None: + """ + Abandon what is typed -- the comment goes back to what it was, a new + one is not made -- and go back to the text, on the words. + """ + + shown = self.shown_range() + + if self.pending is not None: + self.pending = None + + if self.position() >= 0: + self.show(*self.current, scroll=False) + else: + self.current = None + self.container.set_visibility(False) + elif self.current is not None: + self.text.set_value(self.current[1].text) + + if shown is not None: + self.select_words(*shown) + + def close(self) -> None: + """ + Put the panel away, keeping what was typed, and go back to the text. + """ + + shown = self.shown_range() + self.commit() + self.pending = None + self.container.set_visibility(False) + + if shown is not None: + self.select_words(*shown) + else: + self.transcript.focus_caret() + + def go_next(self) -> None: + self.step(1) + + def go_previous(self) -> None: + self.step(-1) + + def step(self, direction: int) -> None: + if self.pending is not None: + return + + self.commit() + + entries = self.entries() + at = self.position() + direction + + if not 0 <= at < len(entries): + return + + caption, comment = entries[at] + self.show(caption, comment, scroll=True) + + # At either end the button just used is disabled; keep focus on + # something that can still take it. + if at == 0: + self.focus(self.next) + elif at == len(entries) - 1: + self.focus(self.previous) + + def go_to_words(self) -> None: + """ + From the card to the text: the words are selected, and the text + scrolled to them. + """ + + shown = self.shown_range() + + if shown is None: + return + + if self.transcript.body is not None: + self.transcript.body.scroll_to_block(shown[0].index) + + self.select_words(*shown) + + # -- Keeping up with the text ------------------------------------------------------ + + def sync(self) -> None: + """ + The text has been redrawn: an undo, a merge, a delete or an edit may + have replaced the comment on screen or taken it away. Find it again, or + shut the panel. A card that is still right is not touched, so what is + being typed in it is not. + """ + + if not self.container.visible: + return + + if self.pending is not None: + if any(caption is self.pending[0] for caption in self.editor.captions): + return + + self.pending = None + self.container.set_visibility(False) + return + + if self.current is None: + return + + caption, comment = self.current + + if any(comment is held for held in caption.comments) and any( + caption is held for held in self.editor.captions + ): + return + + # Replaced rather than gone, as an undo does: the same words in the + # caption of the same number. + for candidate in self.editor.captions: + if candidate.index != caption.index: + continue + + for held in candidate.comments: + if held.words == comment.words: + self.current = (candidate, held) + self.text.set_value(held.text) + self.draw(scroll=False) + return + + self.current = None + self.container.set_visibility(False) + + @staticmethod + def focus(element: ui.element) -> None: + ui.run_javascript( + f"const t = getElement({element.id});" + "const el = t && (t.$el || t);" + "if (el && el.focus) el.focus();" + ) + + def focus_text(self) -> None: + """ + Into the text area, so what is typed next is the comment. + """ + + self.text.run_method("focus") diff --git a/utils/styles.py b/utils/styles.py index 6b5af8f..9481d5e 100644 --- a/utils/styles.py +++ b/utils/styles.py @@ -1444,7 +1444,10 @@ /* A comment. The words it is on are underlined with a dotted line: a pattern rather than a colour alone (1.4.1), and a different one from - the solid bars of the flagged and changed words. It is drawn with + the solid bars of the flagged and changed words. It is 3px thick and + sits 4px below the text, clear of the highlight's own frame lines: + a thinner line closer in was lost against them on words that are + both highlighted and commented. It is drawn with text-decoration, so it needs no room of its own, shows on any wash a word can have, and leaves the box-shadows that draw the other markings alone. Nothing is added to the line: a marker after the last word @@ -1452,61 +1455,12 @@ .comment-word { text-decoration-line: underline; text-decoration-style: dotted; - text-decoration-thickness: 2px; + text-decoration-thickness: 3px; text-decoration-color: var(--color-comment-line); - text-underline-offset: 3px; + text-underline-offset: 4px; text-decoration-skip-ink: none; } - /* The box a comment is written and read in. Absolute against the - component's own root, like the speaker menu. */ - .comment-box { - position: absolute; - z-index: 9500; - box-sizing: border-box; - width: 19rem; - max-width: calc(100% - 1rem); - padding: 0.5rem 0.75rem 0.5rem; - border-radius: 4px; - background-color: var(--color-bg-surface); - color: var(--color-text-primary); - border: 1px solid var(--color-border-subtle); - border-left: 4px solid var(--color-comment-line); - box-shadow: 0 4px 14px var(--color-shadow-medium); - font-size: 0.95rem; - } - .comment-box-quote { - margin-bottom: 0.35rem; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - color: var(--color-text-muted); - font-size: 0.85rem; - } - .comment-box-text { - display: block; - box-sizing: border-box; - width: 100%; - min-height: 4.5rem; - padding: 0.35rem 0.5rem; - resize: vertical; - border: 1px solid var(--color-border-subtle); - border-radius: 4px; - background-color: var(--color-bg-surface); - color: var(--color-text-primary); - font: inherit; - } - .comment-box-actions { - display: flex; - flex-wrap: wrap; - justify-content: flex-end; - gap: 0.25rem; - margin-top: 0.4rem; - } - .comment-box-remove { - margin-right: auto; - } - /* Shared hover message. A CSS box rather than title=, which the browser draws itself and stylesheets cannot reach. @@ -2003,6 +1957,33 @@ .validation-panel.is-muted { --vp-tone: var(--color-border-subtle); } + /* The comments panel, a sibling of this one: the colour of the underline + on the words it is about, which is also the one colour in the editor + that means nothing else (blue is the playing word, and the + progress marks). */ + .validation-panel.is-comment { + --vp-tone: var(--color-comment-line); + } + .comments-panel-words { + max-width: 100%; + } + .comments-panel-words .q-btn__content { + text-align: left; + white-space: normal; + } + .comments-panel-text { + margin-top: 8px; + } + .comments-panel-actions { + display: flex; + flex-wrap: wrap; + justify-content: flex-end; + gap: 4px; + margin-top: 4px; + } + .comments-panel-remove { + margin-right: auto; + } /* Which kinds of issue are stepped through: a row of checkboxes under the panel, ticked while that kind is shown. */ .validation-panel-wrap { diff --git a/utils/transcript_editor.js b/utils/transcript_editor.js index 050f3d5..a6ce0e5 100644 --- a/utils/transcript_editor.js +++ b/utils/transcript_editor.js @@ -282,51 +282,6 @@ export default { /> - - `, props: { @@ -497,23 +452,6 @@ export default { stamps: {}, timed: [], menu: { open: false, x: 0, y: 0, id: null, speaker: null }, - // The comment box. peek is a box opened by a click on commented text: - // it shows the comment without taking focus from the text, and goes - // away at the next key or click. Opened from the key or the button it - // takes focus, and keeps what is typed until it is saved or cancelled. - comment: { - open: false, - x: 0, - y: 0, - id: null, - start: 0, - end: 0, - text: "", - original: "", - quote: "", - existing: false, - peek: false, - }, // The block the caret is in, so the caption being edited can look // like it. Purely presentational, and separate from activeId, which // is the caption the recording is playing -- the reader is very @@ -555,7 +493,6 @@ export default { this.onTime = () => this.markCurrentWord(); this.onOutside = (event) => { if (!event.target.closest(".speaker-menu")) this.closeMenu(); - if (!event.target.closest(".comment-box")) this.leaveComment(); }; this.onEscape = (event) => { if (event.key === "Escape") this.closeMenu(true); @@ -735,159 +672,44 @@ export default { // The toolbar's Comment button, and the key: ask the server to open the // comment on the selected words. The server decides what that means -- - // a new comment, or the one the words already have -- and answers with - // showComment. + // a new comment, or the one the words already have -- and shows it in the + // comments panel under the video. commentSelection() { this.flush(); this.$emit("opencomment", { spans: this.highlightSpans() }); }, - // Open the comment box beside a stretch of a block's text. `payload` is - // what the server worked out: the block, the character range the comment - // is on, its text so far, whether it already exists, and whether the box - // should take focus (not when it was only clicked on). - showComment(payload) { - this.$nextTick(() => { - const block = this.$refs.body?.querySelector( - `.transcript-text[data-id="${payload.id}"]` - ); - if (!block) return; - - const from = this.pointAt(block, payload.start); - const to = this.pointAt(block, payload.end); - if (!from || !to) return; - - const range = document.createRange(); - range.setStart(from.node, from.offset); - range.setEnd(to.node, to.offset); - - // Under the last line of the words, so the box does not cover them. - const rects = range.getClientRects(); - const box = rects.length ? rects[rects.length - 1] : range.getBoundingClientRect(); - const root = this.$el.getBoundingClientRect(); - - // Estimates, only used to keep the box on screen: its width follows - // the stylesheet, and its height is a few lines of text and a row of - // buttons. - const width = 304; - const height = 190; - const gap = 6; - const edge = 8; - - const x = Math.max(edge, Math.min(box.left, window.innerWidth - edge - width)); - let y = box.bottom + gap; - if (y + height > window.innerHeight - edge) { - y = Math.max(edge, box.top - gap - height); - } - - const quote = this.plainText(block.textContent).slice(payload.start, payload.end); - - this.comment = { - open: true, - x: x - root.left, - y: y - root.top, - id: payload.id, - start: payload.start, - end: payload.end, - text: payload.text, - original: payload.text, - quote: quote.length > 60 ? quote.slice(0, 59) + "\u2026" : quote, - existing: !!payload.existing, - peek: !payload.focus, - }; - - if (payload.focus) { - this.$nextTick(() => { - const area = this.$refs.commentText; - if (!area) return; - area.focus(); - area.setSelectionRange(area.value.length, area.value.length); - }); - } - }); - }, - - closeComment() { - this.comment = { ...this.comment, open: false, peek: false }; - }, - - // Put the reader's selection back on the words, the way it was when the - // box opened, so they carry on from where they were. - returnToComment() { - const { id, start, end } = this.comment; - this.selectRange(id, start, id, end); - }, - - saveComment() { - const text = this.comment.text.trim(); - if (!this.comment.open || !text) return; - - const { id, start, end } = this.comment; - this.returnToComment(); - this.closeComment(); - this.$emit("savecomment", { id, start, end, text }); - }, - - removeComment() { - if (!this.comment.open) return; - - const { id, start, end } = this.comment; - this.returnToComment(); - this.closeComment(); - this.$emit("removecomment", { id, start, end }); - }, - - cancelComment() { - if (!this.comment.open) return; - - this.returnToComment(); - this.closeComment(); - }, - - // The reader moved on without choosing: a click elsewhere, or tabbing out - // of the box. What they typed is not thrown away -- it is saved, if there - // is something to save and it is not what was there. Escape, and Cancel, - // are how to abandon it. A peek has nothing typed in it to save. - leaveComment() { - if (!this.comment.open) return; + // The comment the caret is in, as the block and the comment's number in + // it -- what the comments panel under the video follows. Nothing for a + // selection, or a caret in words that have no comment. + commentAtCaret() { + const selection = window.getSelection(); + if (!selection || selection.rangeCount === 0 || !selection.isCollapsed) { + return null; + } - const text = this.comment.text.trim(); + const node = selection.anchorNode; + const element = node && (node.nodeType === 1 ? node : node.parentElement); + const word = element?.closest?.("[data-comment]"); + const block = element ? this.blockOf(element) : null; - if (!this.comment.peek && text && text !== this.comment.original.trim()) { - const { id, start, end } = this.comment; - this.closeComment(); - // stay: the reader has already put the caret somewhere else, and the - // redraw must not take it back to these words. - this.$emit("savecomment", { id, start, end, text, stay: true }); - return; - } + if (!word || !block) return null; - this.closeComment(); + return { id: Number(block.dataset.id), cm: Number(word.dataset.comment) }; }, - // Every key typed in the box stays in it: the page has shortcuts of its - // own (undo, save, play) that would otherwise act on the transcript while - // a sentence is being written. Escape abandons the comment and - // Ctrl/Cmd+Enter saves it. - onCommentKeydown(event) { - event.stopPropagation(); + // Tell the server when the caret comes into a different comment, so the + // panel can show it. Said once per comment rather than per key, and again + // for a click, which also opens the panel if it is shut -- an arrow key + // that happens to cross a commented word does not. + reportCommentCaret(clicked = false) { + const at = this.commentAtCaret(); + const key = at ? `${at.id}:${at.cm}` : null; - if (event.key === "Escape") { - event.preventDefault(); - this.cancelComment(); - } else if (event.key === "Enter" && (event.ctrlKey || event.metaKey)) { - event.preventDefault(); - this.saveComment(); - } - }, + if (key === this.lastCommentKey && !clicked) return; + this.lastCommentKey = key; - onCommentFocusOut(event) { - const next = event.relatedTarget; - if (next && event.currentTarget.contains(next)) return; - // Focus leaving the box for nowhere in particular (the window losing - // focus, say) is not the reader choosing anything. - if (!next) return; - this.leaveComment(); + if (at) this.$emit("caretcomment", { ...at, click: clicked }); }, // The text node and offset a block's character offset falls at. An @@ -1444,6 +1266,7 @@ export default { const at = this.caret(); this.caretId = at ? at.id : null; if (at) this.lastCaret = { id: at.id, offset: at.offset }; + this.reportCommentCaret(); }, // Leaving the editor sends whatever was pending and takes the editing @@ -2083,10 +1906,6 @@ export default { } } - // A box opened by a click was only there to be read; the reader typing - // on has done with it. - if (this.comment.open && this.comment.peek) this.closeComment(); - const at = this.caret(); if (!at) return; @@ -2309,22 +2128,6 @@ export default { const block = this.blockOf(event.target); if (!block) return; - // A click on commented words shows the comment beside them. The server - // knows what it says and answers with showComment; nothing here changes - // where the caret is. - // Not at the end of a drag: someone selecting words to copy them is not - // asking to read the note. - if ( - event.target.closest("[data-comment]") && - !this.subtitleMode && - window.getSelection()?.isCollapsed - ) { - const where = this.caret(); - if (where && where.id === Number(block.dataset.id)) { - this.$emit("peekcomment", { id: where.id, offset: where.offset }); - } - } - // Where in the block the caret landed, so the recording can be moved to // that word rather than to the start of the block. The browser places // the caret on mousedown, so it is already correct by the time click @@ -2333,6 +2136,7 @@ export default { const at = this.caret(); this.caretId = id; + this.reportCommentCaret(true); this.$emit("blockclick", { id, diff --git a/utils/transcript_editor.py b/utils/transcript_editor.py index 507618a..3bea686 100644 --- a/utils/transcript_editor.py +++ b/utils/transcript_editor.py @@ -194,9 +194,6 @@ def select_range( def comment_selection(self) -> None: self.run_method("commentSelection") - def show_comment(self, payload: dict) -> None: - self.run_method("showComment", payload) - def focus_caret(self) -> None: self.run_method("focusCaret") @@ -217,6 +214,9 @@ def __init__(self, editor) -> None: # Called with the index of the caption the caret has come to rest # in -- the validation panel follows it (see caret_moved). self.on_caret: Optional[Callable[[int], None]] = None + # The panel under the video where comments are written and read + # (utils/comments_panel.py), set by the page; transcriptions only. + self.comments_panel = None self.overlay: Optional[ui.element] = None # The speech strip under the video, when there are word timings to # draw one from -- see set_timeline. @@ -600,9 +600,7 @@ def build(self) -> None: "togglehighlight", lambda event: self.toggle_highlight(event.args) ) self.body.on("opencomment", lambda event: self.open_comment(event.args)) - self.body.on("peekcomment", lambda event: self.peek_comment(event.args)) - self.body.on("savecomment", lambda event: self.save_comment(event.args)) - self.body.on("removecomment", lambda event: self.remove_comment(event.args)) + self.body.on("caretcomment", lambda event: self.caret_comment(event.args)) self.body.on("addblock", lambda event: self.add_after(event.args)) self.body.on("deleteblock", lambda event: self.delete(event.args)) self.body.on("blockclick", lambda event: self.seek(event.args)) @@ -672,6 +670,10 @@ def refresh(self) -> None: # And where the captions divide the recording, which the strip under # the video draws. self.refresh_timeline() + # And the comment the panel is showing, which an undo, a merge or a + # delete may have replaced or taken away. + if self.comments_panel is not None: + self.comments_panel.sync() # ── lookup ────────────────────────────────────────────────────────────── @@ -1028,43 +1030,18 @@ def tell_to_select_words(self) -> None: close_button="Close", ) - def show_comment_on( - self, caption: SRTCaption, start: int, end: int, focus: bool - ) -> bool: - """ - Open the comment box for the words around a selection. Says whether - there was anything to open it for. - """ - - target = self.editor.comment_target(caption, start, end) - - if target is None or self.body is None: - return False - - self.body.show_comment( - { - "id": caption.index, - "start": target["start"], - "end": target["end"], - "text": target["text"], - "existing": target["index"] is not None, - "focus": focus, - } - ) - - return True - def open_comment(self, args) -> None: """ The key or the button: comment on the selected words, or open the - comment they already have. + comment they already have -- in the comments panel, which is where a + comment is written, read and changed. A comment belongs to one block, so a selection reaching across blocks is refused. A selection that merely ends at the very edge of the next block, taking none of it, is still within one. """ - if self.editor.data_format == "srt": + if self.editor.data_format == "srt" or self.comments_panel is None: return raw = args.get("spans") if isinstance(args, dict) else None @@ -1114,95 +1091,32 @@ def open_comment(self, args) -> None: caption, start, end = (reaching or taken)[0] - if not self.show_comment_on(caption, start, end, focus=True): + if not self.comments_panel.open_for(caption, start, end): self.tell_to_select_words() - def peek_comment(self, args) -> None: + def caret_comment(self, args) -> None: """ - A click on commented words: show the comment beside them, without - taking focus from the text. Nothing for a click anywhere else. + The caret has come into a comment's words (or a click has landed on + them): the panel shows that comment. """ - if self.editor.data_format == "srt": + if self.editor.data_format == "srt" or self.comments_panel is None: return caption = self.caption(self.block_id(args)) - offset = args.get("offset") if isinstance(args, dict) else None - - if caption is None or isinstance(offset, bool) or not isinstance(offset, int): - return - - target = self.editor.comment_target(caption, offset, offset) - - if target is None or target["index"] is None: - return - - self.show_comment_on(caption, offset, offset, focus=False) - - def comment_args(self, args): - """ - A comment event's block and range, checked, or None. - """ - - caption = self.caption(self.block_id(args)) - start = args.get("start") if isinstance(args, dict) else None - end = args.get("end") if isinstance(args, dict) else None + number = args.get("cm") if isinstance(args, dict) else None if ( caption is None - or isinstance(start, bool) - or isinstance(end, bool) - or not isinstance(start, int) - or not isinstance(end, int) + or isinstance(number, bool) + or not isinstance(number, int) + or not 0 <= number < len(caption.comments) ): - return None - - return caption, start, end - - def save_comment(self, args) -> None: - """ - Keep the text the reader wrote as the comment on those words. - """ - - checked = self.comment_args(args) - text = args.get("text") if isinstance(args, dict) else None - - if checked is None or not isinstance(text, str): - return - - caption, start, end = checked - - if not self.editor.set_comment(caption, start, end, text): return - self.refresh() - self.changed() - - # Offsets are in the text, which a comment does not change, so the - # selection can be put back as it was. - if self.body is not None and not args.get("stay"): - self.body.select_range(caption.index, start, caption.index, end) - - def remove_comment(self, args) -> None: - """ - Take the comment off the words. - """ - - checked = self.comment_args(args) - - if checked is None: - return - - caption, start, end = checked - - if not self.editor.remove_comment(caption, start, end): - return - - self.refresh() - self.changed() - - if self.body is not None: - self.body.select_range(caption.index, start, caption.index, end) + self.comments_panel.follow( + caption, caption.comments[number], click=args.get("click") is True + ) def move_word(self, args) -> None: """ From b8d7ab0a21c9048deaed4e9927ae5f3054c5d954 Mon Sep 17 00:00:00 2001 From: erklu Date: Sun, 18 Oct 2026 09:07:19 +0000 Subject: [PATCH 06/13] Mark where the comments card is in the text, and say where in words (#131) A transcription does not show a caption's number, so "Paragraph N" on the card was something nobody could use. The card now says who is speaking and when the caption starts, as the editor shows them beside the text. And the text shows which comment the card is on, so Previous and Next can be followed: the caption gets a rule and an outline in the comment colour, as the validation panel marks its caption, and the words of the comment get a tint laid over whatever else they are, so a highlight still shows through. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01LCjpD3ehigLiUGH1oVQQAW --- AGENTS.md | 3 +- tests/test_comments.py | 216 ++++++++++++++++++++++++++++++++++++- utils/comments_panel.py | 57 ++++++++-- utils/styles.py | 24 +++++ utils/transcript_editor.js | 23 +++- utils/transcript_editor.py | 36 +++++++ 6 files changed, 347 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9537cb6..5e3a24b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -141,9 +141,10 @@ A note on a stretch of words, with a text of its own: `SRTCaption.comments`, a l - **Rules.** A comment belongs to **one caption**: a selection reaching across blocks is refused with a notice (a selection that merely brushes the edge of the next block is not across). A word has **at most one comment**: asking to comment on words that already have one opens that comment (`comment_target()`), never a second over it, so no drawing, navigation or export has to deal with overlap; `restore_comments()` enforces it on read too. Text is stripped, never empty (Save is disabled, `set_comment()` returns "" for it) and at most `COMMENT_MAX_LENGTH` (1000) characters. Transcriptions only for now: the key is not bound in `subtitleMode`, there is no panel for `data_format == "srt"`, `set_comment`/`remove_comment` refuse, the dialog group is absent and the export option is not offered -- the same reason as highlights (SRT has nowhere to keep it). - **Saved** as `"comments": [{"words": [...], "text": "..."}]` per segment, only when there are any. `restore_comments(saved, text)` ignores anything unusable (junk shapes, empty text, non-int or bool or negative or out-of-range positions, a word already taken by an earlier comment) and cuts an over-long text. -- **The panel** (`utils/comments_panel.py`, `CommentsPanel`) is where a comment is written, read and changed. It is a card in the right column under the video, built like `ValidationPanel` (same `validation-panel*` classes, `.is-comment` sets `--vp-tone` to the comment colour): a head with Previous / position / Next and Close, then the words (a button that goes to them), "Paragraph N", a text area and Remove / Cancel / Save. **One comment at a time**, shut until needed. `pages/srt.py` builds it for transcriptions only (`transcript.comments_panel`), and `TranscriptEditor.refresh()` ends with `comments_panel.sync()` so a comment that an edit or an undo replaced is found again (same caption index, same words) and one that is gone hides the card. +- **The panel** (`utils/comments_panel.py`, `CommentsPanel`) is where a comment is written, read and changed. It is a card in the right column under the video, built like `ValidationPanel` (same `validation-panel*` classes, `.is-comment` sets `--vp-tone` to the comment colour): a head with Previous / position / Next and Close, then the words (a button that goes to them), where they are (`where_text()`: the speaker and the start time, as the editor shows them beside the text -- **not a paragraph number, which a transcription does not show**), a text area and Remove / Cancel / Save. **One comment at a time**, shut until needed. `pages/srt.py` builds it for transcriptions only (`transcript.comments_panel`), and `TranscriptEditor.refresh()` ends with `comments_panel.sync()` so a comment that an edit or an undo replaced is found again (same caption index, same words) and one that is gone hides the card. - **Opening it.** Ctrl+Shift+K (Ctrl only, by `event.code === "KeyK"`, no Alt/Meta) or the toolbar's **Comment** button (`sym_o_add_comment`, beside Highlight, `mousedown` prevented so the selection survives) -> `commentSelection()` -> `opencomment {spans}` -> `TranscriptEditor.open_comment()` -> `CommentsPanel.open_for()`, which shows the existing comment on those words or an empty card for a new one, with focus in the text area. A file that has comments opens with the first one on the card (`open_first()`, no focus, text not moved). - **The caret drives the card** (the same idea as the validation cards). The component reports `caretcomment {id, cm, click}` **once per comment** when the caret enters one (`reportCommentCaret()`, called from `updateCaretBlock()` and from `onClick` with `click: true`), and `CommentsPanel.follow()` shows it. An open card follows the caret; a **shut** card opens only for a *click*, so an arrow key crossing commented words does not throw a card up under someone typing; the comment already shown is left alone. A selection that is not collapsed reports nothing (a drag to copy words does not open it). +- **Marking where the card is, in the text.** `CommentsPanel.mark()` (called from `draw()` and from `sync()`, cleared by `hide()`) tells the component, through `TranscriptEditor.mark_comment()` and the props `commentId` / `commentIndex`, which caption and which comment the card is on. The caption gets `.transcript-cell-commenting` (a 3px rule and a 2px outline in the comment colour, like the validation panel's `-reviewing`, after the editing/invalid rules so it wins) and the words of the comment get a `data-comment-current` attribute (`isCommentOnShow()`), drawn as an 18% tint of the comment colour in a `background-image`, which paints above `background-color`, so a highlight's, flagged or changed word's own wash shows through. An attribute and not a class so `runClass()` is untouched. A comment being written marks its caption only. Previous/Next also scroll the caption to the middle (`scroll_to_block`) without moving the caret or taking focus, so the buttons can be pressed again. Contrast: text at least 6.7:1 (light) / 5.9:1 (dark) on the tint over every wash, the comment line at least 4.3:1 / 3.4:1; pinned in tests. - **What is typed is kept when the reader moves on** (`commit()`): to another comment, into the text, with Previous/Next, or shutting the card -- if it is non-empty and different from what was there. Escape (in the text area) and Cancel are the ways to abandon; Ctrl/Cmd+Enter and Save keep it. Previous/Next are disabled while a new comment is pending. **Keys typed in the text area do not reach the page** (a `keydown` handler with `stopPropagation`; NiceGUI's `ui.keyboard(ignore=[])` is global and listens on the document, so otherwise Ctrl+Z would undo the transcript while a sentence is being written and Ctrl+S would save). After Save, Cancel and Remove the selection goes back on the words (`select_words`). A `sr-only` live label (`spoken`) says "Comment 2 of 5" and the like. - **Drawing.** `review_runs()` gives a commented word `cm` (the comment's index in the caption); the first word also `cf` and `ct` (the text), the last `cl`. The space between two words of the same comment is in it (not across a line break, not between two comments). A word is drawn with `comment-word`: a **dotted 3px underline, 4px below the text**, in `--color-comment-line` (`#a1136a` light, `#ff9fd2` dark: 6.3-9.4:1 on every wash a word can have, except 4.6:1 on the dark highlight, all above 3:1). A pattern and not colour alone (1.4.1), drawn with `text-decoration` so it takes no room, touches no box-shadow (the other markings keep theirs) and needs no `::after` -- which the review/edit words already use for a hover message that the editor turns off to protect the caret, and a marker after the last word would sit where the caret goes. It is thick and offset because a thin line close to the text was lost against the highlight's own frame lines on words that are both highlighted and commented (rendered in Chromium, both themes, wrapped and not). The comment colour is magenta, not blue: blue is the progress/playing tint. The first word has `aria-description="Comment: "`; **not tested with a screen reader**. - **RTF export** (`export_rtf(..., comments=False)`, an "Include comments" checkbox, **off by default and hidden when there is none**, `rtf_comments_available()`, bulk exports included): each comment is written as a Word comment -- `{\*\atrfstart N}` before its words, `{\*\atrfend N}{\*\atnid SC}{\*\atnauthor Scribe}\chatn{\*\annotation{\*\atnref N}{\*\atndate D}\pard\plain\f0\fs20 text}` after (`rtf_date()` packs the date as RTF wants it). The pieces of `marked_spans()` are cut where a comment begins or ends so a marker never lands inside a piece; the text between the markers is the caption's own text throughout. Author is the constant "Scribe" (the editor does not know the reader's name). Checked by converting an export with LibreOffice and by opening it in Word (comments by "Scribe" with the right ranges). Highlights show in the export only with "Include highlights" ticked, which is separate. A clean export is byte-for-byte unchanged. diff --git a/tests/test_comments.py b/tests/test_comments.py index df70f64..1efba84 100644 --- a/tests/test_comments.py +++ b/tests/test_comments.py @@ -823,6 +823,10 @@ def __init__(self): self.scrolled = [] self.asked = 0 self.caret_asked = 0 + self.comment_mark = (-1, -1) + + def set_comment_mark(self, block_id, comment_index=-1): + self.comment_mark = (block_id, comment_index) def comment_selection(self): self.asked += 1 @@ -932,7 +936,7 @@ def test_a_new_comment_opens_empty_on_the_words_with_focus(self, panel, editor): assert panel.text.value == "" assert panel.heading.text == "New comment" assert panel.words_button.text == "\u201ctv\u00e5 tre\u201d" - assert panel.detail.text == "Paragraph 1" + assert panel.detail.text == "A \u00b7 00:00:00.000" assert panel.focused == 1 def test_opening_changes_nothing_by_itself(self, panel, editor): @@ -1592,6 +1596,127 @@ def test_the_validation_panel_is_still_subtitles_only(self): assert 'if data_format == "srt":' in page[start - 200 : start] +class TestWhereItIs: + """ + A transcription does not show a caption's number, so the card says where + a comment is by what is shown beside the text, and the text marks it. + """ + + def test_it_is_the_speaker_and_the_start_time_as_the_editor_shows_them( + self, panel, editor + ): + editor.captions[1].speaker = "Speaker 2" + editor.captions[1].comments = [comment({0}, "here")] + + panel.show(editor.captions[1], editor.captions[1].comments[0]) + + from utils.transcript_editor import format_time_label + + assert panel.detail.text == ( + "Speaker 2 \u00b7 " + format_time_label(editor.captions[1].get_start_seconds()) + ) + assert "Paragraph" not in panel.detail.text + + def test_without_a_speaker_it_is_the_time_alone(self, panel, editor): + from utils.comments_panel import where_text + + editor.captions[0].speaker = "" + + assert where_text(editor.captions[0]) == "00:00:00.000" + + def test_the_comment_on_show_is_marked_in_the_text(self, panel, editor): + editor.captions[1].comments = [comment({0}, "a"), comment({2}, "b")] + + panel.show(editor.captions[1], editor.captions[1].comments[1]) + + assert panel.transcript.body.comment_mark == (editor.captions[1].index, 1) + + def test_a_comment_being_written_marks_its_caption_only(self, panel, editor): + panel.open_for(editor.captions[0], 5, 9) + + assert panel.transcript.body.comment_mark == (editor.captions[0].index, -1) + + def test_stepping_moves_the_marking_with_the_card(self, panel, editor): + editor.captions[0].comments = [comment({0}, "first")] + editor.captions[1].comments = [comment({1}, "second")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + + panel.go_next() + + assert panel.transcript.body.comment_mark == (editor.captions[1].index, 0) + + panel.go_previous() + + assert panel.transcript.body.comment_mark == (editor.captions[0].index, 0) + + def test_following_the_caret_marks_the_comment_it_lands_on(self, panel, editor): + editor.captions[1].comments = [comment({1}, "second")] + + panel.follow(editor.captions[1], editor.captions[1].comments[0], click=True) + + assert panel.transcript.body.comment_mark == (editor.captions[1].index, 0) + + def test_closing_the_card_takes_the_marking_off(self, panel, editor): + editor.captions[0].comments = [comment({0}, "x")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + + panel.close() + + assert panel.transcript.body.comment_mark == (-1, -1) + + def test_a_comment_that_is_gone_takes_the_marking_with_it(self, panel, editor): + editor.captions[0].comments = [comment({0}, "x")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + editor.captions[0].comments = [] + + panel.sync() + + assert panel.container.visible is False + assert panel.transcript.body.comment_mark == (-1, -1) + + def test_the_marking_is_set_again_when_the_comment_has_moved_in_its_caption( + self, panel, editor + ): + editor.captions[0].comments = [comment({2}, "late")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + editor.captions[0].comments.insert(0, comment({0}, "early")) + + panel.sync() + + assert panel.transcript.body.comment_mark == (editor.captions[0].index, 1) + + def test_removing_the_last_comment_takes_it_off(self, panel, editor): + editor.captions[0].comments = [comment({1}, "only")] + panel.show(editor.captions[0], editor.captions[0].comments[0]) + + panel.remove() + + assert panel.transcript.body.comment_mark == (-1, -1) + + def test_the_body_keeps_the_marking_and_ignores_a_repeat(self): + from utils.transcript_editor import TranscriptBody + + body = TranscriptBody.__new__(TranscriptBody) + body._props = {"commentId": -1, "commentIndex": -1} + updates = [] + body.update = lambda: updates.append(1) + + body.set_comment_mark(3, 1) + body.set_comment_mark(3, 1) + + assert (body._props["commentId"], body._props["commentIndex"]) == (3, 1) + assert len(updates) == 1 + + def test_the_editor_clears_it_with_none(self, view): + marks = [] + view.body.set_comment_mark = lambda *a: marks.append(a) + + view.mark_comment(None) + view.mark_comment(2, 0) + + assert marks == [(-1, -1), (2, 0)] + + class TestPanelHelpers: def test_the_quote_is_one_line(self): from utils.comments_panel import quote_of @@ -1683,6 +1808,16 @@ def test_the_classes_come_from_the_one_function(self): assert 'run.cm !== undefined ? "comment-word" : null' in source assert 'run.cl ? "comment-end" : null' in source + def test_the_comment_on_show_is_marked_on_its_caption_and_its_words(self): + source = self.source() + + assert "commentId: { type: Number, default: -1 }" in source + assert "commentIndex: { type: Number, default: -1 }" in source + assert "'transcript-cell-commenting': block.id === commentId" in source + assert "block.id === this.commentId" in source + assert "run.cm === this.commentIndex" in source + assert source.count("isCommentOnShow(run, block)") == 3 # two templates and the method + def test_nothing_of_the_old_box_is_left(self): source = self.source() @@ -1726,6 +1861,85 @@ def test_nothing_is_added_after_the_last_word(self): assert ".comment-end::after" not in css assert ".comment-word::after" not in css + def test_the_caption_on_show_has_a_rule_and_an_outline_in_the_comment_colour(self): + rule = self.rule(" .transcript-cell-commenting {") + + assert "border-left-width: 3px;" in rule + assert "inset 0 0 0 2px var(--color-comment-line)" in rule + assert "var(--color-comment-line)" in rule + + def test_it_comes_after_the_rules_it_has_to_win_over(self): + css = self.css() + at = css.index(" .transcript-cell-commenting {") + + for earlier in ( + " .transcript-cell-editing {", + " .transcript-cell-invalid {", + " .transcript-cell-highlighted {", + ): + assert css.index(earlier) < at + + def test_the_words_on_show_get_a_tint_that_leaves_their_own_wash_showing(self): + rule = self.rule(" .comment-word[data-comment-current] {") + + # background-image paints above background-color, so a highlighted, + # flagged or changed word keeps its own wash underneath. + assert "background-image" in rule + assert "background-color" not in rule + assert "18%" in rule + assert "padding" not in rule + assert "box-shadow" not in rule + + @pytest.mark.parametrize( + "line,walls", + [ + ( + "#a1136a", + [ + ("#ffffff", "#111827"), + ("#fff1a3", "#2b2400"), + ("#ede9fb", "#2f1c66"), + ("#dcf2ee", "#10453d"), + ("#dbe4f0", "#111827"), + ], + ), + ( + "#ff9fd2", + [ + ("#16181d", "#e6e8eb"), + ("#5a4a00", "#fff6cc"), + ("#2f2748", "#ece7ff"), + ("#1d3b37", "#dff5f0"), + ("#1c3252", "#e6e8eb"), + ], + ), + ], + ) + def test_the_tint_keeps_the_text_and_the_line_readable(self, line, walls): + def rgb(colour): + return [int(colour[i : i + 2], 16) for i in (1, 3, 5)] + + def luminance(channels): + linear = [ + (c / 255) / 12.92 + if c / 255 <= 0.03928 + else ((c / 255 + 0.055) / 1.055) ** 2.4 + for c in channels + ] + return 0.2126 * linear[0] + 0.7152 * linear[1] + 0.0722 * linear[2] + + def ratio(a, b): + lighter, darker = sorted((luminance(a), luminance(b)), reverse=True) + return (lighter + 0.05) / (darker + 0.05) + + for wall, text in walls: + tinted = [ + round(w * 0.82 + t * 0.18) for w, t in zip(rgb(wall), rgb(line)) + ] + + assert ratio(rgb(text), tinted) >= 4.5 + assert ratio(rgb(line), tinted) >= 3 + def test_the_panel_has_the_colour_of_the_underline(self): rule = self.rule(" .validation-panel.is-comment {") diff --git a/utils/comments_panel.py b/utils/comments_panel.py index 6a6be9a..95804c8 100644 --- a/utils/comments_panel.py +++ b/utils/comments_panel.py @@ -37,6 +37,7 @@ from utils.caption import SRTCaption, WordComment from utils.srt_comments import COMMENT_MAX_LENGTH +from utils.transcript_editor import format_time_label # The words a comment is on, as shown in the panel and read out. QUOTE_LENGTH = 80 @@ -63,6 +64,17 @@ def heading_text(pending: bool, position: int, total: int) -> str: return f"Comment {position_text(position, total)}" +def where_text(caption: SRTCaption) -> str: + """ + Where a comment is, in what the reader can see beside the text: who is + speaking and when (the caption's number is not shown in a transcription). + """ + + when = format_time_label(caption.get_start_seconds()) + + return f"{caption.speaker} · {when}" if caption.speaker else when + + class CommentsPanel: """ Built once, hidden, under the video. @@ -165,7 +177,7 @@ def __init__(self, transcript) -> None: "Save", on_click=self.save ).props("dense no-caps") - self.container.set_visibility(False) + self.hide() # -- What there is ------------------------------------------------------ @@ -276,6 +288,34 @@ def commit(self) -> bool: def value_changed(self) -> None: self.save_button.set_enabled(bool(self.draft())) + def hide(self) -> None: + """ + Put the card away, and take its marking off the text. + """ + + self.container.set_visibility(False) + self.transcript.mark_comment(None) + + def mark(self) -> None: + """ + Mark in the text what the card is about: the caption, and for a + comment already written, its words. + """ + + if not self.container.visible: + self.transcript.mark_comment(None) + elif self.pending is not None: + self.transcript.mark_comment(self.pending[0].index) + elif self.current is not None: + caption, comment = self.current + at = next( + (i for i, held in enumerate(caption.comments) if held is comment), + -1, + ) + self.transcript.mark_comment(caption.index, at) + else: + self.transcript.mark_comment(None) + # -- Showing ---------------------------------------------------------------- def show( @@ -387,7 +427,7 @@ def draw(self, scroll: bool = True) -> None: self.words_button.set_text(f"“{quote_of(caption.text[start:end])}”") spoken = quote_of(caption.text[start:end]).replace('"', "'") self.words_button.props(f'aria-label="Go to the words: {spoken}"') - self.detail.set_text(f"Paragraph {caption.index}") + self.detail.set_text(where_text(caption)) if scroll and self.transcript.body is not None: self.transcript.body.scroll_to_block(caption.index) @@ -395,6 +435,7 @@ def draw(self, scroll: bool = True) -> None: self.remove_button.set_visibility(not pending) self.cancel_button.set_visibility(True) self.save_button.set_enabled(bool(self.draft())) + self.mark() # -- Acting ------------------------------------------------------------------- @@ -454,7 +495,7 @@ def remove(self) -> None: caption_next, comment_next = entries[min(max(before, 0), len(entries) - 1)] self.show(caption_next, comment_next, scroll=False) else: - self.container.set_visibility(False) + self.hide() self.select_words(caption, start, end) @@ -473,7 +514,7 @@ def cancel(self) -> None: self.show(*self.current, scroll=False) else: self.current = None - self.container.set_visibility(False) + self.hide() elif self.current is not None: self.text.set_value(self.current[1].text) @@ -488,7 +529,7 @@ def close(self) -> None: shown = self.shown_range() self.commit() self.pending = None - self.container.set_visibility(False) + self.hide() if shown is not None: self.select_words(*shown) @@ -554,10 +595,11 @@ def sync(self) -> None: if self.pending is not None: if any(caption is self.pending[0] for caption in self.editor.captions): + self.mark() return self.pending = None - self.container.set_visibility(False) + self.hide() return if self.current is None: @@ -568,6 +610,7 @@ def sync(self) -> None: if any(comment is held for held in caption.comments) and any( caption is held for held in self.editor.captions ): + self.mark() return # Replaced rather than gone, as an undo does: the same words in the @@ -584,7 +627,7 @@ def sync(self) -> None: return self.current = None - self.container.set_visibility(False) + self.hide() @staticmethod def focus(element: ui.element) -> None: diff --git a/utils/styles.py b/utils/styles.py index 9481d5e..1b22a0b 100644 --- a/utils/styles.py +++ b/utils/styles.py @@ -1932,6 +1932,30 @@ box-shadow: inset 0 0 0 2px var(--color-warning-border); } + /* The caption the comments card under the video is on, marked as the + validation panel marks its caption but in the comment colour: a + thicker rule and a full outline, so shape and not only colour say + which. After the editing and invalid rules, so it wins over them. */ + .transcript-cell-commenting { + border-left-color: var(--color-comment-line); + border-left-width: 3px; + padding-left: calc(1rem - 2px); + box-shadow: inset 0 0 0 2px var(--color-comment-line); + } + /* And the words of the comment the card is showing: a tint of the comment + colour laid over whatever else the word has. It is a background-image, + which paints above background-color, so a highlight's or a flagged + word's own wash still shows through it and nothing is taken away. At + 18% the text keeps at least 6.7:1 (light) and 5.9:1 (dark) on every + wash a word can have, and the comment line at least 4.3:1 and 3.4:1 + against it. */ + .comment-word[data-comment-current] { + background-image: linear-gradient( + color-mix(in srgb, var(--color-comment-line) 18%, transparent), + color-mix(in srgb, var(--color-comment-line) 18%, transparent) + ); + } + /* ── Validation panel, under the video (issue #138) ── */ /* Design B: a stripe down the left and a tinted header in the colour of the issue shown -- the panel's own is-warning / is-error / diff --git a/utils/transcript_editor.js b/utils/transcript_editor.js index a6ce0e5..6b58f35 100644 --- a/utils/transcript_editor.js +++ b/utils/transcript_editor.js @@ -86,7 +86,7 @@ export default { @keydown="onControlKeydown($event, block.id, 'speaker')" >{{ block.speaker }}:
Delete caption
{{ run.t }}
{{ run.t }}
{{ run.t }}
+ >{{ run.t }}