Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ export default class StyleEditor {
this.sentenceHiliteRuleSelector,
false,
);
const hiliteTextColor = sentenceRule?.style?.color;
const hiliteTextColor = sentenceRule?.style?.color || undefined;
let hiliteBgColor = sentenceRule?.style?.backgroundColor;
if (!hiliteBgColor) {
hiliteBgColor = kBloomYellow;
Expand Down
94 changes: 78 additions & 16 deletions src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditorSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,12 @@ function ChangeSizeAbsolute(
editor.changeSizeInternal(newSize + "pt", shouldSetDefaultRule);
}

function GetUserModifiedStyleSheet(): any {
function GetUserModifiedStyleSheet(): CSSStyleSheet | undefined {
for (let i = 0; i < document.styleSheets.length; i++) {
if (document.styleSheets[i].title == "userModifiedStyles")
if (document.styleSheets[i].title === "userModifiedStyles")
return <CSSStyleSheet>document.styleSheets[i];
}
// this is not a valid constructor
//return new CSSStyleSheet();
return {};
return undefined;
}

function GetFooStyleRuleFontSize(): number {
Expand All @@ -83,8 +81,8 @@ function ParseRuleForFontSize(ruleText: string): number {
}

function GetRuleForFooStyle(): CSSRule | null {
const x: CSSRuleList = (<CSSStyleSheet>GetUserModifiedStyleSheet())
.cssRules;
const x = GetUserModifiedStyleSheet()?.cssRules;
if (!x) return null;

for (let i = 0; i < x.length; i++) {
if (x[i].cssText.indexOf("foo-style") > -1) {
Expand All @@ -95,8 +93,7 @@ function GetRuleForFooStyle(): CSSRule | null {
}

function GetRuleForNormalStyle(): CSSRule | null {
const x: CSSRuleList = (<CSSStyleSheet>GetUserModifiedStyleSheet())
.cssRules;
const x = GetUserModifiedStyleSheet()?.cssRules;
if (!x) return null;

for (let i = 0; i < x.length; i++) {
Expand All @@ -108,8 +105,7 @@ function GetRuleForNormalStyle(): CSSRule | null {
}

function GetRuleForCoverTitleStyle(): CSSRule | null {
const x: CSSRuleList = (<CSSStyleSheet>GetUserModifiedStyleSheet())
.cssRules;
const x = GetUserModifiedStyleSheet()?.cssRules;
if (!x) return null;
for (let i = 0; i < x.length; i++) {
if (x[i].cssText.indexOf("Title-On-Cover-style") > -1) {
Expand All @@ -128,8 +124,9 @@ function GetCalculatedFontSize(target: string): number {
}

function GetRuleMatchingSelector(selector: string): CSSRule | null {
const x = (<CSSStyleSheet>GetUserModifiedStyleSheet()).cssRules;
const count = 0;
const sheet = GetUserModifiedStyleSheet();
const x = sheet?.cssRules;
if (!x) return null;
for (let i = 0; i < x.length; i++) {
if (x[i].cssText.indexOf(selector) > -1) {
return x[i];
Expand All @@ -139,7 +136,9 @@ function GetRuleMatchingSelector(selector: string): CSSRule | null {
}

function HasRuleMatchingThisSelector(selector: string): boolean {
const x = (<CSSStyleSheet>GetUserModifiedStyleSheet()).cssRules;
const sheet = GetUserModifiedStyleSheet();
const x = sheet?.cssRules;
if (!x) return false;
let count = 0;
for (let i = 0; i < x.length; i++) {
if (x[i].cssText.indexOf(selector) > -1) {
Expand All @@ -150,8 +149,8 @@ function HasRuleMatchingThisSelector(selector: string): boolean {
}

function countFooStyleRules(): number {
const x: CSSRuleList = (<CSSStyleSheet>GetUserModifiedStyleSheet())
.cssRules;
const x = GetUserModifiedStyleSheet()?.cssRules;
if (!x) return 0;

let count = 0;
for (let i = 0; i < x.length; i++) {
Expand Down Expand Up @@ -320,6 +319,69 @@ describe("StyleEditor", () => {
if (rule != null) expect(ParseRuleForFontSize(rule.cssText)).toBe(20);
});

it("putAudioHiliteRulesInDom stores audio highlight legacy properties", () => {
const editor = new StyleEditor(
"file://" + "C:/dev/Bloom/src/BloomBrowserUI/bookEdit",
);

const sentenceSelector = "foo-style span.ui-audioCurrent";
const paddedSentenceSelector =
"foo-style span.ui-audioCurrent > span.ui-enableHighlight";
const paragraphSelector = "foo-style.ui-audioCurrent p";

// sanity check that the rules do not yet exist
expect(HasRuleMatchingThisSelector(sentenceSelector)).toBeFalsy();
expect(HasRuleMatchingThisSelector(paddedSentenceSelector)).toBeFalsy();
expect(HasRuleMatchingThisSelector(paragraphSelector)).toBeFalsy();

editor.putAudioHiliteRulesInDom(
"foo-style",
"rgb(1, 2, 3)",
"rgb(4, 5, 6)",
);

const sentenceRule = GetRuleMatchingSelector(sentenceSelector);
const paddedSentenceRule = GetRuleMatchingSelector(
paddedSentenceSelector,
);
const paragraphRule = GetRuleMatchingSelector(paragraphSelector);

expect(sentenceRule?.cssText).toContain(
"background-color: rgb(4, 5, 6)",
);
expect(sentenceRule?.cssText).toContain("color: rgb(1, 2, 3)");
expect(paddedSentenceRule?.cssText).toContain(
"background-color: rgb(4, 5, 6)",
);
expect(paddedSentenceRule?.cssText).toContain("color: rgb(1, 2, 3)");
expect(paragraphRule?.cssText).toContain(
"background-color: rgb(4, 5, 6)",
);
expect(paragraphRule?.cssText).toContain("color: rgb(1, 2, 3)");
});

it("getAudioHiliteProps reads colors from audio highlight legacy properties", () => {
const editor = new StyleEditor(
"file://" + "C:/dev/Bloom/src/BloomBrowserUI/bookEdit",
);

const origProps = editor.getAudioHiliteProps("foo-style");

expect(origProps?.hiliteTextColor).not.toBe("rgb(1, 2, 3)");
expect(origProps?.hiliteBgColor).not.toBe("rgb(4, 5, 6)");

editor.putAudioHiliteRulesInDom(
"foo-style",
"rgb(1, 2, 3)",
"rgb(4, 5, 6)",
);

const props = editor.getAudioHiliteProps("foo-style");

expect(props.hiliteTextColor).toBe("rgb(1, 2, 3)");
expect(props.hiliteBgColor).toBe("rgb(4, 5, 6)");
});

// Skipped because currently we're running in jsdom. Making use of the existing rule depends on
// getComputedStyle, which jsdom does not support. ChatGpt thinks it also depends on actual
// dom element sizes, which jsdom also does not support. Attempts to polyfill proved difficult.
Expand Down
60 changes: 33 additions & 27 deletions src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.less
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ div.ui-audioCurrent:not(.ui-suppressHighlight):not(.ui-disableHighlight) p {
// color: @textOnLightBackground;
}

// In the Edit tab, we now use pseudo-elements to display the highlight instead of just background-color nd color, to avoid
// computed styles getting stuck in the book (BL-15300, see audioTextHighlightManager.ts). Here overwrite the highlighting
// colors from basePage.less which are otherwise used to display highlighting (e.g. in Bloom Player), so that only the
// pseudo-elements are displaying the highlights
body.bloom-audio-customHighlights {
span.ui-audioCurrent:not(.ui-suppressHighlight):not(.ui-disableHighlight),
div.ui-audioCurrent:not(.ui-suppressHighlight):not(.ui-disableHighlight) p,
.ui-audioCurrent .ui-enableHighlight {
background-color: transparent !important;
color: inherit !important;
}
}

.bloom-ui-current-audio-marker:before {
background-image: url(currentTextIndicator.svg);
background-repeat: no-repeat;
Expand Down Expand Up @@ -76,33 +89,26 @@ div.ui-audioCurrent p {
position: unset; // BL-11633, works around Chromium bug
}

.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;
}

span {
position: unset; // BL-11633, works around Chromium bug
}

p {
// Override the normal yellow highlight so it doesn't clash with the ones we just added
background: none;
}
// Use pseudoelements to display TBT highlighting in the Edit tab. See audioTextHighlightManager.ts
::highlight(bloom-audio-current) {
// Read from CSS variables so the Format dialog's stored audio highlight colors
// control the normally-yellow recording highlight
background-color: var(
--bloom-audio-current-highlight-background,
@highlightColor
);
color: var(--bloom-audio-current-highlight-color, @textOnLightBackground);
}
Comment thread
nabalone marked this conversation as resolved.
// Note: This highlighting is expected to persist across sessions, but to be hidden
// (displayed with the yellow color) while each segment is playing.
::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-audioBody {
Expand Down
43 changes: 39 additions & 4 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.clearAllManagedHighlights(
pageDocBody ?? undefined,
);
}

private removeAudioCurrent(parentElement: Element) {
Expand Down Expand Up @@ -996,7 +1002,9 @@ 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
// The current element is unchanged, so avoid tearing down and rebuilding anything in the DOM.
// We still need to refresh the custom-highlight pseudoelement state so yellow/split highlights stay in sync without a flash.
this.refreshAudioTextHighlights(newElement);
return;
}

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

this.refreshAudioTextHighlights(newElement);
Comment thread
nabalone marked this conversation as resolved.
}

private refreshAudioTextHighlights(currentHighlight?: Element | null) {
const activeHighlight = currentHighlight ?? this.getCurrentHighlight();
const currentTextBox = activeHighlight
? ((this.getTextBoxOfElement(
activeHighlight,
) as HTMLElement | null) ?? null)
: null;
// The manager keeps both the yellow current highlight and the blue post-split
// highlights in sync so callers do not need separate refresh paths.
this.audioTextHighlightManager.refreshHighlights(
activeHighlight,
currentTextBox,
);
}

// Scrolls an element into view.
Expand Down Expand Up @@ -1166,6 +1191,12 @@ export default class AudioRecording implements IAudioRecorder {
}

this.resetAudioIfPaused();
// According to copilot:
// Clear any split-complete state before rebuilding the current highlight.
// Otherwise the custom-highlight logic will still treat the textbox as "post split"
// and suppress the yellow current highlight we want to show as Speak starts.
this.clearAudioSplit();

// If we were paused highlighting one sentence but are recording in text box mode,
// things could get confusing. At least make sure the selection reflects what we
// actually want to record.
Expand All @@ -1181,8 +1212,6 @@ export default class AudioRecording implements IAudioRecorder {

const id = this.getCurrentAudioId();

this.clearAudioSplit();

return axios
.post("/bloom/api/audio/startRecord?id=" + id)
.then(() => {
Expand Down Expand Up @@ -2042,7 +2071,7 @@ export default class AudioRecording implements IAudioRecorder {
this.updateDisplay();
}
private showPlaybackOrderUi(docBody: HTMLElement) {
this.removeAudioCurrent(docBody);
this.removeAudioCurrentFromPageDocBody();
this.playbackOrderCache = [];
const translationGroups = this.getVisibleTranslationGroups(docBody);
if (translationGroups.length < 1) {
Expand Down Expand Up @@ -4375,6 +4404,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 +4414,10 @@ export default class AudioRecording implements IAudioRecorder {
currentTextBox.classList.remove("bloom-postAudioSplit");
currentTextBox.removeAttribute("data-audioRecordingEndTimes");
}

this.audioTextHighlightManager.clearSplitHighlights(
currentTextBox ?? undefined,
);
}
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.

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

Expand Down
Loading