Skip to content

BL-16071 Fix blank decodable reader tool#7827

Open
nabalone wants to merge 1 commit intomasterfrom
BL-16071_fix_blank_decodable_reader_tool
Open

BL-16071 Fix blank decodable reader tool#7827
nabalone wants to merge 1 commit intomasterfrom
BL-16071_fix_blank_decodable_reader_tool

Conversation

@nabalone
Copy link
Copy Markdown
Contributor

@nabalone nabalone commented Apr 8, 2026


Open with Devin

This change is Reviewable

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 4 files

@nabalone nabalone force-pushed the BL-16071_fix_blank_decodable_reader_tool branch from cb2e1ce to 98ceab1 Compare April 8, 2026 23:40
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚩 Pre-existing: ReaderToolSwitch render-time code only removes 'turned-off', never re-adds it

The ReaderToolSwitch component at src/BloomBrowserUI/bookEdit/toolbox/readers/ReaderToolSwitch.tsx:18-22 only removes the turned-off class when checked is true, but never adds it back when checked is false. This means when switching from a reader book to a non-reader book, the tool content div will remain without turned-off, and isToggleOff() at src/BloomBrowserUI/bookEdit/toolbox/readers/readerTools.ts:638-643 will return false, causing setMarkupType(1) or setMarkupType(2) to be called for a non-reader book. This is a pre-existing issue (the same code path runs via showTool()createToggle()) and is not introduced or worsened by this PR. The onChange handler correctly uses classList.toggle('turned-off', !checked) to handle user-initiated toggles. The fix would be to also add turned-off during render when checked is false.

(Refers to lines 18-22)

Open in Devin Review

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant