Repository navigation
Translate the mail dialog, the Playground web server and the copy-path error - #647
Open
ryanwelcher wants to merge 3 commits into
Open
ryanwelcher wants to merge 3 commits into
ryanwelcher wants to merge 3 commits into
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
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
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