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 4bee60f
Browse filesBrowse the repository at this point in the historyBrowse files
Draw the window's confirmations as the design system's notices, and say a failed setup there (#602)
## Why
Part of the redesign in #542, item 8 of #557: the toasts. The window's
confirmations were the older component library's snackbars, restyled by
hand in the page's own stylesheet, and only one of their sentences could
be translated.
One thing they could not say at all: why a site failed to set up. That
was shown only if the create-site dialog happened to be open when the
setup failed, and lost otherwise (#600 left it for here).
## What changes
**The confirmations are the design system's notices**, stacked in the
window's bottom corner as the prototype has them: green for what worked,
red for what did not, each with a **Dismiss** button.
What they do is what the snackbars did:
- One for something that worked is said politely and goes by itself
after six seconds.
- One for something that did not is said at once and does not time out.
- A repeat of the newest is not shown or said twice, and three are kept
at most.
What is different:
- **Every notice has a Dismiss button.** A snackbar was dismissed by
pressing it anywhere. Dismissing one leaves the focus on the stack, as
the snackbars' list had it.
- **The stack is a region named "Notifications"**, which a screen reader
can go to.
- **A site that could not be set up is said there**, as an error. The
create-site dialog's message is its own again: which answer is missing.
Its two ways in no longer differ over a message left behind, since none
is.
- **The sentences can be translated.** The ones a value or a branch
chooses are worded in `confirmations.cjs`, with unit tests: what the
apply flow did, a saved patch, an opened pull request, a deletion or a
setup that failed.
- **Tooltips of the design system are drawn over everything.** They were
under the stack, so the name of a notice's button was hidden by the
notice above it.
Not in this pull request, by decision: **no new confirmations.** The
prototype raises a toast for many things the app does not confirm (a
rename, a deletion, a server starting). The app says what it said
before, plus the failed setup.
## How to test this
Platforms: any. The change is in the renderer only.
The journeys walk the stack on both platforms. From the repository's
root:
```bash
npm run build:once && npx playwright test --project=journeys toasts.spec create-site.spec i18n.spec
```
`toasts.spec.js` is new: a confirmation is shown and said once, goes at
its time and not before, can be dismissed; an error is said at once,
stays, and is joined by a later success without going with it; a
notice's tooltip is over the notice above it. `create-site.spec.js`
covers the failed setup, with a dialog open and with none. Each claim
was checked by breaking it: 23 breaks, all red.
By hand, on the current head, on either platform:
**Starting state:** a site that is set up.
1. Open the site's menu (⋮, top right) and choose **Copy path**.
- A green notice "Copied the path" appears in the bottom right corner,
above the footer. It goes after about six seconds.
2. Choose **Copy path** again and click the notice's **✕**.
- It goes at once.
3. Turn the network off, click **Create new site**, fill the dialog in
and click **Create site**.
- The dialog closes. When the clone fails, a red notice says "The site
could not be created." and why. It is still there a minute later.
4. While it is showing, choose **Copy path** again and rest the pointer
on the green notice's **✕**.
- The green notice is under the red one. A "Dismiss" tooltip shows over
the red notice, not behind it.
5. Click the red notice's **✕**.
- It goes.
**What must not have happened:**
- Step 3 left nothing in the sites list for the site that failed, and no
dialog open.
- The red notice did not go by itself in step 4, when the green one did.
- A notice never stopped a click on what is beside it: the stack takes a
press only where a notice is.
## Risks and limitations
- **Review:** two passes by a separate context. 3 to fix here on each,
all fixed. The first pass caught a mistake of mine: I had set the
lifetime to ten seconds, believing that was the snackbars'. It was six,
and is six. Details below.
- **Checked on macOS only**, at the default size and in a 1024×728
window. Windows is CI's.
- **With the site's status read slowed by a second, two journeys fail
that this change does not touch**: `site-header.spec.js` (a viewport
check) and `trunk-update.spec.js` (an update asked for straight after a
ticket is linked). Both fail the same way on `redesign/wpds` without
this change; I checked. They pass at normal speed and have been passing
on the Windows runner. The eight other files that read a toast pass
slowed, 25 of 25.
- **The manual steps above were not run by a person.** I looked at
screenshots of one notice, of two, and of the tooltip.
- **A failed setup's notice shows the error as the main process words
it** ("Error invoking remote method 'wordpress:setup': Error: …"). The
dialog showed the same text. It is now in a notice that stays in front
of a newcomer.
- **Three newer notices push out the oldest, an error included.** As
before. A reducer that drops successes first would be a change of
behaviour.
- **A notice raised while a dialog is open cannot be reached by
keyboard** until the dialog closes: the dialog keeps the focus. It is
still said, and can be dismissed with the pointer, which leaves the
dialog open; I ran that with the rename dialog.
- **The "Notifications" region is always there**, empty when nothing is
said.
- **The five sentences worded where they are raised** ("Updated to the
latest trunk", "This site is ready to work on", and three more) go
through the translator, but no test would notice if one stopped: the
unit tests reach the module's, and the pseudo-locale journey reads
"Copied the path".
- **The guide's other pages** still describe the redesign's earlier
screens (#561). One sentence is corrected here because this change made
it false.
## Related
Part of #557. Part of #542. Follows #601. Settles what #600 left about a
failed setup.
---
<details>
<summary>Design decisions and alternatives considered</summary>
- **Six seconds, not the prototype's 3.2.** The prototype's is short for
a sentence, and the app's has not changed.
- **A Dismiss button on every notice.** The prototype's notices have
none, and its errors time out like the rest. An error that stays needs a
way to go, and a success could always be sent away early.
- **The dialog no longer shows why a setup failed.** Two places saying
it, one of them only sometimes, is what lost it. With the notice saying
it always, the dialog's line is its own and needs no owner outside it.
- **No shadow in the prototype; one here.** A notice over a page of
cards reads as one more card without it. It is the design system's
elevation token.
- **The journeys find the stack by its label, not its role.** A
confirmation can be raised while a dialog is open, for something done
inside it, and an open dialog hides everything else from the roles.
- **Every lookup of a toast's text goes through the stack.** A notice is
said through a live region elsewhere in the document, which holds the
same text; a search of the whole page finds both.
</details>
<details>
<summary>Review outcome (required — see AGENTS.md)</summary>
3 [fix here] · 3 [follow-up] on the first pass, all 3 fixed. 3 [fix
here] · 2 [follow-up] on the second, all 3 fixed.
- **Review:** completed — a separate agent context, against
`.github/instructions/code-review.instructions.md`; head `3297f74`, base
`6c7e6792026c533f8e91de7105c61ef5505d6e2b`; it read the diff and the
libraries' source and ran lint and the unit tests, not the journeys.
Fixed:
1. The lifetime was ten seconds, said to be the snackbars'. Theirs was
six (`NOTICE_TIMEOUT` in `@wordpress/components` 41). It is six.
2. Dismissing a notice left the focus at the top of the document. It is
put on the stack.
3. The guide said a failed setup's reason goes to the log and not to a
dialog.
- Follow-ups taken: the design system's tooltips under the stack; two
steps of the journey. Not reproduced when run: dismissing a notice
closing an open dialog.
- **Second pass:** completed — a separate agent context; head
`953f4a32e08a9b3a274a7a9979a4382d7f87b755`, same base. No finding in the
app's code. Fixed, all in `toasts.spec.js`:
1. The check that a confirmation is still there depended on how fast the
machine was. The clock is stopped under it.
2. The check that a repeat is not shown twice could be read before a
second one had appeared. It is told by what is left after one is
dismissed, and by what was said by the end.
3. The tooltip's place could be read before the tooltip had been moved
to its button. It is read once it reaches the notice above.
- Also taken: the guide's sentence said the notice stays until
dismissed, which three newer ones can undo. Left: the main process's
wording in the notice; in Risks.
- **Since review:** `953f4a3` →
`ba41d4e633b8c7f8e001a5e3ef9930907302f67e`: the journey and one sentence
of the guide. No source under `src/` changed. Not reviewed again; every
suite was run on it, and eight breaks go red against the rewritten
journey.
Checks on `ba41d4e633b8c7f8e001a5e3ef9930907302f67e`, macOS:
| Check | Result |
|---|---|
| `npx eslint . --max-warnings=0` | clean |
| `npm test` | 1846 pass, 2 skipped |
| `npm run test:electron` | 1847 pass, 1 skipped |
| `npm run test:e2e` | 95 of 95 |
| `npm run test:e2e:packaged` | 15 of 15, on an unsigned build |
| Ten journey files in a 1024×728 window | 33 of 33 |
| Eight journey files with the status read slowed | 25 of 25 |
</details>
<details>
<summary>Implementation notes</summary>
- `src/renderer/components/toast-stack.jsx` (new): `ToastStack`.
- `src/renderer/confirmations.cjs`: the reducer is as it was. New:
`toastView`, `TOAST_LIFETIME_MS`, `setupFailureMessage`,
`applyDoneMessage`, `patchSavedMessage`, `savedAndResetMessage`. The
module now needs `@wordpress/i18n`.
- `src/renderer/index.jsx`: the stack in the same portal on `body`; the
failed setup's notice; `createSiteError` gone.
- `src/renderer/index.html`: the snackbars' rules are gone.
`src/renderer/shell.css`: the stack, and `--wp-ui-tooltip-z-index` on
`:root`, which is where the design system reads it from.
- `tests/e2e/helpers/ui.cjs`: `toasts`, `toast`. Eight journey files
used `getByTestId( 'snackbar' )` or a search of the whole page.
- The toasts journey uses Playwright's clock. Stopped, it also stops the
app's menus from being pressed, so it is stopped only between a notice
appearing and its time being up.
</details>
<details>
<summary>Screenshots or recording</summary>
Not attached: I have no way to upload an image from here. What the
screenshots I took show:
- **Two notices** in the bottom right corner, above the footer, 360
pixels wide: a red one with a warning icon and four lines of bold text,
and under it a green one with a tick, "Copied the path". Each has a ✕ at
its right.
- **The tooltip:** a small dark "Dismiss" label above the green notice's
✕, drawn over the lower edge of the red notice.
</details>
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Copy file name to clipboardExpand all lines: TESTING.md
+1-1Lines changed: 1 addition & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -221,7 +221,7 @@ Every matrix uses `fail-fast: false`, so a Windows failure never hides the macOS
221
221
-**`npm test` is separated from the end-to-end suite by filename, not by directory.**`node --test` is run with no path at all, so it walks the whole repository and collects by name: `*.test.cjs` and four other shapes Node treats as test files — `*-test.cjs`, `*_test.cjs`, `test-*.cjs`, `test.cjs`. The journeys are `*.spec.js`, which matches none of them. Now that both suites live under `tests/`, that naming is the only thing keeping them apart, and a file under `tests/e2e/` named any of the five would silently join the fast suite and cost it an app launch. **End-to-end files are `.spec.js`.** The separation is worth defending: the fast suite has to stay something you run without thinking.
222
222
- End-to-end selectors read the text and roles already on screen. No `data-testid` — the visible copy is the contract, and renaming a button is a change worth noticing.
223
223
- Journeys run in the `en-US` locale, the source language, so no translation catalog loads and they read English text as written. The exception is `tests/e2e/journeys/i18n.spec.js`, which launches with `lang: 'en-XA'` (Chromium's pseudo-locale) and fails on any visible text that did not go through `__()`. See [Translatable strings](CONTRIBUTING.md#translatable-strings).
224
-
- What is on screen is named once, in `tests/e2e/helpers/ui.cjs`, for the journeys, the other end-to-end specs and `scripts/screenshots/` alike, so renaming a button or rebuilding a card is one change. It holds every control that more than one of those files reaches, and the locators that find a part of the screen rather than something to press: the rendered app, an entry of the sites list, what a window with no site says, the open site's heading, a card, the work-item card with its heading and its ticket's number, and a ticket's row. A sentence one journey asserts, a button only one journey presses, and a selector only one file needs stay in that file: they are what it is about.
224
+
- What is on screen is named once, in `tests/e2e/helpers/ui.cjs`, for the journeys, the other end-to-end specs and `scripts/screenshots/` alike, so renaming a button or rebuilding a card is one change. It holds every control that more than one of those files reaches, and the locators that find a part of the screen rather than something to press: the rendered app, an entry of the sites list, what a window with no site says, the open site's heading, a card, the work-item card with its heading and its ticket's number, a ticket's row, the question asked before a deletion, and the window's confirmations. A sentence one journey asserts, a button only one journey presses, and a selector only one file needs stay in that file: they are what it is about.
225
225
- No test reads a component's source, the `.jsx` under `src/renderer/`, as text. A test that greps the component for a call, a literal or the order of two strings pins the file rather than the screen: it goes red when the code moves and stays green when the screen is wrong. What is on the page and where, a journey asserts, with order read as document order rather than from bounding boxes (the page scrolls as cards appear). How a component is allowed to be written, a lint rule asserts: `react-hooks/refs` is on for the renderer for that reason.
226
226
- Journeys mark each assertion as an **invariant** (must hold under any model of how work is stored) or a **characterisation** (true because of how the app stores things today). A red invariant is a bug; a red characterisation is a prompt to read it and update it deliberately.
227
227
- A journey that needs something only a running process would say, a mail caught or a server's address, says it itself: `webContents.send` from `app.evaluate`, on the channel and in the shape the real sender uses. Check the payload against both ends, the handler in `src/preload.js` and the place `src/main.js` sends it, and say in the test what the stand-in leaves out, such as the store write the real sender makes first. To assert that the page did *not* react, wait until it has heard: after the telling, ask the main process one more question from the page (`window.api.getSitesWithMeta()` is a plain read) and assert once that answer is back. An absence read before then passes whatever the app does.
Copy file name to clipboardExpand all lines: docs/guide/creating-a-site.md
+1-1Lines changed: 1 addition & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -28,7 +28,7 @@ While the clone runs, the site view shows a **Setting up new site…** card with
28
28
29
29
The clone is the first step of the [initial setup checklist](./setup-wizard); the remaining steps stay locked until it finishes. Then the app carries on by itself — installing the dependencies and running the first build without waiting for you — so the only step left to click is starting the dev server.
30
30
31
-
If setup fails, the half-created site is removed from the list — the row simply disappears. The reason goes to the application log rather than to a dialog, so **Help → Open App Log** is where to look when a site never finishes.
31
+
If setup fails, the half-created site is removed from the list — the row simply disappears. The reason is said in a red notice in the bottom corner of the window. Unlike a green one, it does not go away after a few seconds: it waits to be dismissed. The setup's own output is in the application log: **Help → Open App Log** is where to look for the detail.
return`The site is still listed because its folder could not be deleted${code}. Close anything using it, then try again. Folder: ${res.path}`;
117
+
if(res.code){
118
+
// translators: 1: an error code of the system, such as EBUSY. 2: the path of a folder.
119
+
returnsprintf(__('The site is still listed because its folder could not be deleted (%1$s). Close anything using it, then try again. Folder: %2$s'),res.code,res.path);
120
+
}
121
+
// translators: %s: the path of a folder.
122
+
returnsprintf(__('The site is still listed because its folder could not be deleted. Close anything using it, then try again. Folder: %s'),res.path);
123
+
}
124
+
125
+
/**
126
+
* The notice for a site that could not be set up (#557): that it could not,
127
+
* and what went wrong, as it was reported. The setup runs for minutes after
128
+
* the dialog that asked for it has closed, so this is the one place its
129
+
* failure is said.
130
+
*
131
+
* @param {*} error What the setup was rejected with.
132
+
* @return {string} The sentence.
133
+
*/
134
+
functionsetupFailureMessage(error){
135
+
// translators: %s: what went wrong, as the system reported it.
136
+
returnsprintf(__('The site could not be created. %s'),String(error));
137
+
}
138
+
139
+
/**
140
+
* The confirmation for a patch or a pull request that is in the checkout and
141
+
* built, or taken back out. The apply flow names what it did with a verb and
142
+
* a noun, which it also prints in the terminal; here each pair it has is a
143
+
* sentence of its own, so that it can be translated whole.
144
+
*
145
+
* @param {string} verb 'Applied', 'Reverted', 'Checked out' or 'Restored'.
0 commit comments