diff --git a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts index 3afef01cc193..977d0dbe2275 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts +++ b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts @@ -4126,6 +4126,16 @@ export default class AudioRecording implements IAudioRecorder { ); this.updateButtonStateHelper(expectedVerb, response); + + // The helper just recomputed this.haveAudio from the server, for whatever element is + // current now. The Advanced section's Recording Mode radio buttons are driven by values + // derived from it, so they have to be recomputed here too. Several paths that change which + // box is current (clicking in another text box, Next/Back, typing new text) refresh the + // buttons through here and nothing else, which left the radio buttons describing the + // *previous* box. That is BL-16632: after recording By Whole Text Box and then clicking a + // box with no recording, "By Sentence" stayed disabled, saying to use Clear first, even + // though Clear itself was (correctly) disabled because that box has no audio. + this.updateAudioStateInDisplay(); } private updateButtonStateHelper( @@ -4484,6 +4494,13 @@ export default class AudioRecording implements IAudioRecorder { // 1- When calling changeStateAndSetExpected() with expectedVerb = "" // 2- While doing auto segmenting // 3- An unrecoverable error has occurred + // Every path that gets here has nothing recordable selected: the page has no editable + // text, the click landed on something that can't be recorded, or we could not work out + // what is current. So the current selection has no audio, by definition. Saying so keeps + // the Advanced section honest -- otherwise it goes on reporting the audio of the box we + // were on before, which is the BL-16632 mismatch ("By Sentence" disabled telling you to + // press Clear, while Clear is disabled because there is nothing to clear). + this.haveAudio = false; this.setStatus("record", Status.Disabled); this.setStatus("play", Status.Disabled); this.setStatus("split", Status.Disabled); @@ -4491,6 +4508,12 @@ export default class AudioRecording implements IAudioRecorder { this.setStatus("prev", Status.Disabled); this.setStatus("clear", Status.Disabled); this.setStatus("listen", Status.Disabled); + + // The Advanced section's controls (the Recording Mode radio buttons, Insert Segment + // Marker) are driven by derived values that have to be refreshed here too. Otherwise + // deleting all the text on a page disables every button but leaves those controls enabled, + // still describing the text that used to be there (BL-16632). + this.updateAudioStateInDisplay(); } private showBusy(): void { @@ -4738,6 +4761,30 @@ export default class AudioRecording implements IAudioRecorder { await this.setShowPlaybackOrderMode(isOn); } + // Recompute just the parts of the display that depend on the current audio state: whether the + // current selection has a recording, whether that recording is a whole-text-box one, and + // whether the page has anything recordable. This is what the button-state refresh invalidates + // when it recomputes this.haveAudio, so that refresh calls this rather than the whole of + // updateDisplay (BL-16632). Deliberately narrow in two ways: + // - It does not publish the *mode* fields (recordingMode, inShowPlaybackOrderMode, + // showingImageDescriptions). Those belong to the code that changes those modes, and + // republishing them from a button refresh fights it -- e.g. it would undo the uiState reset + // removePlaybackOrderUi deliberately makes on page change while leaving its own field set. + // - It never *changes* anything, so it can't disturb a caller further up the stack: no + // highlight is chosen (which would recurse back here via getCurrentTextBox), and + // this.recordingMode is left alone -- updateDisplay's subscription downgrade could + // otherwise switch the mode out from under button statuses just computed for the old one. + private updateAudioStateInDisplay(): void { + const hasRecordableDivs = + this.getRecordableDivs(true, false).length > 0; + const currentPlaybackMode = this.getCurrentPlaybackMode(false); + this.uiState.haveACurrentTextboxModeRecording = + this.haveAudio && currentPlaybackMode === RecordingMode.TextBox; + this.uiState.hasAudio = this.haveAudio; + this.uiState.hasRecordableDivs = hasRecordableDivs; + this.notifyStateChanged(); + } + private updateDisplay(maySetHighlight = true): void { // It's a bit expensive to do the test for text present, but without it, // Import Recording will be improperly enabled on an empty page. diff --git a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts index 7640ad46c9ee..fa6bfb431893 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts +++ b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts @@ -1763,6 +1763,183 @@ describe("audio recording tests", () => { }); }); + // BL-16632: The Recording Mode radio buttons are driven by uiState.hasAudio and + // uiState.haveACurrentTextboxModeRecording, which are derived from this.haveAudio. The + // button-state refresh recomputes this.haveAudio from the server whenever the current + // element changes, so anything that refreshes the buttons must also refresh those derived + // flags. Otherwise "By Sentence" keeps saying 'first use the "Clear" button to remove your + // recording' about a box that has no recording (and whose Clear button is disabled). + describe("- recording mode enablement when the current box changes (BL-16632)", () => { + // Only box1 has a recording. + function setupApiResponsesWithRecordingForBox1() { + vi.spyOn(axios, "get").mockImplementation((url: string) => { + if (url.includes(kAnyRecordingApiUrl)) { + return Promise.resolve({ data: url.includes("box1") }); + } else { + return Promise.reject("Fake 404 error 16632."); + } + }); + } + + // A page where box1 has been recorded By Whole Text Box and box2 has text the user + // just typed but has never recorded. Both are marked up for TextBox recording, which + // is what the tool does to a newly typed box while that mode is selected. + function setupPageWithRecordedBox1AndUnrecordedBox2() { + const box1 = + '
This box has been recorded.
This box has never been recorded.
Text the user just typed.
"; + box1.classList.add("audio-sentence"); + ( + recording as unknown as { highlightedElement: HTMLElement } + ).highlightedElement = box1; + + await recording.changeStateAndSetExpectedAsync("record"); + + expect(recording.uiState.hasRecordableDivs).toBe(true); + }); + + // The mirror image: when the last of the text goes away, the tool disables every button + // (the empty expected verb), and the Advanced section's controls have to follow. They used + // to stay enabled, still describing the text that had just been deleted. + it("disables the recording modes once the last of the text is deleted", async () => { + setupDefaultApiResponses(); + SetupIFrameFromHtml( + 'Text that is about to be deleted.
This box has been recorded.
