Repository navigation
Translate the mail dialog, the Playground web server and the copy-path error - #647
Merged
Merged
Conversation
…h error Wraps the strings #630 lists that no other open batch covers, and drops the English sentence applyDoneMessage built from a verb and a noun for a pair it has no sentence for, in favour of a generic word. Adds two en-XA journeys: a mail on its Rendered and Raw tabs, and the Playground web server stopped, starting, running and after it exits. Part of #630.
…uncement and the generic confirmation The journeys now open a mail with no subject, and read the live region the Start button speaks into, which is outside the notices scanned. The fallback confirmation gets a context, since 'Done' is also a button label, and joins the test that runs every sentence through a catalog.
|
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 |
…rings # Conflicts: # tests/e2e/journeys/i18n.spec.js
…rings # Conflicts: # src/renderer/index.jsx # tests/e2e/journeys/i18n.spec.js
…rings # Conflicts: # tests/e2e/journeys/i18n.spec.js
…rings # Conflicts: # tests/e2e/journeys/i18n.spec.js
…rings # Conflicts: # tests/e2e/journeys/i18n.spec.js
ryanwelcher
added a commit
that referenced
this pull request
Oct 7, 2026
…eaks left in wrapped code (#653) ## Why #630 is the last batch of #622. #647 did the part that didn't depend on the other batches. This does the rest, now that #636, #643, #644, #645 and #646 are merged: the en-XA sweep #630 asks for, and the two rule breaks it named in already-wrapped code. ## What changes **Found by the sweep:** - xterm's own screen-reader strings: the terminal input's label ("Terminal input") and its "too much output" announcement, set through `Terminal.strings` before each terminal is created. - Main's refusal to submit while a patch is applied. It was `${label} is applied. Revert it…`, a sentence spliced around the name, and it's shown as-is in the patch pane. It's now two whole sentences, one with the name and one without. - The Playground web server's three errors from main (no folder to serve, exits before it's ready, times out). #647's review deferred these here. **Rule breaks #630 named:** - `apply-card.cjs` adds a full stop to a headline that lacks one. #630 said to drop it once main's errors carried their own. They don't, and can't all: `patch-apply.js` messages double as terminal bullets, the fallback headline is Git's own stderr line, and four paths pass a thrown error's text. So the full stop stays but goes through `_x('%s.', …)` for the translator to choose, and a sentence already ended in any script's mark (`\p{Sentence_Terminal}`) keeps it. - `workItem.label` is removed from both project types. Nothing read it once the sentences that named a work item became one per kind. **Already done, nothing to change:** `use-dev-server.jsx` had nothing left after #646. ## How to test this Any platform, current head. The suites cover it: ```bash # from the repository root npm test npm run build:once npx playwright test --project=journeys tests/e2e/journeys/i18n.spec.js ``` - `ipc-wiring.test.cjs` has two new pseudo-locale tests, for main's refusal (with and without a name) and the web server's failures. Both are red on the old `main.js`. - `apply-card.test.cjs` checks the translated full stop, and that sentences ended in `。`, `؟` and `।` keep their mark. - The i18n Terminal journey now finds the input by its translated name. The apply card journey now expects the whole refusal, full stop included, as one translated string. To look by hand: run `npx electron . --lang=en-XA` from the repository root, open a site, open the Terminal tray, and inspect the terminal's textarea. Its `aria-label` is bracketed and accented. **What must not have happened:** with no `--lang`, nothing reads differently. The refusal, the web server errors and the terminal's label are the same English as before, and `mail.spec.js`, `terminal.spec.js` and the rest of the English journeys pass unchanged. ## Risks and limitations - **What the sweep covered:** I launched en-XA with a Core site and a Gutenberg site, opened every tray, log tab and settings tab, the site menu and the create dialog, and scanned the whole window. After the xterm fix, the only English left was product names, commands, the `$` prompt and PHP version numbers. It didn't open every dialog or failure state; the existing i18n journeys cover many of those. - **Left on purpose** (#622's do-not-wrap list): main's argument guards (`'Site is not registered'`, `'No running script'`), the `github-prs.js` and `trac-view.js` errors that are replaced by a status before display, and `console.error` messages. - **Untested:** `'Timed out starting web server'` is wrapped but has no test. It needs fake timers for its 20-second wait. - #648 (the a11y "Notifications" text) is separate. ## Related Closes #630. Part of #622. --- <details> <summary>Review outcome</summary> **1 [fix here] · 1 [follow-up]. The fix is applied in c2eb4ba.** - **Review:** completed by a separate agent with a fresh context, against `.github/instructions/code-review.instructions.md`. Reviewed head c53312a, base 8303849 (trunk). `npm run lint` clean, `npm test` 2081/2081, full journey suite 147/147 on c53312a. - **Fixed:** the list of marks that end a sentence missed Devanagari `।`, Armenian, Ethiopic, Khmer and Myanmar, so a Hindi sentence would have got a second mark. It now uses `\p{Sentence_Terminal}`, with a Hindi case in the test. The doc comment's over-long line was rewrapped. - **Deferred:** a test for the web server's 20-second timeout (see Risks). - **Confirmed by the review:** - xterm reads `promptLabel` in `open()` and `tooMuchOutput` on each announcement, so setting both before `new Terminal` is early enough. - The refusal's English is unchanged for every label, including a blank one. - Nothing reads `workItem.label`. - No English journey asserts a changed string. - **Since review:** c53312a → c2eb4ba, which contains only the fix above. Lint, unit tests (2081) and the i18n journeys were rerun on c2eb4ba. One i18n journey (site details and menu, which this branch doesn't touch) failed once in that run, then passed three times on its own and in the full run before. That's a flake. </details> Nothing on screen changes in English, so there are no screenshots. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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
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
#630 is the last batch of #622. This PR is the part of it that doesn't wait on the five open batches (#636, #643, #644, #645, #646): the mail dialog, the Playground web server and the copy-path error were still plain English, and
applyDoneMessagespliced a verb and a noun into an English sentence for any pair it had no sentence for.What changes
applyDoneMessage's fallback is_x('Done', 'an action that finished')instead of`${verb} the ${noun}`. No caller reaches it today: the three inuse-apply-patch.jsxpass only the five pairs it has sentences for.Not in this PR, because another open PR already wraps them or because #630 says to wait for one:
COPY_BUTTON_LABELSuse-dev-server.jsxleftoversapply-card.cjs:191periodproject-type.cjsworkItem.label--lang=en-XAwalkHow to test this
Two en-XA journeys cover this, and both are red without the change:
The English journeys in
mail.spec.jsstill pass, so an English speaker sees the same text as before.What the journeys can't reach is the copy-path alert. It needs the clipboard to refuse a write, and I couldn't stage that.
To look by hand (any platform, current head): run
npx electron . --lang=en-XAfrom the repository root, start a site's dev server, and open a mail from the Email tray. Every label and both tabs show accented and in brackets. The Playground web server only appears in a build that shipslocal-playground-web, so a checkout won't show it.What must not have happened: with no
--lang, the mail dialog and the web server read exactly as they did before.Risks and limitations
@wordpress/a11yadds a screen-reader-only "Notifications" paragraph at DOM-ready, before the app loads its locale, so it stays English. Translate the remaining strings, and fix the rule breaks in wrapped code #630 doesn't list it, so it isn't fixed here.'local-playground-web directory not found.', or a thrown error) and stay English. That belongs with the main-process strings, not this renderer batch.Related
Part of #630. Part of #622.
Review outcome
4 [fix here] · 2 [follow-up]. All 4 fixed in c9c5c0c. One more finding was dropped as wrong (below).
Review: completed by a separate agent with a fresh context, against
.github/instructions/code-review.instructions.md. Reviewed head 4f8b808, base e94f507 (trunk).npm run lintclean,npm test2033/2033.Fixed:
speak()writes outside the area scanned. It now asserts#a11y-speak-polite.__('Email')only shows for a mail with no subject. The journey now opens one.Donefallback wasn't in the unit test that runs every sentence through a catalog. It is now.__('Done')would share a msgid with theDonebutton in@wordpress/components. It now has a context.Each new journey assertion was checked to fail with its string unwrapped.
Deferred:
Dropped: the finding that "Dismiss", "Choose application…",
COPY_BUTTON_LABELSand "Setting up new site…" were unwrapped. The reviewer read the issues, not the PR diffs; Translate ticket and branch notices and input errors #644, Translate the Terminal and Logs output #646 and Translate the setup checklist and the trunk update banners #636 wrap them.Since review: 4f8b808 → c9c5c0c, which contains only the fixes above. Lint, unit tests and the i18n and mail journeys were rerun on c9c5c0c.
Nothing on screen changes in English, so there are no screenshots.
🤖 Generated with Claude Code