You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit e94f507
Browse filesBrowse the repository at this point in the historyBrowse files
## Why
The Review & submit dialog was still plain English in a translated app.
This is the #624 batch of #622.
## What changes
- **The diff pane stops reading English to decide what it holds.** Main
returned the literal `'No changes.'` and failures were prefixed
`'Error'`, and the dialog compared against both. Main now returns an
empty patch, a failed load sets `patchLoadFailed` with the raw error,
and the pane says "No changes." or "Error: %s" in the contributor's
language.
- **Wrapped:** the dialog, its diff pane, the Trac and mentor
destinations, and the sentences they take from `changes-note.cjs`
(`patchReviewContext`, `discardDisabledReason`), `pr-checkout.cjs`
(`prSubmissionRefusal`, `prSubmissionBlocked`), `wporg-handle.cjs` and
`patch-provenance.cjs`.
- **Rewritten, not just wrapped:** "N days old" uses `_n`; "Save, then
open #N" and "Save patch as %s" use `sprintf`; the heading is one
sentence per kind (ticket, issue); `<strong>Save</strong>`, the event
name and the applied patch's name go through `createInterpolateElement`.
User text (an event name, a patch's file name) goes in as an element,
never into the string, so a `<` in it stays text.
- **An English fix on the way:** with no name on record, the refusal
used to read "Revert The patch you applied before…". Each sentence now
has a version without a name.
- `@wordpress/element` is now a direct dependency (a one-line lockfile
change; it was already installed through `@wordpress/components`).
- Deliberately not here: the pull request card (#625), the Copy button's
labels (#630), and the changes note and pull request checkout sentences
(#628, #629).
## 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 "Review &
submit|nothing to send|mentor card"`. They open the dialog in the en-XA
pseudo-locale with an old trunk, with no ticket and with one, with
nothing to send, with a remembered username and event, over someone
else's pull request, and over an applied patch with and without a name,
and fail on any visible English.
To look at it by hand, on either platform, on the current head: run `npx
electron . --lang=en-XA` from the repository root, open a site with an
edited file and click the pseudo-localised **Review & submit changes**.
**Expected:** every string in the dialog is accented and in brackets
except the pull request card, the Copy button, file paths and the diff
itself.
**What must not have happened:** in English, the dialog reads exactly as
before, apart from "Generating patch..." becoming "Generating patch…". A
tree with nothing to send still shows "No changes." and offers no
destinations, and a diff that fails to load still says so instead of
claiming there are no changes (`pr-checkout.spec.js` covers that one).
Not covered: the invalid-username error. It is built in main, which has
no catalog until #634 lands, so its journey step is left for after that.
## Risks and limitations
- Review: 1 [fix here] · 3 [follow-up], all low. The fix is in; the
three follow-ups are below.
- Saving or copying with nothing to send now writes a 0-byte file and
empties the clipboard; before, both carried the text "No changes.". The
buttons were already enabled in that state. Follow-up: disable them when
there is no diff.
## Related
Fixes#624. Part of #622. Pairs with #634 for the two messages that come
from main.
---
<details>
<summary>Design decisions and alternatives considered</summary>
The journey's scanner (`unwrapped`) now treats a text node as translated
when the element around it, past bold, italic or code, holds one
bracketed string from first character to last. A sentence with a link or
a bold word in it is several text nodes, and only the whole is
bracketed. "[Foo] English [Bar]" still fails, so English spliced between
two wrapped strings is still caught. Sentences that sit side by side in
one notice each got their own `<span>` so the rule can see where one
ends.
`'(<discard />)'` is a translatable string of punctuation: some
languages write brackets differently.
</details>
<details>
<summary>Review outcome (required, see AGENTS.md)</summary>
- **Review:** completed. Reviewer: a separate agent context (Claude
Code, read-only) against
`.github/instructions/code-review.instructions.md`; head `90ae783` /
base `8dbed36`. `npm run lint` clean, `npm test` 1954 pass / 0 fail,
journeys 115 pass. Outcome: 1 [fix here] · 3 [follow-up], all 🔵.
- Tests [fix here]: the journeys missed the remembered mentor card, the
empty diff and the applied-patch warning. **Fixed** in `529b7ae`;
reaching them also showed the applied patch's headline shared an element
with the next sentence, now its own `<span>`.
- Architecture [follow-up]: `loadError` in `patch-diff-pane.jsx` and the
applied-patch headline in `review-dialog.jsx` pick a string inside
`.jsx`, where the unit suite cannot reach them. **Deferred:** both are
two-way choices now covered by journeys.
- Behaviour [follow-up]: Save and Copy with nothing to send (above).
**Deferred:** the buttons were enabled in that state before this PR.
- Tests [follow-up]: the invalid-username step of the issue's journey.
**Deferred** until #634 lands.
- **Since review:** `90ae783 → 529b7ae` checked; the change is the
journeys and the one `<span>`. Lint clean, `npm test` 1954 pass,
journeys 119 pass on the new head.
</details>
<details>
<summary>Implementation notes</summary>
- `tests/e2e/helpers/git-site.cjs` gains a `trunkDate` option on
`makeSite`: the app reads a site's age from its trunk commit, not from
its record.
- `NOT_A_HANDLE` in `wporg-handle.cjs` became `notAHandle()`, so it is
translated when said.
</details>
<details>
<summary>Screenshots or recording</summary>
Nothing changes in English. In the pseudo-locale the dialog is bracketed
throughout, which the journeys assert.
</details>
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<strong>{appliedPatchLabel} is part of this checkout.</strong>{' '}
28
-
The app cannot safely separate its author’s changes from edits made afterward, so this combined patch cannot be submitted as your work. You can still use <strong>Save</strong> to keep an unattributed copy; revert the applied patch before submitting.
46
+
<span>{headline}</span>{' '}
47
+
<span>
48
+
{createInterpolateElement(
49
+
// translators: <strong>Save</strong> is the name of the Save button.
50
+
__('The app cannot safely separate its author’s changes from edits made afterward, so this combined patch cannot be submitted as your work. You can still use <strong>Save</strong> to keep an unattributed copy; revert the applied patch before submitting.'),
51
+
{strong: <strong/>}
52
+
)}
53
+
</span>
29
54
</Notice.Description>
30
55
</Notice.Root>
31
56
);
@@ -64,7 +89,7 @@ export function ReviewDialog({
64
89
}){
65
90
return(
66
91
<Modal
67
-
title="Review & submit changes"
92
+
title={__('Review & submit changes')}
68
93
onRequestClose={onClose}
69
94
shouldCloseOnClickOutside
70
95
isFullScreen
@@ -73,14 +98,22 @@ export function ReviewDialog({
Could not load your changes. Close this panel and try again. The error is shown below.
116
+
{__('Could not load your changes. Close this panel and try again. The error is shown below.')}
84
117
</Notice.Description>
85
118
</Notice.Root>
86
119
) : null}
@@ -123,8 +156,8 @@ export function ReviewDialog({
123
156
{!loading&&hasChanges&&(
124
157
<divclassName="patch-destinations">
125
158
<div>
126
-
<Textvariant="heading-md"render={<div/>}>Where this patch goes</Text>
127
-
<Textvariant="body-sm"className="muted-label"render={<div/>}>The pull request is the one the app sends for you. The others save a file for you to send.</Text>
159
+
<Textvariant="heading-md"render={<div/>}>{__('Where this patch goes')}</Text>
160
+
<Textvariant="body-sm"className="muted-label"render={<div/>}>{__('The pull request is the one the app sends for you. The others save a file for you to send.')}</Text>
0 commit comments