Repository navigation
Translate the setup checklist and the trunk update banners - #636
Open
ryanwelcher wants to merge 3 commits into
Open
ryanwelcher wants to merge 3 commits into
ryanwelcher wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Oct 6, 2026
The checklist, the trunk update card, the question an update asks over edits, and the banners for an old trunk, an incomplete update and a finished one are wrapped, with the sentences they take from setup-steps, update-plan, next-action and project-type's setup copy. Module-level strings become functions or getters so they are translated when shown, and sentences built from parts become whole ones: the step counters, the file count, the update summary and the link-ticket hint, which now takes the work item's kind instead of splicing in its label. Fixes #626.
…te steps are translated when asked for The notes for the dirty-tree answers' details reached their labels too, and the notes for the dirty-tree errors reached the "Unknown error" fallback on the same line. A unit test now loads a catalog after update-plan.cjs is required, so a message turned back into a constant fails it.
ryanwelcher
force-pushed
the
fix/626-setup-and-trunk-strings
branch
from
October 6, 2026 19:24
4b925eb to
7e0e1ad
Compare
ryanwelcher
added a commit
that referenced
this pull request
Oct 6, 2026
## Why The Open a pull request card, its GitHub sign-in, and the errors main sends it were plain English in a translated window. This is the #625 batch of #622, which needed the catalog in main from #634. ## What changes - **The card** (`pull-request-destination.jsx`) wraps every string. Sentences with a link or code in them use `createInterpolateElement`, with logins, branches and repository names passed in as elements so they are never read as markup. The failure messages are a function instead of a module-level constant. - **Per-project text** in `project-type.cjs` (`cards.*`) is getters, read when the card renders. Main's refusal with nothing linked used to splice the work item's label into a sentence; it is now `cards.prNeedsWorkItem`, one sentence per project type. - **Main process:** the sign-in errors in `github-auth.cjs` and the pull request errors in `github-pr.cjs` are wrapped, including the step labels passed to `failure()`, which the issue did not list. The "Trunk has moved" message uses `_n`, and a GitHub failure is composed with `%1$s: %2$s%3$s` so a translator can change the punctuation. GitHub's own reason and the bracketed diagnostic for GitHub support stay as they are. Not wrapped, per #622: the pull request body and the default pull request title, because they leave the app. ## How to test this The new journeys cover the card in the pseudo-locale on both projects. Run them from the repository root on either platform: ``` npx playwright test --project=journeys tests/e2e/journeys/i18n.spec.js -g "pull request card" ``` Both fail without this change. The Core journey walks every state #625 lists: signed out, "Not now" and "Show this again", the device code, signed in with the form and its fold, a `rate-limited` failure, a dry run, and a pull request opened from a checkout behind trunk. The Gutenberg one walks signed out, signed in with the fold open, and the opened pull request, checking the words that are Gutenberg's own. What the journeys cannot reach is a real GitHub. They stub every `github:*` handler, so the main-process errors never reach the screen in them. Unit tests in `github-auth`, `github-pr` and `pr-stage` cover those strings in the pseudo-locale instead. To see one on screen by hand (either platform, current head): 1. From the repository root, run `npx electron . --lang=en-XA`. 2. Open a Core site with a linked ticket and an edit, and click **Review & submit changes**. 3. In **Open a pull request**, click **Sign in with GitHub** and finish in the browser. - Expected: every line of the card is accented and in brackets, apart from your login, your fork's name, the code and the branch. **What must not have happened:** no English an English speaker sees has changed. The English journeys in `open-pull-request.spec.js`, `review-changes.spec.js` and `pr-checkout.spec.js` still pass with their exact strings. ## Risks and limitations - The other open batches (#636, #644, #645, #646) also append tests to the end of `i18n.spec.js`, so whichever merges later resolves that by keeping both. - One finding is deferred: `prFailureMessage` decides between five user-facing strings inside a `.jsx` file the suite cannot load. It predates this PR (it was the old constant map), so moving it to a `.cjs` module with a test is a follow-up. ## Related Fixes #625. Part of #622. Follows #634. --- <details> <summary>Design decisions and alternatives considered</summary> - **Not stacked on #635.** #622 asks for batches that review on their own. Now that #635 has merged, its `unwrapped()` scanner change and the `@wordpress/element` dependency, which this PR also needed, come from trunk. - **The textarea placeholder** is three strings joined with a newline. A single `__()` with `\n` in it is refused by `@wordpress/i18n-no-collapsible-whitespace`. - **"Signed in as …"** is wrapped in its own `<span>`, so the sentence and the **Sign out** button beside it are separate elements, as in #635. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 2 [fix here] · 1 [follow-up]. Both [fix here] findings are fixed; the follow-up is deferred (see Risks). - 🟡 The Gutenberg journey linked no issue, so it never showed the form, the fold or the loop-back. Fixed: it links one in the record and walks those states. - 🔵 No test showed the main-process strings or the stage labels come out translated. Fixed: pseudo-locale unit tests for the scope refusal, a composed `failure()`, the plural stale message and `prStageLabel`. - Style: three dead entries in the journey's `THEIRS` filter were removed. - **Review:** completed. Fresh agent context, head `11ef362` / base `c34aa45`. `npm run lint` clean, `npm test` 2002 pass / 0 fail, i18n and PR journeys 25 pass. - **Since review:** `11ef362 → ef457fd`. The only change is the test fixes above, checked by the authoring session, not re-reviewed by a fresh context. `src` and `tests` lint clean, unit suite 2005 pass / 0 fail, i18n journeys 12 pass. The review did not run the journeys itself; it was read-only. - **After #635 merged:** `ef457fd → 9559a1f` (trunk merged from GitHub) → `012fca8` (trunk at `e94f507` merged; `i18n.spec.js` resolved as trunk's file plus this PR's two tests, nothing else changed). Checked by the authoring session, not re-reviewed: `src` and `tests` lint clean, unit suite 2036 pass / 0 fail, i18n, open-pull-request, review-changes and pr-checkout journeys 31 pass. </details> <details> <summary>Screenshots or recording</summary> None. Nothing changes on screen in English, and the pseudo-locale journeys are the check for the translated state. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…unk-strings # Conflicts: # tests/e2e/journeys/i18n.spec.js
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The setup checklist and the trunk update's cards and banners were still plain English in a translated app. This is the #626 batch of #622.
What changes
setup-steps.cjs,update-plan.cjs,next-action.cjsandproject-type.cjs'ssetupcopy. Also the dirty-tree dialog's errors inuse-trunk-update.jsxand the alert inuse-dev-server.jsx.update-plan.cjs's messages and step labels were module-level constants and are now functions (SKIP_INSTALL_MESSAGEbecameskipInstallMessage(), and so on);project-type.cjs'ssetup.*are getters, likedescriptionalready was.sprintf('step %1$d of %2$d')), the file count and the day count (_n), "Next step: %s", and the update summary, which was three fragments and is now one of four sentences (updateSummarySentence). The link-ticket hint took the work item's label as a noun ("Link a %s…") and now takes its kind and has one sentence each, as Translate the remaining strings, and fix the rule breaks in wrapped code #630 asks for.npm install,npm run build,package.json,src/) are passed in as%s.How to test this
Covered by journeys in
tests/e2e/journeys/i18n.spec.js, all red without this change. From the repository root:npx playwright test --project=journeys tests/e2e/journeys/i18n.spec.js -g "checklist|trunk banners|did not finish". In the en-XA pseudo-locale they reach the checklist with steps done and ready, the checklist after a failed install, the old-trunk banner, the question an update asks over an edit (each answer chosen), the update card on its first step, and the banner after an update that did not finish, and fail on any visible English.To look at it by hand, on either platform, on the current head: run
npx electron . --lang=en-XAfrom the repository root and open a site whose trunk is more than 14 days old.What must not have happened: in English, the checklist, the cards and the banners read exactly as before.
Not reached by a journey, and why:
updateSummarySentence, which a unit test covers.Risks and limitations
tray.spec.js's "the tray's edge is moved with the arrow keys…" failed once in the full run. It fails about one run in five ontrunktoo, so it is not this branch.tests/e2e/journeys/i18n.spec.js, so whichever merges later resolves that by keeping both.Related
Fixes #626. Part of #622.
Design decisions and alternatives considered
The dirty-tree answers' details carry their own leading dash ("— a .diff on your machine…"), with the space outside the string: punctuation is the translator's, and a string that starts with a space is easy to break. The link-ticket hint takes
workItemNouninstead ofworkItemLabel, which removes one of the places #630 lists as splicing the label into a sentence.Review outcome (required, see AGENTS.md)
.github/instructions/code-review.instructions.md; head1790e42/ base8dbed36.npm run lintclean,npm test1954 pass / 0 fail, journeys 117 pass. Outcome: 2 [fix here] · 0 [follow-up], both 🔵.4b925eb: each note sits directly above its own string, and the fallback is on its own line.update-plan.cjsmessage functions went back to English constants. Fixed in4b925eb: a unit test loads a catalog after the module is required. Checked by turning one back into a constant, which fails it.work-item.cjs, and the translator example "2m 5s", which the app writes as "2m 05s".1790e42 → 4b925ebchecked; the change is the fixes above. Lint clean,npm test1955 pass, journeys 116 pass and 1 known flaky (above) on the new head.4b925eb → 7e0e1ad, rebased onto trunke94f507. Onlytests/e2e/journeys/i18n.spec.jsconflicted: resolved as trunk's file plus this PR's four tests, dropping this PR's copies of the imports andMY_EDIT, which trunk now has. Translate the Review & submit dialog #635 brought the identicaltrunkDatechange togit-site.cjs, so it is no longer in this diff. Checked by the authoring session, not re-reviewed: build and lint clean,npm test2035 pass / 0 fail / 2 skipped, the full journey suite 134 pass.Implementation notes
update-plan.cjsexports have no consumer left under their old names insrc,testsordocs.main.jsreadsproject-type.cjsbut neversetup, so the getters change nothing there.Screenshots or recording
Nothing changes in English. In the pseudo-locale every screen in this batch is bracketed, which the journeys assert.
🤖 Generated with Claude Code