Skip to content

Improve Leveled Reader tool UI (BL-16585) - #8087

Open
hatton wants to merge 11 commits into
masterfrom
BL-16585ImproveLeveledReaderTBUI
Open

Improve Leveled Reader tool UI (BL-16585)#8087
hatton wants to merge 11 commits into
masterfrom
BL-16585ImproveLeveledReaderTBUI

Conversation

@hatton

@hatton hatton commented Jul 20, 2026

Copy link
Copy Markdown
Member

Redesigns the Leveled Reader toolbox panel to match the approved mockup (BL-16585).

Highlights

  • Over-level warning banner + within/over-level color legend.
  • Section headers uppercased with a consistent underline rule; "This Page"/"This Book" sub-labels get half-em spacing.
  • Stats table: MEASURE column header, right-aligned quiet-grey Max values, em dash for no-limit Max cells, first-letter-capitalized row labels.
  • Level indicator split into a large "Level N" + a small muted "of M" on one line, with boxed prev/next arrow buttons (shared ReaderToolNav, so the Decodable tool's "Stage N of M" gets the same treatment).
  • Teal "Set up Levels" link with a colorable edit icon; outlined "Copy Book Stats" button.
  • Single horizontal panel padding so every row shares one left edge; scrollbar-gutter: stable reserves scrollbar width on the reader panels.
  • "For this Level" hides when the level has no real reminders.

Localization

  • Split the combined LevelNofM / StageNofM strings into LevelNumber / StageNumber + a shared ReaderTools.OfCount (old strings marked obsolete as of 6.5) so the two parts can be styled separately.
  • New strings: banner/legend in BloomMediumPriority.xlf; Measure and the split level/stage strings in Bloom.xlf. English (DistFiles/localization/en/) only.

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

Devin review


This change is Reviewable

Redesign the Leveled Reader toolbox panel to match the approved mockup:
- Over-level warning banner and a within/over-level color legend.
- Section headers uppercased with a consistent underline rule; "This Page"/
  "This Book" sub-labels get half-em breathing room.
- Stats table: a MEASURE column header, right-aligned quiet-grey Max values,
  em dash for no-limit Max cells, first-letter-capitalized row labels.
- Level indicator split into a large "Level N" plus a small muted "of M",
  on one line, with boxed prev/next arrow buttons. This required splitting
  the localized "Level {0} of {1}" / "Stage {0} of {1}" strings into
  LevelNumber/StageNumber + a shared ReaderTools.OfCount (old combined
  strings marked obsolete as of 6.5).
- Teal "Set up Levels" link with a colorable edit icon; outlined
  "Copy Book Stats" button.
- Single horizontal panel padding so every row shares one left edge.
- Reserve scrollbar width (scrollbar-gutter: stable) on the reader panels.
- Hide "For this Level" when the level has no real reminders (an empty
  field comes back as [""]).

New localization strings added to the English XLF files (banner/legend to
BloomMediumPriority; Measure and the split level/stage strings to Bloom.xlf).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR redesigns the Leveled Reader toolbox panel and unifies UI elements across the Decodable Reader tool to match an approved mockup (BL-16585). The localization layer is refactored to split combined LevelNofM/StageNofM strings into independently-styled parts, and shared scrollbar styling is introduced via the new bloomDarkScrollbars CSS class.

  • New readerToolStyles.ts centralises accent colours, muted-text colours, section-header CSS, and contained/text button CSS so both reader tool panels share one design language.
  • isBookOverLevel computes the over-level banner state by re-checking all stat pairs with the same > comparison and 1-decimal rounding used by StatsRow, correctly excluding the two hardcoded-maxNum=0 rows.
  • Scrollbar unification via .bloomDarkScrollbars (class-based opt-in with & * descendant rule) is applied to the toolbox, page-thumbnail list, Collection tab, and the Publish tab's dark tab strip.

Important Files Changed

Filename Overview
src/BloomBrowserUI/bookEdit/toolbox/readers/leveledReader/LeveledReaderToolControls.tsx Major UI redesign adding OverLevelBanner, StatsLegend, conditional 'For this Level' section, and unified bottom action group; isBookOverLevel logic correctly mirrors StatsRow coloring for all 12 stat pairs.
src/BloomBrowserUI/bookEdit/toolbox/readers/readerToolStyles.ts New shared styles file exporting accent/muted colour tokens, section-header CSS, and contained/text button CSS used by both reader tool panels.
src/BloomBrowserUI/bookEdit/toolbox/readers/ReaderToolNav.tsx Navigation redesigned from a flat layout to a flex row with labelled 'Level {0}' + muted 'of {1}' and boxed arrow buttons; both reader tools share this component.
src/BloomBrowserUI/bookEdit/toolbox/readers/ReaderSetupButton.tsx New shared 'Set Up Levels/Stages' button component wrapping BloomButton with a contained blue style and background-image icon.
src/BloomBrowserUI/bookEdit/toolbox/readers/decodableReader/DecodableReaderToolControls.tsx Refactored to use shared styles, replaced the plain Link with a BloomButton for 'Generate Report', added uniform panel padding, and restructured the bottom action group to mirror the leveled reader layout.
src/BloomBrowserUI/bloomUI.less Adds the .bloomDarkScrollbars class with a mixin that styles WebKit scrollbar pseudo-elements; the & * descendant rule gives full coverage to all scrolling children with a single root class.
DistFiles/localization/en/BloomMediumPriority.xlf Adds nine new localizable strings (GenerateReport, GenerateReport.ToolTip, StageNumber, OfCount, LevelNumber, Measure, OverLevel, OverLevelWarning, WithinLevel) all correctly marked sil:dynamic=true.
DistFiles/localization/en/Bloom.xlf Adds obsolete notes to MakeLetterWordReport, StageNofM, and LevelNofM strings; no IDs changed, existing translations preserved.

Reviews (7): Last reviewed commit: "Merge remote-tracking branch 'origin/mas..." | Re-trigger Greptile

hatton and others added 2 commits July 20, 2026 16:23
…son (BL-16585)

- The banner's "This book reads above Level {0}" was raw JSX, so {0}
  evaluated to the number 0, corrupting the registered/harvested English
  fallback. Pass it as a string literal like the other Spans.
- isBookOverLevel compared raw average values against the limits while the
  rows display/color those values rounded to one decimal, so a raw 5.04
  could trip the banner while every row showed green "5.0". Compare against
  the same one-decimal rounding (a no-op for integer stats).
- Remove a duplicated comment block in ReaderToolNav.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
isBookOverLevel is documented to mirror the per-row orange coloring, but it
omitted the "total sentences in the book" measure, so if that limit were
exceeded the row would color orange while the banner stayed hidden. Add the
pair. (Harmless today since maxSentencesPerBook() currently returns 0, so it
is guarded out; this keeps the mirror complete if that limit becomes active.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hatton

hatton commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-20 16:40 MDT up to commit 5012e531.

  • 2 Bugs — both already addressed in this PR and mirrored as resolved review threads: the sentences-per-book pair (added in 5012e531) and the banner/row rounding mismatch (fixed in 2c48bec). Devin's re-report of them reflects the pre-fix code; its own informational flag "Banner vs. row rounding is consistent" and Greptile's summary agree the current code is consistent.
  • 0 Investigate flags.
  • 7 Informational items (not mirrored — low signal), including notes confirming the obsolete strings were retained and the rounding is now consistent.

Greptile: clean (positive summary, no inline findings). CI: green.

hatton and others added 2 commits July 22, 2026 16:10
…(BL-16585)

Leveled/Decodable Reader tools: move the Set Up button below the "Book is
Leveled/Decodable" toggle, put Copy Book Stats / Generate Report in that stack,
give Set Up a contained (blue) style bearing the tool's accordion-header icon,
uppercase the toggle labels, and let the toggle scroll with the panel content.

Add a reusable `bloomDarkScrollbars` class, defined once in bloomUI.less, and
apply it to the dark panels (Collection tab, the Edit-tab toolbox and page-list
iframes, and the Publish tab's dark tab strip), replacing the ad-hoc toolbox
scrollbar styling. Light panels keep the browser default.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread DistFiles/localization/en/Bloom.xlf Outdated
@hatton

hatton commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-22 up to commit fa52d368c.

  • 2 Bugs — both the over-level-banner issues Devin flagged on earlier commits; Devin now marks both resolved, and their threads were already resolved. No action.
  • 1 Investigate flag — the new reader-tool XLF entries aren't translated consistently (Generate Report is translate="no" and stays English; sibling new strings are translatable). Posted as an open thread; left for a developer decision (see the preflight decision report).
  • 8 Informational — reviewed, nothing actionable.

Greptile: pass — its only substantive points were the same XLF translate="no" question (above) and that readerOutlineButtonCss is exported-but-unused (trivial dead code). CI green (pr-automation, track). No CodeRabbit review posted on this PR.

@hatton

hatton commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-23 up to commit e4426a14.

This commit just merges the latest master into the branch (only added base's jquery.text-markup files); no reader-tool code changed since the last consultation.

  • 2 Bugs — the two over-level-banner issues from earlier commits; Devin again marks both resolved, and their review threads are already resolved. No action.
  • 1 Investigate — the translate="no" inconsistency on the new reader-tool strings. Unchanged from before and already mirrored as an open review thread awaiting a developer decision (see the preflight decision report). Not re-posted.
  • 9 Informational — not mirrored (low signal); several were positive confirmations (banner logic consistent with per-row coloring, react-tabs className handled correctly, obsolete strings correctly retained). One noted readerOutlineButtonCss is an unused export (dead code, not a bug).

CI green (pr-automation, track). Greptile re-review in progress.

hatton and others added 2 commits July 23, 2026 14:19
…ss (BL-16585)

Per developer decision on the preflight review:

- The new "Generate Report" button and its tooltip were marked
  translate="no", which would have kept them English for every user —
  a regression from the translatable button they replace
  (MakeLetterWordReport). Switch both to sil:dynamic="true" to match the
  predecessor, the sibling reader-tool strings, and the runtime
  (BloomButton) mechanism that displays them, so the label is translated
  like the rest of the feature.
- Remove readerOutlineButtonCss from readerToolStyles.ts: it was exported
  but never imported anywhere (flagged by both the local review and Devin).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hatton

hatton commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-23 up to commit 3fcf586c.

This commit implements the developer's decision from the preflight report (make "Generate Report" translatable; remove an unused style export) and merges the latest master.

  • 2 Bugs — the two over-level-banner issues; Devin still marks both resolved, threads already resolved. No action.
  • 1 Investigate ("New XLF entries omit translate=\"no\"…") — this is the translation-consistency item that was just fixed in this very commit: GenerateReport/GenerateReport.ToolTip are now sil:dynamic="true" like their siblings. Devin re-emitted the flag but its analysis text is unchanged from the prior commit (it still describes GenerateReport as translate="no"), so it no longer matches the code — a stale re-emission. Outcome documented on the now-resolved thread (Improve Leveled Reader tool UI (BL-16585) #8087 (comment)). Not re-posted.
  • 9 Informational — not mirrored (low signal).

CI green (pr-automation, track). Greptile re-review: pass, no new findings.

@hatton hatton left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@hatton reviewed 13 files and all commit messages, and made 1 comment.
Reviewable status: 13 of 15 files reviewed, 4 unresolved discussions.


src/BloomBrowserUI/bookEdit/toolbox/readers/ReaderToolNav.tsx line 37 at r3 (raw file):

    }

    // Boxed square buttons at the right, matching the mockup. Unlike before,

Don't put comments in our code referring to the mockup, or what things looked like "before".

hatton and others added 3 commits July 23, 2026 15:14
The six new decodable/leveled reader strings this branch added
(GenerateReport, GenerateReport.ToolTip, StageNumber, LevelNumber,
Measure, and the shared ReaderTools.OfCount) were in Bloom.xlf; move
them to BloomMediumPriority.xlf, grouped with the existing
DecodableReaderTool and LeveledReaderTool entries. IDs, source text,
notes, and sil:dynamic="true" are unchanged, so every l10n lookup still
resolves — only the priority file changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review feedback: reword three comments so they describe what the
code does now, without pointing at the design mockup or how things
looked "before" (ReaderToolNav arrow buttons, the LeveledReader label
first-letter capitalization, and the shared reader accent color).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@hatton hatton left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@hatton+JHAI made 1 comment and resolved 1 discussion.
Reviewable status: 10 of 15 files reviewed, 3 unresolved discussions (waiting on hatton).


src/BloomBrowserUI/bookEdit/toolbox/readers/ReaderToolNav.tsx line 37 at r3 (raw file):

Previously, hatton (John Hatton) wrote…

Don't put comments in our code referring to the mockup, or what things looked like "before".

[Claude Opus 4.8] Done in c0052bd3. Reworded the three comments so they describe what the code does now, dropping the mockup reference and the "before" framing: the boxed arrow buttons here in ReaderToolNav, the first-letter capitalization in LeveledReaderToolControls, and the shared reader accent color in readerToolStyles.ts. I also swept the rest of the branch's changed files — the only other "no longer / used to" comments were pre-existing on master, so I left those alone.

@hatton hatton left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@hatton reviewed 5 files and all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on hatton).

@hatton
hatton marked this pull request as ready for review July 23, 2026 21:22

@StephenMcConnel StephenMcConnel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@StephenMcConnel reviewed 15 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on hatton).


src/BloomBrowserUI/bookEdit/toolbox/readers/ReaderToolNav.tsx line 95 at r4 (raw file):

                    `}
                >
                    {props.isForLeveled ? "Level {0}" : "Stage {0}"}

Are we sure that "Level X" always precedes "of Y" in every language? I agree that assumption makes nicer styling easier (or possible), but it is an assumption.


src/BloomBrowserUI/bookEdit/toolbox/readers/readerToolsModel.ts line 105 at r4 (raw file):

            "EditTab.Toolbox.DecodableReaderTool.StageNumber": "Stage {0}",
            "EditTab.Toolbox.LeveledReaderTool.LevelNumber": "Level {0}",
            "EditTab.Toolbox.ReaderTools.OfCount": "of {0}",

See my earlier comment about localization.

@StephenMcConnel StephenMcConnel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@StephenMcConnel resolved 3 discussions.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on hatton).

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.

2 participants