From e307c1eefbee7ff8fa4a00daaed3c10c47cab879 Mon Sep 17 00:00:00 2001 From: John Thomson Date: Mon, 1 Jun 2026 13:00:26 -0500 Subject: [PATCH 1/2] Fix change field handling of narration (BL-16368) - changing to None, or something that doesn't have text already: keeps the current text and any recording, but recording get a new ID and is no longer locked (or duplicated) - changing to something that has a value: also gets any existing recording. --- .../js/CanvasElementContextControls.tsx | 247 ++++++++++++------ .../bookEdit/js/CanvasElementManager.ts | 35 ++- .../web/controllers/EditingViewApi.cs | 27 +- 3 files changed, 224 insertions(+), 85 deletions(-) diff --git a/src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx b/src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx index babb2bae3b1c..d7515f6cc599 100644 --- a/src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx +++ b/src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx @@ -1017,22 +1017,26 @@ function adjustAutoSizeForVisibleEditableInTranslationGroup(tg: HTMLElement) { OverflowChecker.AdjustSizeOrMarkOverflow(visibleEditable); } -function setEditableContentFromKnownDataBookValueIfAny( +async function setEditableContentFromKnownDataBookValueIfAny( editable: HTMLElement, dataBook: string | null, tg: HTMLElement, ) { if (!dataBook) { - return; + return true; } - wrapWithRequestPageContentDelay( + return wrapWithRequestPageContentDelay( () => - new Promise((resolve, reject) => { + new Promise((resolve, reject) => { get( `editView/getDataBookValue?lang=${editable.getAttribute("lang")}&dataBook=${dataBook}`, (result) => { try { - const content = result.data; + const response = + typeof result.data === "string" + ? { content: result.data } + : result.data; + const content = response.content; // content comes from a source that looked empty, we don't want to overwrite something the user may // already have typed here. // But it may well have something in it, because we usually have an empty paragraph to start with. @@ -1043,13 +1047,19 @@ function setEditableContentFromKnownDataBookValueIfAny( // the user can always correct it back to what he just typed. const temp = document.createElement("div"); temp.innerHTML = content || ""; - if (temp.textContent.trim() !== "") { + const keptExistingContent = + temp.textContent.trim() === ""; + if (!keptExistingContent) { editable.innerHTML = content; + applyEditableAudioIdentityFromDataBookValue( + editable, + response, + ); } adjustAutoSizeForVisibleEditableInTranslationGroup( tg, ); - resolve(); + resolve(keptExistingContent); } catch (error) { reject(error); } @@ -1063,6 +1073,53 @@ function setEditableContentFromKnownDataBookValueIfAny( ); } +function applyEditableAudioIdentityFromDataBookValue( + editable: HTMLElement, + dataBookValue: { + id?: string; + dataAudioRecordingMode?: string; + dataDuration?: string; + dataAudioRecordingEndTimes?: string; + recordingMd5?: string; + hasAudioSentenceClass?: boolean; + hasBloomPostAudioSplitClass?: boolean; + }, +) { + const syncAttribute = (attributeName: string, value?: string) => { + if (value) { + editable.setAttribute(attributeName, value); + } else { + editable.removeAttribute(attributeName); + } + }; + + if (dataBookValue.id) { + editable.setAttribute("id", dataBookValue.id); + } + + syncAttribute( + "data-audiorecordingmode", + dataBookValue.dataAudioRecordingMode, + ); + syncAttribute("data-duration", dataBookValue.dataDuration); + syncAttribute( + "data-audiorecordingendtimes", + dataBookValue.dataAudioRecordingEndTimes, + ); + syncAttribute("recordingmd5", dataBookValue.recordingMd5); + + if (dataBookValue.hasAudioSentenceClass) { + editable.classList.add("audio-sentence"); + } else { + editable.classList.remove("audio-sentence"); + } + if (dataBookValue.hasBloomPostAudioSplitClass) { + editable.classList.add("bloom-postAudioSplit"); + } else { + editable.classList.remove("bloom-postAudioSplit"); + } +} + function applyAppearanceClassForEditable(editable: HTMLElement) { editable.classList.remove( "bloom-contentFirst", @@ -1153,7 +1210,7 @@ function makeLanguageMenuItem( } } - setEditableContentFromKnownDataBookValueIfAny( + void setEditableContentFromKnownDataBookValueIfAny( editableInLang, dataBookValue, tg, @@ -1369,25 +1426,30 @@ function makeFieldTypeMenuItem( l10nId: null, english: noneLabel, onClick: () => { - clearFieldTypeClasses(); - for (const editable of Array.from( - tg.getElementsByClassName("bloom-editable"), - ) as HTMLElement[]) { - editable.removeAttribute("data-book"); - // There's a bit of guess-work involved in what would be most helpful here. - // clearFieldTypeClasses removes any field-type-specific style class, - // and we generally expect a bloom-editable to have some style class. - // Should it be Normal-style or Bubble-style? Bubble-style is the default - // for canvas elements, so I decided to go with that. - const hasStyleClass = Array.from(editable.classList).some( - (className) => className.endsWith("-style"), - ); - if (!hasStyleClass) { - editable.classList.add("Bubble-style"); + void wrapWithRequestPageContentDelay(async () => { + clearFieldTypeClasses(); + for (const editable of Array.from( + tg.getElementsByClassName("bloom-editable"), + ) as HTMLElement[]) { + theOneCanvasElementManager?.makeEditableAudioIndependent( + editable, + ); + editable.removeAttribute("data-book"); + // There's a bit of guess-work involved in what would be most helpful here. + // clearFieldTypeClasses removes any field-type-specific style class, + // and we generally expect a bloom-editable to have some style class. + // Should it be Normal-style or Bubble-style? Bubble-style is the default + // for canvas elements, so I decided to go with that. + const hasStyleClass = Array.from( + editable.classList, + ).some((className) => className.endsWith("-style")); + if (!hasStyleClass) { + editable.classList.add("Bubble-style"); + } } - } - adjustAutoSizeForVisibleEditableInTranslationGroup(tg); - setMenuOpen(false); + adjustAutoSizeForVisibleEditableInTranslationGroup(tg); + setMenuOpen(false); + }, "setCanvasFieldTypeToNone"); }, icon: !activeType && , }, @@ -1397,71 +1459,90 @@ function makeFieldTypeMenuItem( l10nId: null, english: fieldType.label, onClick: () => { - clearFieldTypeClasses(); - const editables = Array.from( - tg.getElementsByClassName("bloom-editable"), - ) as HTMLElement[]; - if (fieldType.readOnly) { - const readOnlyDiv = document.createElement("div"); - readOnlyDiv.setAttribute( - "data-derived", - fieldType.dataDerived, - ); - if (fieldType.hint) { - readOnlyDiv.setAttribute("data-hint", fieldType.hint); - } - if (fieldType.functionOnHintClick) { + void wrapWithRequestPageContentDelay(async () => { + clearFieldTypeClasses(); + const editables = Array.from( + tg.getElementsByClassName("bloom-editable"), + ) as HTMLElement[]; + if (fieldType.readOnly) { + const readOnlyDiv = document.createElement("div"); readOnlyDiv.setAttribute( - "data-functiononhintclick", - fieldType.functionOnHintClick, + "data-derived", + fieldType.dataDerived, ); - } - readOnlyDiv.classList.add(...fieldType.classes); - tg.parentElement!.insertBefore(readOnlyDiv, tg); - tg.remove(); - // Tempting to do this here, but we're going to reload the page, - // and it's only after the reload that the div we just created will have - // its content. Another call to this function will happen after the reload, - // and that can do a better job of fixing it. Doing it now would just - // reduce the height to its minimum, forcing the new content to be shown on - // one line. - //ensureFieldFitsOnCustomPage(readOnlyDiv); - // Reload the page to get the derived content loaded. - post("common/saveChangesAndRethinkPageEvent", () => {}); - } else { - removeConflictingStyleClasses(fieldType, editables); - tg.classList.add(...fieldType.classes); - for (const editable of editables) { - editable.classList.add(...fieldType.editableClasses); - editable.setAttribute("data-book", fieldType.dataBook); - if ( - fieldsControlledByAppearanceSystem.includes( - fieldType.dataBook, - ) - ) { - applyAppearanceClassForEditable(editable); - } else { - editable.classList.remove( - "bloom-contentFirst", - "bloom-contentSecond", - "bloom-contentThird", + if (fieldType.hint) { + readOnlyDiv.setAttribute( + "data-hint", + fieldType.hint, ); } - if ( - editable.classList.contains( - "bloom-visibility-code-on", - ) - ) { - setEditableContentFromKnownDataBookValueIfAny( - editable, + if (fieldType.functionOnHintClick) { + readOnlyDiv.setAttribute( + "data-functiononhintclick", + fieldType.functionOnHintClick, + ); + } + readOnlyDiv.classList.add(...fieldType.classes); + tg.parentElement!.insertBefore(readOnlyDiv, tg); + tg.remove(); + // Tempting to do this here, but we're going to reload the page, + // and it's only after the reload that the div we just created will have + // its content. Another call to this function will happen after the reload, + // and that can do a better job of fixing it. Doing it now would just + // reduce the height to its minimum, forcing the new content to be shown on + // one line. + //ensureFieldFitsOnCustomPage(readOnlyDiv); + // Reload the page to get the derived content loaded. + post("common/saveChangesAndRethinkPageEvent", () => {}); + } else { + const fieldTypeChanged = + activeType !== fieldType.dataBook; + removeConflictingStyleClasses(fieldType, editables); + tg.classList.add(...fieldType.classes); + for (const editable of editables) { + editable.classList.add( + ...fieldType.editableClasses, + ); + editable.setAttribute( + "data-book", fieldType.dataBook, - tg, ); + if ( + fieldsControlledByAppearanceSystem.includes( + fieldType.dataBook, + ) + ) { + applyAppearanceClassForEditable(editable); + } else { + editable.classList.remove( + "bloom-contentFirst", + "bloom-contentSecond", + "bloom-contentThird", + ); + } + let keptExistingContent = false; + if ( + editable.classList.contains( + "bloom-visibility-code-on", + ) + ) { + keptExistingContent = + await setEditableContentFromKnownDataBookValueIfAny( + editable, + fieldType.dataBook, + tg, + ); + } + if (fieldTypeChanged && keptExistingContent) { + theOneCanvasElementManager?.makeEditableAudioIndependent( + editable, + ); + } } + adjustAutoSizeForVisibleEditableInTranslationGroup(tg); } - adjustAutoSizeForVisibleEditableInTranslationGroup(tg); - } - setMenuOpen(false); + setMenuOpen(false); + }, "setCanvasFieldType"); }, icon: activeType === fieldType.dataBook && ( diff --git a/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts b/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts index 809302870cf6..eb069d9002bc 100644 --- a/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts +++ b/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts @@ -6183,7 +6183,10 @@ export class CanvasElementManager { return patriarchDuplicateElement; } - private copyAnySoundFileAndAttributesForEditable( + /// + /// Copies any audio-related ids and files from one editable canvas field to another. + /// + public copyAnySoundFileAndAttributesForEditable( sourceElement: HTMLElement, copiedElement: HTMLElement, ): void { @@ -6225,6 +6228,36 @@ export class CanvasElementManager { } } + /// + /// Makes audio-sentence ids in the editable independent by assigning new ids and copying + /// any corresponding audio files when they exist. + /// + public makeEditableAudioIndependent(editable: HTMLElement): void { + if (!editable) { + return; + } + + const audioElements: Element[] = []; + if ( + editable.classList.contains("audio-sentence") && + editable.getAttribute("id") + ) { + audioElements.push(editable); + } + + audioElements.push( + ...Array.from(editable.querySelectorAll(".audio-sentence[id]")), + ); + + audioElements.forEach((audioElement) => { + const oldId = audioElement.getAttribute("id"); + if (!oldId) { + return; + } + this.copySoundFileAndAttributes(audioElement, oldId, audioElement); + }); + } + private copySoundFileAndAttributes( sourceElement: Element, sourceId: string, diff --git a/src/BloomExe/web/controllers/EditingViewApi.cs b/src/BloomExe/web/controllers/EditingViewApi.cs index 92dfdb86a905..b56a5d13806b 100644 --- a/src/BloomExe/web/controllers/EditingViewApi.cs +++ b/src/BloomExe/web/controllers/EditingViewApi.cs @@ -157,7 +157,32 @@ private void HandleGetDataBookValue(ApiRequest request) var dataBook = request.RequiredParam("dataBook"); var multiText = View.Model.CurrentBook.BookData.GetMultiTextVariableOrEmpty(dataBook); var value = multiText.GetExactAlternative(lang) ?? ""; - request.ReplyWithText(value); + var matchingDataDivElement = + View.Model.CurrentBook.RawDom.SelectSingleNode( + $"//div[@id='bloomDataDiv']/div[@data-book='{dataBook}' and @lang='{lang}']" + ) as SafeXmlElement; + + request.ReplyWithJson( + new + { + content = value, + id = matchingDataDivElement?.GetAttribute("id"), + dataAudioRecordingMode = matchingDataDivElement?.GetAttribute( + "data-audiorecordingmode" + ), + dataDuration = matchingDataDivElement?.GetAttribute("data-duration"), + dataAudioRecordingEndTimes = matchingDataDivElement?.GetAttribute( + "data-audiorecordingendtimes" + ), + recordingMd5 = matchingDataDivElement?.GetAttribute("recordingmd5"), + hasAudioSentenceClass = matchingDataDivElement + ?.GetAttribute("class") + ?.Contains("audio-sentence") == true, + hasBloomPostAudioSplitClass = matchingDataDivElement + ?.GetAttribute("class") + ?.Contains("bloom-postAudioSplit") == true, + } + ); } /// From 9aff65d91936d7ad717d7e677e8458d978c54c5b Mon Sep 17 00:00:00 2001 From: John Thomson Date: Mon, 8 Jun 2026 16:54:28 -0500 Subject: [PATCH 2/2] Post-review improvements --- .../bookEdit/js/CanvasElementContextControls.tsx | 5 +---- src/BloomExe/web/controllers/EditingViewApi.cs | 11 +++++------ 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx b/src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx index d7515f6cc599..1899b01f2eac 100644 --- a/src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx +++ b/src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx @@ -1032,10 +1032,7 @@ async function setEditableContentFromKnownDataBookValueIfAny( `editView/getDataBookValue?lang=${editable.getAttribute("lang")}&dataBook=${dataBook}`, (result) => { try { - const response = - typeof result.data === "string" - ? { content: result.data } - : result.data; + const response = result.data; const content = response.content; // content comes from a source that looked empty, we don't want to overwrite something the user may // already have typed here. diff --git a/src/BloomExe/web/controllers/EditingViewApi.cs b/src/BloomExe/web/controllers/EditingViewApi.cs index b56a5d13806b..e18475e896a1 100644 --- a/src/BloomExe/web/controllers/EditingViewApi.cs +++ b/src/BloomExe/web/controllers/EditingViewApi.cs @@ -175,12 +175,11 @@ private void HandleGetDataBookValue(ApiRequest request) "data-audiorecordingendtimes" ), recordingMd5 = matchingDataDivElement?.GetAttribute("recordingmd5"), - hasAudioSentenceClass = matchingDataDivElement - ?.GetAttribute("class") - ?.Contains("audio-sentence") == true, - hasBloomPostAudioSplitClass = matchingDataDivElement - ?.GetAttribute("class") - ?.Contains("bloom-postAudioSplit") == true, + hasAudioSentenceClass = matchingDataDivElement?.HasClass("audio-sentence") + ?? false, + hasBloomPostAudioSplitClass = matchingDataDivElement?.HasClass( + "bloom-postAudioSplit" + ) ?? false, } ); }