Send the Comic Book readmes' missing sections to Crowdin (BL-16750) - #8233
Send the Comic Book readmes' missing sections to Crowdin (BL-16750)#8233andrew-polk wants to merge 2 commits into
Conversation
Three whole sections of the Paper Comic Book and Digital Comic Book
template readmes had no {i18n="..."} annotations, so 26 strings had never
been sent to Crowdin and showed in English whatever UI language the user
chose:
## Limitations of Bloom's current comic book support
## Tips on using the Canvas Toolbox
## A note on White on Black Text
...along with all of their bullets and paragraphs. The sections either
side of them are annotated, so these were almost certainly added later
and the annotations forgotten.
The text is identical in both readmes, so it gets shared comic.template.*
ids rather than per-readme ones, per the note at the top of each file:
Crowdin translates each string once and both readmes pick it up. Also
annotated the four "n/a" cells in the Leveled Reader table, which sit in
raw HTML whose header cells already carried inline i18n attributes; all
four hold the same text, so they share one id.
Regenerating the English XLIFFs adds 27 trans-units. Nothing was lost --
`--extract --preserve` keeps a unit that is no longer in the HTML, so the
"Obsolete as of 6.3" full-bleed unit survives, moved to the end of the
file.
Nothing was catching this. The extractor only picks up blocks carrying an
i18n attribute; text without one is skipped at every stage, and since the
extractor writes the same XLIFF either way, regenerating it produces no
diff. So this also brings in check-readme-i18n.js, which found the bug,
and wires it into build-prod with --strict so it cannot come back.
Verified both ways: it fails (exit 1) on a heading with the annotation
removed, and passes on all ten readmes as they now stand.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sharing an id between blocks that say the same thing is deliberate and useful -- this PR does exactly that for the Leveled Reader's four "n/a" cells, so the string is translated once. But the XLIFF holds one source per id, so if the texts ever differ, one block silently displays the other's translation. That is now the check's third rule, guarding the technique the rest of this PR relies on. Verified: changing one of those four cells to "not applicable" makes the check fail with both texts named; restoring it passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| "build:clean": "node scripts/clean.js", | ||
| "// 'build-prod': 'production build: clean, pageSizes, then build:ui and build:content, then l10n'": " ", | ||
| "build-prod": "pnpm run build:clean && pnpm --dir ../content run build:pageSizes && npm-run-all build:ui build:content && npm-run-all --parallel build:l10n:translate build:l10n:create", | ||
| "build-prod": "pnpm run build:clean && pnpm --dir ../content run build:pageSizes && npm-run-all build:ui build:content && npm-run-all --parallel build:l10n:translate build:l10n:create && pnpm run check:readme-i18n:strict", |
There was a problem hiding this comment.
[Devin] Investigate: Strict readme check gates the whole production build
build-prod now ends with check:readme-i18n:strict, which exits non-zero on any finding. It scans every generated readme under output/browser/templates, so untranslated text in any template readme (not just the ones changed here) will now fail the full production build.
[Claude Opus 5 (1M context) during preflight] Correct reading, and it is the deliberate intent — but it is a judgment call about everyone's build, so it is going to the developer rather than being decided here.
For context on the risk: all ten readmes pass as of this PR, so the gate can only fire on a newly introduced problem, and the only way to introduce one is to add readme text without an {i18n="..."} annotation. The check is also the only enforcement point available — GitHub Actions doesn't build the front-end for PRs, so build-prod (TeamCity) is where it has to live if it is to be enforced at all.
The alternative is to leave it warn-only, which is what it was until this commit: it then reports and nobody notices, which is exactly how these 26 strings went missing for several releases.
|
[Claude Opus 5 (1M context) during preflight] Consulted Devin on 2026-08-25 18:05 UTC up to commit It raised no bugs, one Investigate flag, and one informational item.
CI: |
|
| Filename | Overview |
|---|---|
| src/BloomBrowserUI/scripts/check-readme-i18n.js | Adds the readme-localization validator, but its text-node ancestor traversal assigns null to every i18n id and makes strict validation fail on valid content. |
| src/BloomBrowserUI/package.json | Adds checker scripts and makes the broken strict validator a required final step of build-prod. |
| src/content/templates/template books/Digital Comic Book/ReadMe-en.md | Adds localization annotations to the previously uncovered comic-book sections using shared identifiers. |
| src/content/templates/template books/Paper Comic Book/ReadMe-en.md | Adds matching localization annotations to the duplicated Paper Comic Book guidance. |
| src/content/templates/template books/Leveled Reader/ReadMe-en.md | Assigns one shared localization identifier to the four identical “n/a” table cells. |
| DistFiles/localization/Digital Comic Book/ReadMe-en.xlf | Adds the extracted English translation units corresponding to the newly annotated Digital Comic Book sections. |
| DistFiles/localization/Paper Comic Book/ReadMe-en.xlf | Adds matching comic translation units and preserves the explicitly obsolete full-bleed unit. |
| DistFiles/localization/Leveled Reader/ReadMe-en.xlf | Adds the English translation unit for the shared not-applicable table value. |
Reviews (1): Last reviewed commit: "Also catch one i18n id shared by blocks ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
1 participant
Problem
Three whole sections of the Paper Comic Book and Digital Comic Book template readmes — "Limitations of Bloom's current comic book support", "Tips on using the Canvas Toolbox", and "A note on White on Black Text", together with all their bullets — had never been sent to Crowdin. A user reading either readme saw those sections in English no matter which UI language they had chosen, and no translator had ever been offered the text. 26 strings in all.
Cause
Template readmes are localized as whole files:
HtmlXliff.exe --extractturnsReadMe-en.htmintoReadMe-en.xlf, that goes to Crowdin, and the returned translations are injected to makeReadMe-<lang>.htm. The extractor only picks up blocks carrying ani18nattribute, which comes from an{i18n="some.id"}annotation on the block in the markdown. Those sections had no annotations, so they were skipped at every stage — and because the extractor writes the same XLIFF either way, regenerating it produced no diff. Nothing anywhere reported it.Fix
comic.template.*ids rather than per-readme ones, per the note at the top of each: Crowdin translates each string once and both readmes pick it up.n/acells in the Leveled Reader table, whose header cells already carried inlinei18nattributes. All four hold the same text, so they share one id.--extract --preservekeeps a unit that is no longer in the HTML, so theObsolete as of 6.3full-bleed unit survives, moved to the end of its file.scripts/check-readme-i18n.js(pnpm check:readme-i18n), which found this, and wired it intobuild-prodwith--strictso it cannot come back. Four checks: text noi18nattribute covers; an id with text but no trans-unit; one id shared by blocks whose text differs; and a live trans-unit with nothing left in the HTML — tolerating units deliberately marked obsolete.Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16750
Devin review
This change is