Skip to content

Translate the mail dialog, the Playground web server and the copy-path error - #647

Merged
ryanwelcher merged 8 commits into
trunkfrom
fix/630-remaining-strings
Oct 7, 2026
Merged

ryanwelcher merged 8 commits into
trunkfrom
fix/630-remaining-strings

Conversation

@ryanwelcher

Copy link
Copy Markdown
Collaborator

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 applyDoneMessage spliced a verb and a noun into an English sentence for any pair it had no sentence for.

What changes

  • The mail dialog: "From:", "To:", "CC:", "Date:", the Rendered and Raw tabs, and the "Email" title of a mail with no subject.
  • The Playground web server: its Start and Stop buttons, the loading announcement, "Starting…", the panel's heading, "Stopped", and "Server exited with code %d".
  • The copy-path failure: the alert and the clipboard-unavailable message in it.
  • applyDoneMessage's fallback is _x('Done', 'an action that finished') instead of `${verb} the ${noun}`. No caller reaches it today: the three in use-apply-patch.jsx pass 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:

Item Where
COPY_BUTTON_LABELS #646
"Dismiss" on the deep-link notice, "Choose application…" #644
"Setting up new site…" #636
use-dev-server.jsx leftovers after #646
apply-card.cjs:191 period after #645
project-type.cjs workItem.label after #643, #636, #644
The full --lang=en-XA walk after all five merge

How to test this

Two en-XA journeys cover this, and both are red without the change:

# from the repository root
npm run build:once
npx playwright test --project=journeys tests/e2e/journeys/i18n.spec.js -g "a mail the site sent|Playground web server"
  • a mail the site sent is fully translatable: a mail on its Rendered tab and its Raw tab, and a mail with no subject, whose dialog is titled "Email".
  • the Playground web server is fully translatable: stopped, starting (including the live-region announcement), running, and after it exits with code 1.

The English journeys in mail.spec.js still 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-XA from 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 ships local-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

  • Found while writing the web server journey: @wordpress/a11y adds 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.
  • The web server's other errors come from main ('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 lint clean, npm test 2033/2033.

  • Fixed:

    • The web server journey could pass with the loading announcement unwrapped, because 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.
    • The Done fallback wasn't in the unit test that runs every sentence through a catalog. It is now.
    • A bare __('Done') would share a msgid with the Done button in @wordpress/components. It now has a context.

    Each new journey assertion was checked to fail with its string unwrapped.

  • Deferred:

    • The mail journey excuses any text that is part of the mail. No label is excused today, but matching whole text nodes would be stricter.
    • The web server's main-process error strings (see Risks).
  • Dropped: the finding that "Dismiss", "Choose application…", COPY_BUTTON_LABELS and "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

…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.
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository: WordPress/contributor-toolkit/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f2259cf2-54a1-4b8e-bb33-2e250fcdf1bf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

ryanwelcher and others added 6 commits October 6, 2026 16:18
…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
ryanwelcher merged commit 8303849 into trunk Oct 7, 2026
8 checks passed
@ryanwelcher
ryanwelcher deleted the fix/630-remaining-strings branch October 7, 2026 15:02
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)
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