Fix "By Sentence" left disabled on a text box with no recording (BL-16632) by andrew-polk · Pull Request #8147 · BloomBooks/BloomDesktop

andrew-polk · 2026-08-04T18:32:44Z

The Talking Book tool's Recording Mode radio buttons are driven by values derived from whether the
current text box has a recording (hasAudio, haveACurrentTextboxModeRecording,
hasRecordableDivs). Only a full updateDisplay() recomputed those, while the buttons (Record,
Play, Clear …) were refreshed every time the current box changed — so the radio buttons went on
describing the box you were on before.

After recording By Whole Text Box and then clicking a box with no recording — or typing into a
newly added page — "By Sentence" stayed disabled, its tooltip telling you to use Clear first, while
the Clear button itself was correctly disabled because the current box has no audio.

The fix. The button-state refresh now ends by calling a new, deliberately narrow
updateAudioStateInDisplay(), which publishes just those three audio-derived values and changes
nothing else. It is narrow in two ways that matter:

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;
it never changes anything: no highlight is chosen (which would recurse back in via
getCurrentTextBox), and this.recordingMode is left alone, so updateDisplay's
subscription downgrade can't move the mode out from under button statuses just computed.

Two related holes found in review are fixed too:

disableInteraction() published the derived values without refreshing haveAudio first, so the
Advanced section could still report the audio of the box we had just left. Every path that gets
there has nothing recordable selected, so it now says so.
Deleting the last of the text on a page disabled every button but left the Recording Mode radios
and Insert Segment Marker enabled, still describing text that was gone.

Tests. Five regression tests in audioRecordingSpec.ts, each confirmed to fail without its
fix: the highlight moving to an unrecorded box; typing into a page that started out empty;
deleting the last of the text; clicking something that cannot be recorded; and (on the branch
below) a stale server answer arriving after a page change.

Not in this PR: the related race — a checkForAnyRecording answer about the previous box
arriving after the page changed — which is why the card's own repro steps ("record By Whole Text
Box, add a new page, type") only sometimes show the problem. That is a separate, pre-existing
defect; the guard for it (each refresh takes a generation number; only the most recent may apply
what it hears back) is held on the branch BL-16632-stale-button-state-answers pending a decision
about whether it ships with this regression fix.

Not verified live in Bloom: reproducing by hand needs an actual microphone recording. QA test ideas
are on the card.

Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16632

Devin review

This change is

…bled after recording "By Whole Text Box" and clicking different text area https://issues.bloomlibrary.org/youtrack/issue/BL-16632 The Recording Mode radio buttons in the Talking Book tool's Advanced section are driven by uiState.hasAudio, uiState.haveACurrentTextboxModeRecording and uiState.hasRecordableDivs. Those are derived values, and only updateDisplay() recomputes them. But this.haveAudio (which they derive from) is recomputed from the server by the button-state refresh, and several paths that change which text box is current -- clicking in another box, Next/Back, typing new text -- go through that refresh without otherwise calling updateDisplay(). So the radio buttons kept describing the box you were on before: after recording By Whole Text Box and then clicking a box with no recording (or typing into a newly added page), "By Sentence" stayed disabled and its tooltip said to use Clear first -- while the Clear button itself was correctly disabled, because the current box has no audio. Fix: updateButtonStateAsync() now ends by calling updateDisplay(false), so the derived flags are recomputed wherever the button state is. It passes false for maySetHighlight because refreshing what the UI shows must not have the side effect of choosing a new current element. Tests: added two regression tests in audioRecordingSpec.ts, one per repro path in the card (moving the highlight to an unrecorded box; typing into a page that started out empty). Both fail without the fix and pass with it. Ran the front-end suite (pnpm test): 583 passed, 5 skipped, 0 failed; also pnpm typecheck (clean) and pnpm lint (0 errors). Not verified live in Bloom, since reproducing it by hand requires actually recording audio with a microphone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

#8147 Two review findings on the preceding commit, both about the same derived uiState the fix is concerned with. Devin flagged that updateDisplay() is not purely a display refresh: when the WholeTextBoxAudio subscription feature is off it can switch recordingMode from TextBox to Sentence. Calling it at the end of updateButtonStateAsync therefore let the mode change out from under the button statuses that method had just computed for the mode it read at the top. updateDisplay now takes a maySwitchToSentenceMode argument, and the button-state refresh passes false, so that refresh only reports state and the subscription downgrade keeps happening where it did before. The local review found the mirror image of the second bug in the previous commit: changeStateAndSetExpectedAsync("") disables every button and returns before any refresh, so deleting the last of the text on a page left the Recording Mode radio buttons and Insert Segment Marker enabled, still describing the text that had just been deleted. disableInteraction() now refreshes the derived values too. Tests: added a regression test for the deleted-text case (fails without the fix). talkingBook suites 250 passed. pnpm typecheck clean, pnpm lint 0 errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…L-16632) #8147 Devin's re-review raised three findings on the previous commit which turned out to share one root cause: calling the whole of updateDisplay() from the button-state refresh republished *all* of the derived state, when the refresh only invalidates the part derived from this.haveAudio. So the refresh now calls a narrow updateAudioStateInDisplay() instead: it publishes just hasAudio, haveACurrentTextboxModeRecording and hasRecordableDivs, and changes nothing. That removes all three: - it no longer republishes recordingMode, so a subscription tier without whole-text-box audio can't have an un-downgraded mode pushed back out; - it no longer republishes inShowPlaybackOrderMode, so it can't undo the uiState reset removePlaybackOrderUi makes on page change while deliberately leaving its own field set; - it never touches this.recordingMode, so the mode cannot change under button statuses just computed for the old one. The maySwitchToSentenceMode argument added in the previous commit for that purpose is therefore gone again -- updateDisplay is back to its original signature and behavior. Devin also caught that disableInteraction() published the derived values without first refreshing this.haveAudio, so the Advanced section could still report the audio of the box we had just left. Every path that reaches disableInteraction has nothing recordable selected, so it now says so. Tests: added a regression test for clicking something that cannot be recorded (fails without the haveAudio reset). talkingBook suites 252 passed. pnpm typecheck clean, pnpm lint 0 errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

andrew-polk · 2026-08-04T19:08:31Z

[Claude Opus 5 (1M context) from Polk's machine during preflight] Consulted Devin on 2026-08-04 (UTC) up to commit a1f1cd7a61.

Three passes, one per commit on this branch. No Bugs in any of them. What it raised:

Fixed — the display refresh could switch the recording mode out from under button statuses just computed for the old mode (thread), the Advanced section reporting the previous box's audio when everything is disabled, an un-downgraded recording mode being republished for subscriptions without whole-text-box audio, and the playback-order reset being undone on page change. Each has its own resolved thread above.
Assessed as not an issue — the narrow refresh not applying the subscription downgrade: the one case where the two formulas differ is a page with nothing recordable, where "By Sentence" is disabled either way, so it cannot be seen. Reasoning is on its thread.
Informational, not acted on — 3 items in pass 1, 3 in pass 2, and 6 in pass 3 (several repeats). Two are worth knowing: the page's text is now re-checked on every button refresh (a tester is asked to watch for typing stutter), and stale server answers can still reach the Advanced section — that is the race deliberately left to the branch BL-16632-stale-button-state-answers. One was a correction to the PR description, which has been rewritten to match what the code actually does.

CI (pr-automation) passed on every commit; no other review bot is active on this repo (.coderabbit.yml has auto-review disabled).

andrew-polk and others added 2 commits August 3, 2026 17:14

Merge remote-tracking branch 'origin/master' into BL-16632

4931b53