Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 15 additions & 14 deletions src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.less
Original file line number Diff line number Diff line change
Expand Up @@ -76,24 +76,25 @@ div.ui-audioCurrent p {
position: unset; // BL-11633, works around Chromium bug
}

::highlight(bloom-audio-split-1) {
background-color: #bfedf3;
}

::highlight(bloom-audio-split-2) {
background-color: #7fdae6;
}

::highlight(bloom-audio-split-3) {
background-color: #29c2d6;
}

.ui-audioCurrent.bloom-postAudioSplit[data-audiorecordingmode="TextBox"]:not(
.ui-suppressHighlight
):not(.ui-disableHighlight) {
// Special highlighting after the Split button completes to show it completed.
// Note: This highlighting is expected to persist across sessions, but to be hidden (displayed with the yellow color) while each segment is playing.
// This is accomplished because this rule temporarily drops out of effect when .ui-audioCurrent is moved to the span as that segment plays.
// (The rule requires a span BELOW the .ui-audioCurrent, so it drops out of effect the span IS the .ui-audioCurrent).
span:nth-child(3n + 1 of .bloom-highlightSegment) {
background-color: #bfedf3;
}

span:nth-child(3n + 2 of .bloom-highlightSegment) {
background-color: #7fdae6;
}

span:nth-child(3n + 3 of .bloom-highlightSegment) {
background-color: #29c2d6;
}
// The actual colors are now applied with named ::highlight() rules populated from TS.
// Note: This highlighting is expected to persist across sessions, but to be hidden
// (displayed with the yellow color) while each segment is playing.

span {
position: unset; // BL-11633, works around Chromium bug
Expand Down
28 changes: 28 additions & 0 deletions src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import {
} from "../../../react_components/featureStatus";
import { animateStyleName } from "../../../utils/shared";
import jQuery from "jquery";
import { AudioTextHighlightManager } from "./audioTextHighlightManager";

enum Status {
Disabled, // Can't use button now (e.g., Play when there is no recording)
Expand Down Expand Up @@ -207,6 +208,7 @@ export default class AudioRecording implements IAudioRecorder {

private playbackOrderCache: IPlaybackOrderInfo[] = [];
private disablingOverlay: HTMLDivElement;
private audioTextHighlightManager = new AudioTextHighlightManager();

constructor(maySetHighlight: boolean = true) {
this.audioSplitButton = <HTMLButtonElement>(
Expand Down Expand Up @@ -898,6 +900,10 @@ export default class AudioRecording implements IAudioRecorder {
if (pageDocBody) {
this.removeAudioCurrent(pageDocBody);
}

this.audioTextHighlightManager.clearManagedHighlights(
pageDocBody ?? undefined,
);
}

private removeAudioCurrent(parentElement: Element) {
Expand Down Expand Up @@ -997,6 +1003,7 @@ export default class AudioRecording implements IAudioRecorder {

if (oldElement === newElement && !forceRedisplay) {
// No need to do much, and better not to so we can avoid any temporary flashes as the highlight is removed and re-applied
this.refreshAudioTextHighlights(newElement);
return;
}

Expand Down Expand Up @@ -1069,6 +1076,21 @@ export default class AudioRecording implements IAudioRecorder {
);
}
}

this.refreshAudioTextHighlights(newElement);
}

private refreshAudioTextHighlights(currentHighlight?: Element | null) {
const activeHighlight = currentHighlight ?? this.getCurrentHighlight();
const currentTextBox = activeHighlight
? ((this.getTextBoxOfElement(
activeHighlight,
) as HTMLElement | null) ?? null)
: null;
this.audioTextHighlightManager.refreshSplitHighlights(
activeHighlight,
currentTextBox,
);
}

// Scrolls an element into view.
Expand Down Expand Up @@ -4375,6 +4397,7 @@ export default class AudioRecording implements IAudioRecorder {
const currentTextBox = this.getCurrentTextBox();
if (currentTextBox) {
currentTextBox.classList.add("bloom-postAudioSplit");
this.refreshAudioTextHighlights(currentTextBox);
}
}

Expand All @@ -4384,6 +4407,10 @@ export default class AudioRecording implements IAudioRecorder {
currentTextBox.classList.remove("bloom-postAudioSplit");
currentTextBox.removeAttribute("data-audioRecordingEndTimes");
}

this.audioTextHighlightManager.clearManagedHighlights(
currentTextBox ?? undefined,
);
Comment on lines +4411 to +4413

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 clearAudioSplit silently skips highlight cleanup when currentTextBox is null

At audioRecording.ts:4411-4413, if getCurrentTextBox() returns null, clearManagedHighlights receives undefined and returns immediately at audioTextHighlightManager.ts:48-49 — leaving any existing ::highlight() entries in the registry. With the old CSS approach, removing the bloom-postAudioSplit class from the element was sufficient since the CSS selectors would stop matching. With the new approach, the registry entries persist independently. In practice this is mitigated because removeAudioCurrentFromPageDocBody also clears highlights and is called on most highlight transitions. But if a code path calls clearAudioSplit expecting to remove the visual split colors, and no highlight change follows, the colors could linger.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}

private getElementsToUpdateForCursor(): (Element | null)[] {
Expand Down Expand Up @@ -4823,6 +4850,7 @@ export default class AudioRecording implements IAudioRecorder {
}
});
this.nodesToRestoreAfterPlayEnded.clear();
this.refreshAudioTextHighlights();
}
}

Expand Down
129 changes: 129 additions & 0 deletions src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,70 @@ import { RecordingMode } from "./recordingMode";
import axios from "axios";
import $ from "jquery";
import { mockReplies } from "../../../utils/bloomApi";
import { splitHighlightNames } from "./audioTextHighlightManager";

class FakeHighlight {
public ranges: Range[];

public constructor(...ranges: Range[]) {
this.ranges = ranges;
}
}

type FakeHighlightRegistry = Map<string, FakeHighlight>;
type TestCssWithHighlights = {
highlights?: FakeHighlightRegistry;
};

const installCustomHighlightPolyfill = (targetWindow: Window) => {
const targetWindowWithCss = targetWindow as Window & {
CSS?: TestCssWithHighlights;
};
if (!targetWindowWithCss.CSS) {
targetWindowWithCss.CSS = {};
}

const cssWithHighlights = targetWindowWithCss.CSS;
cssWithHighlights.highlights = new Map<string, FakeHighlight>();
(
targetWindow as Window & {
Highlight?: typeof FakeHighlight;
}
).Highlight = FakeHighlight;
};

const getPageWindow = (): Window | undefined => {
const iframe = parent.window.document.getElementById(
"page",
) as HTMLIFrameElement | null;
return iframe?.contentWindow ?? undefined;
};

const getCustomHighlightsRegistry = (): FakeHighlightRegistry => {
const targetWindow = getPageWindow() ?? globalThis.window;
const cssWithHighlights = (
targetWindow as Window & {
CSS?: TestCssWithHighlights;
}
).CSS;
if (!cssWithHighlights?.highlights) {
throw new Error(
"Expected CSS.highlights test polyfill to be installed",
);
}

return cssWithHighlights.highlights;
};

const getSplitHighlightTexts = (): string[][] => {
const registry = getCustomHighlightsRegistry();
return splitHighlightNames.map((name) => {
const highlight = registry.get(name);
return highlight
? highlight.ranges.map((range) => range.toString())
: [];
});
};

// Notes:
// For any async tests:
Expand All @@ -30,13 +94,21 @@ import { mockReplies } from "../../../utils/bloomApi";

describe("audio recording tests", () => {
beforeAll(async () => {
installCustomHighlightPolyfill(globalThis.window);

await setupForAudioRecordingTests();

const pageWindow = getPageWindow();
if (pageWindow) {
installCustomHighlightPolyfill(pageWindow);
}
});

afterEach(() => {
// Clean up any pending timers to prevent "parent is not defined" errors
// when tests finish before timers fire
theOneAudioRecorder?.clearTimeouts();
getCustomHighlightsRegistry().clear();
});

// In an earlier version of our API, checkForAnyRecording was designed to fail (404) if there was no recording.
Expand Down Expand Up @@ -2059,6 +2131,63 @@ describe("audio recording tests", () => {
});
});
});

describe("- custom split highlights", () => {
it("registers the split highlight color groups for the current text box", () => {
SetupIFrameFromHtml(
'<div id="page1"><div class="bloom-translationGroup"><div id="box1" class="bloom-editable audio-sentence ui-audioCurrent bloom-postAudioSplit" data-audiorecordingmode="TextBox" data-audiorecordingendtimes="1.0 2.0 3.0"><p><span id="span1" class="bloom-highlightSegment">One.</span><span id="span2" class="bloom-highlightSegment">Two.</span><span id="span3" class="bloom-highlightSegment">Three.</span><span id="span4" class="bloom-highlightSegment">Four.</span></p></div></div></div>',
);

const recording = new AudioRecording();
recording.markAudioSplit();

expect(getSplitHighlightTexts()).toEqual([
["One.", "Four."],
["Two."],
["Three."],
]);
});

it("clears split highlights while playback moves to an individual segment", async () => {
SetupIFrameFromHtml(
'<div id="page1"><div class="bloom-translationGroup"><div id="box1" class="bloom-editable audio-sentence ui-audioCurrent bloom-postAudioSplit" data-audiorecordingmode="TextBox" data-audiorecordingendtimes="1.0 2.0"><p><span id="span1" class="bloom-highlightSegment">One.</span><span id="span2" class="bloom-highlightSegment">Two.</span></p></div></div></div>',
);

const recording = new AudioRecording();
recording.markAudioSplit();

const setHighlightToAsync = (
recording as unknown as {
setHighlightToAsync(args: {
newElement: Element;
shouldScrollToElement: boolean;
}): Promise<void>;
}
).setHighlightToAsync.bind(recording);

await setHighlightToAsync({
newElement: getFrameElementById("page", "span1")!,
shouldScrollToElement: false,
});

expect(getSplitHighlightTexts()).toEqual([[], [], []]);
});

it("uses only ui-enableHighlight descendants when a segment disables its own background", () => {
SetupIFrameFromHtml(
'<div id="page1"><div class="bloom-translationGroup"><div id="box1" class="bloom-editable audio-sentence ui-audioCurrent bloom-postAudioSplit" data-audiorecordingmode="TextBox" data-audiorecordingendtimes="1.0 2.0"><p><span id="span1" class="bloom-highlightSegment">One.</span><span id="span2" class="bloom-highlightSegment ui-disableHighlight"><span class="ui-enableHighlight">Two</span>&nbsp;&nbsp; <span class="ui-enableHighlight">Three</span></span></p></div></div></div>',
);

const recording = new AudioRecording();
recording.markAudioSplit();

expect(getSplitHighlightTexts()).toEqual([
["One."],
["Two", "Three"],
[],
]);
});
});
});

function StripEmptyClasses(html) {
Expand Down
Loading