Repository navigation
Collection Settings in React: Front & Back Matter page (BL-16733) - #8493
Conversation
e50f325 to
0ab05ab
Compare
0ab05ab to
ef834eb
Compare
ef834eb to
5b07891
Compare
andrew-polk
left a comment
There was a problem hiding this comment.
@andrew-polk reviewed 16 files and all commit messages, and resolved 3 discussions.
Reviewable status: 0 of 16 files reviewed, all discussions resolved.
f39bb2c to
1a18bc6
Compare
|
[Claude Opus 5.5] Consulted Devin on 2026-10-26 19:20 UTC up to commit 76d166e. Its findings are on the PR as resolved threads, each with its outcome. |
76d166e to
c0bf0b8
Compare
|
[Medium impact] The PR appears safe to merge; no actionable defects were found.
|
c0bf0b8 to
02027ff
Compare
andrew-polk
left a comment
There was a problem hiding this comment.
@andrew-polk reviewed 21 files and all commit messages, and resolved 3 discussions.
Reviewable status: 0 of 24 files reviewed, all discussions resolved.
StephenMcConnel
left a comment
There was a problem hiding this comment.
@StephenMcConnel reviewed 24 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on andrew-polk).
src/BloomBrowserUI/collection/settingsPages/FrontBackMatterPage.tsx line 149 at r8 (raw file):
<ConfigrInput label={captionLabel} path="frontBackMatter.qrcodeCaption"
Earlier and just below a few lines, frontBackMatter is treated as though it could be null. Shouldn't this be consistent throughout?
src/BloomBrowserUI/collection/settingsPages/FrontBackMatterPage.tsx line 171 at r8 (raw file):
<ConfigrInput label={countryLabel} path="frontBackMatter.country"
Could frontBackMatter be null here (and in the next two inputs)?
Style lists the front/back matter packs with the chosen one's description; the dialog's GET collection/settings carries the packs and whether the branding chose one. QR Codes has Show QR Codes, Caption and a live preview of the badge, built by the code that builds books' badges (GET collection/qrBadge, with its QR image from GET collection/qrCode) and styled by bloom-badge.less, which books use too. Places has Country, Province and District, under the heading and explanation Config-R draws from the group's label and description (Config-R 1.0.0-alpha.33, which also brings two BL-16739 sub-page fixes). Page Numbering Style goes with the Languages page. Opening a collection now also replaces a saved project-specific pack that no branding chooses (as when the subscription whose branding chose it has expired), as it already did a pack that is not installed, so books and the settings dialogs agree on the pack. It keeps an expired subscription's bookshelf in the file (BL-15056), and leaves alone a copy of the pack the user installed themselves. It is skipped on the command line, and in a Team Collection only an administrator makes the change, since it reaches the whole team. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
e024c72 to
f30c392
Compare
andrew-polk
left a comment
There was a problem hiding this comment.
@andrew-polk+AI made 2 comments.
Reviewable status: 23 of 24 files reviewed, 2 unresolved discussions (waiting on StephenMcConnel).
src/BloomBrowserUI/collection/settingsPages/FrontBackMatterPage.tsx line 149 at r8 (raw file):
Previously, StephenMcConnel (Steve McConnel) wrote…
Earlier and just below a few lines, frontBackMatter is treated as though it could be null. Shouldn't this be consistent throughout?
[Claude Opus 5.5 from Andrew Polk's machine during reviewable-replies] Good question; two different things are both called frontBackMatter here, and the code didn't say so.
path="frontBackMatter.qrcodeCaption" is only a string: it tells Config-R where in its own values to read and write the field. Those are loadedSettings.values, and the dialog mounts the pane only once they are loaded (CollectionSettingsDialog.tsx, {loadedSettings && !notAllowedMessage && <ConfigrPane ...>}), so the path always resolves.
The ?. uses are on the hook's local frontBackMatter, from props.values. The page is a hook, which runs on every render of the dialog, including before the GET answers and after it closes, when props.values is undefined. While the pane is showing, it is always defined.
So nothing changes in behavior, but I added a comment above const frontBackMatter saying this (now in f30c392), so the next reader doesn't have to ask.
src/BloomBrowserUI/collection/settingsPages/FrontBackMatterPage.tsx line 171 at r8 (raw file):
Previously, StephenMcConnel (Steve McConnel) wrote…
Could frontBackMatter be null here (and in the next two inputs)?
[Claude Opus 5.5 from Andrew Polk's machine during reviewable-replies] No: like the caption, these paths are strings Config-R resolves against its own values, which always contain frontBackMatter once the pane is showing. See the other thread for the details, and the new comment above const frontBackMatter (f30c392).
StephenMcConnel
left a comment
There was a problem hiding this comment.
@StephenMcConnel reviewed 1 file and all commit messages, and resolved 2 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on andrew-polk).
Problem
The new (React) Collection Settings dialog shows only a placeholder on its Front & Back Matter page. Choosing a front/back matter style, setting up the QR code on the "Made with Bloom" badge, and entering the place names that front matter shows can all still be done only in the old WinForms dialog.
Fix
The page now has real controls, laid out as in the card's mockup. Everything it shows comes from the dialog's own endpoints and values; it uses nothing from the old dialog.
GET collection/settingscarries the packs and says whether the branding chose one (the list is then disabled). Opening a collection now replaces a saved project-specific pack that no branding chooses, as happens when the subscription that chose it expires, so books and both dialogs agree on the pack. In a Team Collection only an administrator makes that change, because it reaches the whole team.GET collection/qrBadge, with its QR image fromGET collection/qrCode), and the dialog styles it withbloom-badge.less, the badge CSS books use, moved out ofbranding-base.less.The values were already in the save model. The caption box keeps the old dialog's behaviour of opening with the default wording filled in, and
FrontBackMatterValues.QrcodeCaptionsays why C# stores the caption only if it changed. Page Numbering Style goes with the Languages page (BL-16739).Screenshots
The page, with the chosen pack's description and the live badge preview:
Places, below it:
Risk Evaluation
Low to medium. Four shared paths changed:
CollectionSettings.CheckAndFixDependencies, which runs on every collection open. Next to its existing replacement of an uninstalled pack, it now replaces a saved project-specific pack that no branding chooses, and saves. That changes books' front matter in a collection whose project subscription has lapsed. It is limited to Bloom's own project-specific packs that Settings doesn't offer, so custom-installed packs (including a project's own copy of its pack) are untouched, and an expired subscription's bookshelf is kept in the file. It is skipped on the command line, so the harvester keeps a book's pack, and for Team Collection members who are not administrators. (The older uninstalled-pack replacement does not have that check; BL-17046 tracks it.)BookStorage.UpdateQrCode, which every book's badge goes through. The per-badge code moved intoUpdateBadgeso the preview can use it too, and the QR bitmap intoMakeLanguageQrCodeBitmap. What a book gets is unchanged: the same links, the samelang-qr-code.png, the same keep-if-present rule.branding-base.lessintobloom-badge.less, whichbranding-base.lessimports at the same spot; the compiled CSS is identical. This is the first time the React UI bundles a stylesheet fromsrc/content; the rules apply only inside.bloom-branding-wrapper.settings/xmatterreply is now built from the same pack helper as the new GET, and keeps its old shape.Ecosystem Impact
translate="no"while the dialog is alpha-only. The other labels reuse existing ids.@sillsdev/config-r1.0.0-alpha.30 → alpha.33. That brings a group's description (Bl 253 localize tool tips #32), two sub-page fixes (BL-16739: staying on a sub-page when the client re-renders, and leaving the first page's sub-page when a search starts), and Config-R's own dev-tooling upgrades.Automated Tests
CollectionSettingsDialog.test.tsx): the Front & Back Matter page offers the packs and describes the chosen one, is disabled when the branding chooses the pack, asks C# for the badge with the QR setting, caption and badge language as they will be saved (including the sign language, and an untouched spaces-only caption), and shows the HTML C# returns.BookStorageTests): the preview badge is the one books get, with the QR code, bold language name and address, or the plain badge with QR codes off, and has no link.CollectionSettingsTests): a project-specific pack no branding chooses is replaced and saved; it is kept for a Team Collection member who may not change settings, and when the user installed their own copy; an offered pack is kept; an expired subscription's bookshelf stays in the file.collection-settings-front-back-matter.spec.ts, described below.Test of the change: the
CollectionSettingsDialog.test.tsx,BookStorageTestsandCollectionSettingsTeststests above, andcollection-settings-front-back-matter.spec.ts.E2E Coverage
New:
collection-settings-front-back-matter.spec.ts..bloomCollection, and that the reopened dialog shows them.e2e/setBranding): the list shows only that pack, and it is disabled.Not covered end to end: turning QR codes off, the sign-language badge, and replacing a project-specific pack on open, which the unit tests cover.
Live checks (in a running Bloom):
f30c392).f30c392).bookshelf:in the saved file (f30c392).%LOCALAPPDATA%\SIL\Bloom\XMatter) keeps it, and the dialog's GET reports it as the pack (f30c392).f30c392).f30c392).Notion Test Suite
None.
Preflight report: https://bloombooks.github.io/dev-process-artifacts/deciders/bloomdesktop-bl-16733-xmatter-settings.html
Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16733
🤖 Generated with Claude Code
This change is
Devin review
Devin review