Repository navigation
Add the new shell: sites list, page area and footer, in a larger window - #590
Conversation
The first slice of the redesign (#542, #555), following the prototype. The window becomes a "My sites" list beside the open site's page, with a footer under the page, and with no site in it a centred empty state. The list is a DataViews list inside an admin-ui Page, which is what the prototype uses; both packages are new here. The open site's page is an admin-ui Page whose header carries the site's name, its project and the button that hides the list. The old site view is unchanged inside it, apart from the name and project tag that the header now shows, and every site's view stays mounted, hidden when it is not the open one. What the list says about a site, and which site a selection opens, is decided in sites-list.cjs. A row's id is the site's path encoded: DataViews builds element ids from it and points aria-labelledby at them, and a path with a space in it named the row after nothing. The window opens at 1280x800, or the screen's work area where that is smaller, with a minimum size; the arithmetic is window-size.cjs. Toasts move to the bottom-right corner, above the footer. The page now reaches the window's top-right corner, where the open site's actions are, and a toast that stays until dismissed sat on the site's menu. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The packages the two new renderer dependencies bring in are bundled by esbuild, so they are excluded from the package like the ones before them; they were shipping unbundled in app.asar. The list is given every row and a constant view: nothing drawn could change the view, and paginating at a hundred only left the next site out. A site's name now comes first in its entry, with what it has to report after it in brackets, so a screen reader hears the name first and typing the start of a name still finds it. The button that hides the list says its state once, as expanded or not, where a pressed state beside a name that changes said it twice. The page's content keeps a maximum width, as the old view had and the prototype's main column has. The shell's own buttons are taken out of the 8px right margin index.html still gives every button, which put a row's click area short of its edge. createWindow asks the screen with no catch around it: every caller runs after the app is ready, and a failure there should be seen. The unit tests' Electron gains a screen, and a test that the window is given what the screen reports. The screenshot harness selects a site by its entry's button; the text it clicked is now under the list's own button. In the journeys, an absence is read after a round trip, "Deleting site…" is read from its row, and two helpers only the shell journey uses move into it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…n controls The second review pass. The shell is an area whose strings are wrapped, and three of the list's were not: what a site's dot reports, the brackets it is said in after the site's name, and "Deleting". They go through __(), with sprintf and _n where a number or another string is put in, and the translation journey gains the list. The button that hides the list names the element it opens and closes; the shell journey now checks that element is there and holds the list. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: WordPress/contributor-toolkit/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 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 |
…594) ## Why #556 is the second slice of the redesign (#542): the open site's header and its details, as the [prototype](https://github.com/jameskoster/wordpress-contributor-toolkit-prototype) draws them. After #590 the page had a header with only the site's name in it, and the site's view still opened with the old lines: a status pill, a pencil, two dates, the path, a link that opened a menu of applications, and a second menu at the far right. **This PR targets `redesign/wpds`, not `trunk`.** ## What changes - **The site's menu is in the page's header**, and holds everything done to a site as a whole: rename, copy the path, show it in the file manager, update to the latest trunk, open the folder in an application, delete. Renaming was a pencil beside the name and opening the folder was a menu of its own beside the path; both are here, the applications under "Open in". What the menu offers is decided in `site-menu.cjs`. - **The header belongs to the window, and what is in it belongs to the open site.** The window leaves a slot in the header and the open site's view fills it, so the buttons sit where the design puts them and still call that site's own functions. Only the open site fills it. - **The site's view is two columns: its cards, and its details beside them.** The details are what the old first lines said, as labelled facts: whether setup is done, when the site was made, the date of its trunk and its age when that is old, where it is, with a button that copies the path, and what it is a checkout of. They stay in view while the cards scroll. What they say is decided in `site-details.cjs`. - **The details can be put away**, with a button in the header. That is the window's choice and not one site's: it stays as it was left when another site is opened. Where the page is too narrow for two columns, the details go under the cards. Two things this needed: - **The app's own element is a stacking context again**, as in the prototype. The design system's menus and tooltips are drawn on `body` with no z-index, and the page's header, which has one, covered them: the menu opened under the page. #590 had removed this so that toasts could rise over dialogs; the toasts are now drawn on `body` themselves, which keeps that. - **The terminal scrolls sideways.** At the default window size, beside the details, its column is narrower than its eighty columns of text. It moves to the tray in #558. **Deliberately not in this PR:** - the header's process menus and its review button, and the sidebar's Server and Latest build sections (#557); - the staleness notice moving into the sidebar, as the prototype has it: see Risks; - "Open in terminal", PHP version and debugging in the details: the app has no such features or settings yet (#559); - the sites list's row actions and its "server running" dot, which follow in their own PR. ## How to test this Platforms: any. Use the Buildkite build for the current head of this branch. The journeys carry this. From the repository root, `npm run test:e2e` runs them all; `tests/e2e/journeys/site-header.spec.js` is new: - the menu looks for applications as it opens and offers each by name under "Open in", while its own list stays as it was; copying the path, opening the folder in an application or in one to be chosen, and showing it in the file manager all act on the site that is open, and on the other site once that one is opened; - a copy from the menu says so once, in a toast, and leaves the details' own button alone; a copy from that button is said to a screen reader once, and its going back is not; - an application that will not open the folder says why on the page, in view even when the page is scrolled to its end, with the way to choose another; - the details stay in view while the cards scroll; they say the open site's folder, project and setup, and the age of an old trunk; their button copies the path and says so; put away, they are out of the accessibility tree and do not take focus, the button says they are closed, and they stay put away when another site is opened. Renaming, updating and deleting through the menu are walked by `rename-site.spec.js`, `trunk-update.spec.js` and `engine.spec.js`, which now reach them there. `i18n.spec.js` gains the details and the menu in the pseudo-locale. What they cannot reach is how it looks, and the real applications: **Starting state:** the app with two sites, one with an editor installed on the machine. 1. Click the button with three dots at the right of the page's header. The menu opens over the page, not under it. 2. Point at "Open in". A second menu has "Other application…" first and your editors by name under it. Click your editor: the site's folder opens in it. 3. Click the button to the left of the three dots. The details slide away and the cards take the width; click it again and they come back. 4. Make the window narrow, or close the sites list and make it narrow. Below a certain width the details sit under the cards. **What must not have happened:** - The menu and the details button are the open site's: after opening another site, "Copy path" copies that site's path. - A toast, such as "Copied the path", still shows over an open dialog. ## Risks and limitations - **The staleness notice stays in the main column**, where the prototype puts it in the details under "Trunk as of". A put-away sidebar would hide the app's one prompt to update, and the next-action cue points at that banner. The details say the age in words. Worth deciding with the design before #557. - **The details' open state is not remembered across restarts.** The prototype keeps it; the app's only store is electron-store, and a preference for it belongs with the settings (#559). - **The button reads "Hide details" and "Show details"**, where the prototype says "sidebar": the window has two things that could be called that. - **Deleting still asks through the system's confirm box**, and a failed copy through its alert, as before. The prototype's dialogs for these come later. - **The main column is 560px wide at the default window size with both the list and the details open.** The old cards were laid out for more; they wrap, and the terminal scrolls sideways. - **The documentation still shows the old controls.** #561. Two shots of the harness were run to check it still drives the app. - **The manual steps above were not run by a person.** I looked at screenshots of the header, the open menu, the details, the details put away and a toast. - **Checked on macOS only.** Windows is covered by CI. - **Pre-PR review, two passes: everything marked to fix here was fixed, and one follow-up is left**, the guide pages that describe the old controls (#561). Two of the findings were of medium weight: a refused open said out of sight, and rows of the menu moving under the pointer. ## Related Part of #556 and #542. Second slice on `redesign/wpds`, after #590. --- <details> <summary>Design decisions and alternatives considered</summary> **A slot in the header, filled by the site's view.** The alternatives were to lift each site's state up to the window, or to give every site a page and header of its own. The first is a large move whose shape depends on slices not built yet; the second puts a header in every hidden view. A slot moves the markup and nothing else, and it is what the design system's own `Page` does for its sidebar toggle. **The applications are in a menu of their own, under "Open in".** The prototype has one "Open in" item naming the editor chosen in the settings. There are no settings yet, so the app offers what detection found, each by name, and always "Other application…", as the old menu did. They are not in the site's menu's own list because detection answers after the menu is on screen: the rows it adds would push "Delete site" down under a pointer aimed at something else. For the same reason "Other application…" is first in its menu, and what arrives is added under it. **"Setup" is a row of the details.** The prototype has no such row. The old view said "Initialized" or "Uninitialized" in a pill before anything else, and a site whose setup did not finish should still say so somewhere. **The details go under the cards by the page's width, not the window's.** The prototype switches at a window width of 960px. Here the sites list takes a share of the window that the page never sees, and it can be closed, so the rule is a container query on the page. **`useId` for the element the button names.** A site's path is not safe in an id, as #590 found; the id only has to tie a button to the element it shows and hides. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> Run as the repository's review procedure, with the judgement pass in a fresh context that took no part in the change. First pass on `6cb7dd5` against `3344172`; second pass on the fix-up `f1420bb`; `538bc8a` applies what the second pass asked for, and the head, `3eb1ee3`, corrects a journey after CI. **First pass: seven findings to fix here, one follow-up; one medium, none high.** It read the removed block line by line and found nothing lost outright: the setup state and the path are now visible only while the details are open, and the path can still be copied from the menu. It read the slot/fill, the design system's menu and base-ui's focus handling in `node_modules` and found the fill rendered by exactly one site at any moment, context and events reaching it, and the rename dialog taking focus cleanly from the closing menu. Fixed: - **A refused open was said off-screen** (medium). The notice is at the top of the cards and the header's menu can be used from anywhere down the page. It is brought into view when it is said. - **Rows of the menu moved under the pointer**, with "Delete site" below them, as detection answered. The applications are in a menu of their own, so the site's menu's own list never changes. - **The details did not stay in view while the cards scrolled**: the sticky rule had no room to act in. One line. - **A copy from the menu was announced twice** to a screen reader. The menu's copy leaves the details' button alone. - **Two strings chosen by branches in a component** moved to `site-details.cjs`, with unit tests. - **The translation journey could scan the menu before the application was in it.** It waits for it by name. - **The journey's header** now says what its stand-ins leave out of the main process. **Second pass: five findings to fix here, one medium.** - **"Other application…" still moved, inside the new menu** (medium), down as applications arrived and up as the "looking" row went. It is first now, with what arrives and goes below it, and a unit test walks the lists before and after an answer. - **A row lost its identity, and the focus on it, when the list changed**: its key came from its position. It comes from what the row is. - **A refusal still on the page was scrolled to again every time its site was opened.** The effect now brings each refusal into view once. No journey holds this: see below. - **The details' Copy button was itself a live region**, so its change back to "Copy" was announced too. A status region beside it holds "Copied" only while it is true. - **A comment about keys said what the code did not do.** **After CI: one journey step removed.** `538bc8a` added a step claiming that a refusal still on the page does not take the page back to itself when its site is opened again. It was red on Windows, on both attempts, and it was the step that was wrong: opening a site moves the page in any case, because the next-action cue centres the next step each time a site becomes the open one (#252). On macOS that scroll is animated and the step read the page before it had moved. The step is gone, the code's comment says whose the page's position is on opening a site, and the journey was re-run in a window the size of the Windows runner's screen. Left as a follow-up: `docs/guide/trunk-updates.md`, `editors.md` and `managing-sites.md` describe "More", "Open directory in" and the copy icon. The screenshot harness still works: its `site-menu` shot was run. **Checks, on macOS, on the head of this branch:** | Check | Result | | --- | --- | | `npx eslint . --max-warnings=0` | clean | | `npm test` | 1774 pass, 2 skipped | | `npm run test:electron` | 1775 pass, 1 skipped | | `npm run test:e2e` (journeys) | 68 of 68 on the head | | `npm run test:e2e:packaged` | 15 of 15, on an unsigned `npm run pack:dir` build of `f1420bb`; the head differs from it in the renderer and in tests, and CI's packaged smoke passed on `538bc8a` on both platforms | **The tests' claims were audited by breaking each one** in the source and running the test that should notice. 25 breaks, all 25 turn a test red on the line that states the claim. | Break | Caught by | | --- | --- | | Every site draws its actions in the header, not only the open one | `site-header` | | The menu does not look for applications as it opens | `site-header` | | Copying from the menu says nothing | `site-header` | | "Other application" opens the first one found | `site-header` | | Showing in the file manager does nothing | `site-header` | | A refused open is not said on the page | `site-header` | | The details' button does not say it copied | `site-header` | | An old trunk does not say how old in the details | `site-header` | | Hidden details still take focus | `site-header` | | Hidden details are still in the accessibility tree | `site-header` | | The details come back when another site is opened | `site-header` | | The details' button names an element that is not there | `site-header` | | Menus are drawn under the page | `site-header` | | Renaming from the menu opens nothing | `rename-site` | | A site being deleted can be deleted again from the menu | `engine` | | Updating from the menu does nothing | `trunk-update` | | The details' heading is not translatable | `i18n` | | A menu item is not translatable | `i18n` | | A refused open is said out of sight | `site-header` | | A copy from the menu also lights the details' button | `site-header` | | The details scroll away with the cards | `site-header` | | The applications are in the menu's own list | `site-header` | | The way to choose another application is last, under what arrives | `site-header`, and `site-menu` (unit) | | A copy from the details is not said to a screen reader | `site-header` | | The copy's going back is said to a screen reader too | `site-header` | Nothing asserts that a toast paints over an open dialog, or that the terminal can be scrolled sideways: both are about what is painted, which the journeys do not read. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
#595) ## Why #557 rebuilds what is inside the site's page, as the [prototype](https://github.com/jameskoster/wordpress-contributor-toolkit-prototype) draws it. It lists nine things; this is the first two, the Server section and the process menus. Until now a site's server, its build watch and the way to its changes were three large buttons at the top of the cards, with a line of links under them, and they scrolled away with the cards. **This PR targets `redesign/wpds`, not `trunk`.** ## What changes - **The page's header has a menu for each of the site's two processes**, the development server and the build watch. Each says what its process is doing, with a dot, and holds the one thing to do about it. While the server has an address its menu also goes to the site and to its admin. Beside the two menus is the button that opens the review. - **The site's details have a section for each process.** The server's says where the site, its admin and its database are once it has an address, and what to log in with, the password shown only when asked for. The build watch's says what it is doing. Each section has a button that starts and stops its process, named as the header menu's item is. - **Both places say the same.** What they say is decided in one module, `site-processes.cjs`. - **The row of three buttons is gone**, with the line of links under it. - **A server that is starting cannot be pressed.** The button and the menu's item say it is starting and are held. Before, the button could be pressed and did nothing. - **The details tell three ends apart**: a watch that ended by itself, a build that failed before the watch could start, and a build that was stopped. The hook now records which process an end was of, and a build stopped by the watch's button or by Ctrl+C in the terminal is a stop, whatever the kill left it with. Before, a stopped build was logged as "failed with code null". Two things the taller details and the fuller header needed: - **The details stay in view only while they fit.** With the two sections they are taller than the page at the default window size, and a column held in place would keep its own end out of reach until the page's end. Taller than what is in view, they scroll with the cards, as in the prototype. - **The header's actions give way by the page's width.** Short of room, the two processes are their dots and the review button is its first word; each keeps its name. At a 1024px window the site's name was otherwise squeezed to nothing. **Deliberately not in this PR**: the other seven items of #557, each in its own PR; and the sites list's row actions and its "server running" dot. ## How to test this Platforms: any. Use the Buildkite build for the current head of this branch. The journeys carry this. From the repository root, `npm run test:e2e` runs them all: - `dev-server.spec.js`: the server's button in the details starts one server, is held while the server starts, as is the header menu's item, and offers to stop it once it has an address; a third, new journey starts and stops the same server from the header's menu, finds nothing but the start in a stopped server's menu and the site and its admin in a running one's, reads the section's three addresses, follows one from each place and finds it opened by the main process with the window still on the app, and reads the credentials, the password only after asking. - `build-watch.spec.js`: the header and the details say the same of the watch as it starts, is stopped from the header's menu, and ends by itself with an error; a build that fails before the watch is said as the build's failure; and a third, new journey stops that build by the watch's button and by Ctrl+C, and finds neither said as a failure. - `i18n.spec.js`: a fourth journey reads the two processes in the pseudo-locale, stopped, starting and running, in the header, in their menus and in the details. - `site-header.spec.js`: details that fit stay in view and details that do not scroll with the cards; a site still in its setup is offered no server, watch or review; and a header short of room, at two widths, keeps the site's name and every action under its own name. - Every journey that starts a server, a watch or the review now does it through these controls: the mail, the logs, the update with the watch running, the review. What they cannot reach is how it looks, and a real server: **Starting state:** a Core site that has finished setup, with its dev server stopped. 1. In the page's header, click "Server stopped", then "Start development server". The menu's button reads "Server starting…" with an amber dot, and in the details the Server section says how long it has been starting. 2. When it is up, the dot is green, the browser opens the site, and the Server section shows "View site", "wp-admin" and "Database", with the username under them. Click the eye beside the dots: the password shows. 3. Click "wp-admin". It opens in the browser; the app's window does not change. Click "Server running" in the header: "View site" and "wp-admin" are there too, above "Stop development server". 4. In the details, click "Start" beside "Build watch". The header's second menu reads "Build watching". 5. Make the window narrower. First the two menus become their dots, then "Review & submit changes" becomes "Review". The site's name is always there. **What must not have happened:** - A second press while the server starts does nothing: there is one server. - Stopping the server leaves the build watch running. - On a site with no build yet, such as a Gutenberg checkout before its first build, "Start build watch" and then "Stop build watch" while it still says "Build building" leaves a grey dot and "Build watch offline", not a red dot and a failed build. ## Risks and limitations - **The server's address is no longer shown as text**, only behind "View site". The prototype has it so. - **The build watch's tooltip is gone**, the one that said it compiles `src/` edits on save and runs apart from the server. The section's sentence says the first half while the watch runs. - **The next-action cue no longer scrolls to "Start dev server" or to the review button**: they are in the header, which is always in view. It still puts its glow on them and says the step for a screen reader. - **At the default window size the details are taller than the page once a site is set up**, so they scroll with the cards from the start. They stay in view only on a taller window, or while a site is in its setup. - **With the processes shown as dots, two grey dots look alike.** Each has its name as a tooltip and for a screen reader. - **How the window handles a link is unchanged**, and is tracked separately. The new links are opened as the old ones were. - **A paused watch offers "Start"**, as it always has, now beside a sentence saying it comes back by itself. Starting it during an apply is an old gap, left for its own change. - **A start that throws still leaves the button dead until the app is restarted**, and **a boot that never answers still cannot be cancelled**: the button is now held where before it could be pressed and did nothing. Neither is new. - **The guide pages still describe the three buttons.** #561. - **The Logs tab and the details can name the same end differently.** The tab reads "Build watcher (exited 1)" for a build that failed before any watcher existed, and "(stopped)" for a watch that ended with no code, which the header and the details now draw as failed. The tab's label is older than this and is not changed here. - **The time inside "Dev server is starting… (12s)" is not translatable**: the sentence is, the unit is not. - **A menu of the design system, opened again while it is still closing, can miss the Escape that follows.** Found by a journey at machine speed, where it took about one Escape in two; a second closes it, and it did not happen in 60 runs once the closing menu had gone, which takes less time than a person needs. The journey waits for the closed menu to be gone. - **About 1,160 lines added, 546 of them tests.** It is one pull request because the header's menus and the details' sections draw from one module, and the three buttons could not go until both were there. - **The manual steps above were not run by a person.** I looked at screenshots of the header and details with the server stopped and starting, and of the header at three widths. - **Checked on macOS only**, with the journeys this touches also run in a window resized to the Windows runner's 1024×728. Windows itself is CI's. - **Pre-PR review, two passes: everything marked to fix here was fixed, eleven findings, one of medium weight; six follow-ups are left**, all in this list. The medium one was a build stopped by hand shown as a build that failed. ## Related Part of #557 (items 1 and 2 of its nine) and #542. Third slice on `redesign/wpds`, after #590 and #594. --- <details> <summary>Design decisions and alternatives considered</summary> **"Build watch", not "Latest build".** The prototype's section reports the last compile: whether it succeeded, how long ago, how long it took, which files. The app does not record any of that; it knows the watch's state and what the watcher printed. So the section is named for what it is and says the state in a sentence. Recording compiles is a feature of its own. **One module for both places.** The header and the details show the same two processes. Two sets of conditions would drift, and the first sign would be a header saying "running" over a section saying "offline". **A starting server is held, not pressable.** The prototype draws it so. The guard that made a second press do nothing (#488) stays in the hook, but no press reaches it any more, so the journey's claim is the held button and the held menu item, and one server counted. **The header's server menu has the site and its admin in it.** The prototype's menu has only start and stop. The details can be put away, and under about 1088px of window they sit at the foot of the page; the two links most often wanted should not go with them. The database stays in the details. **The processes and the review are not offered during setup.** The old row of buttons was not drawn then either: the checklist's last step is what starts the server the first time. **The details let go by measuring, not by a rule in the stylesheet.** Whether they fit depends on the window's height and on what they are showing, which changes as a server starts. The prototype measures too. **The header gives way by the page's width, with container queries.** The page's width is the window's less the sites list, which can be closed, so a rule on the window's width would be wrong half the time. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> Run as the repository's review procedure, with the judgement pass in a fresh context that took no part in the change. First pass on `6c6cc59` against `f066f94`; second pass on the fix-up `ac56e03`; the head, `e8dee93`, applies what the second pass asked for. **First pass: seven findings to fix here, five follow-ups; none high.** It confirmed that the disabled states, the next-action cue, the setup checklist's own start step and the way out of an update that is waiting for the watch all carried over from the three buttons. Fixed: - **A watch that ended by itself with code 0, or killed by a signal, showed as a plain grey "stopped".** The old dot was red for every end nobody asked for. Every such end is drawn as failed, with the code when there is one. - **A build that failed before the watch could start was blamed on the watch**, which had never started. The hook records which of the two ended. - **What the server's section shows was decided in `index.jsx`**, out of the unit tests' reach. It is in `site-processes.cjs`. - **The journey's double click no longer reached the guard it named** (#488): the second click landed on a held button. The journey now claims what holds: the button and the menu's item are held while the server starts. - **"Dev server is starting…" was not translatable**, and the translation journey scanned none of the new parts. The string is wrapped and a fourth journey scans the two processes in three states. - **The check that the window stays on the app could pass after a link had navigated**: it read the address too early. It asks the main process, after a round trip. - **The header's process menus were reached from three journey files**, each with its own locator. One helper in `ui.cjs`. One follow-up was done here instead: the site's links were only in the details, which can be put away. "View site" and "wp-admin" are in the header's server menu. **Second pass: four findings to fix here, one medium; three follow-ups.** It traced every write of the watch's state and found that the record of which process ended cannot go stale, and that the server's address, "running" and "starting" are set and cleared together, so the section cannot be drawn from inputs that disagree. Fixed: - **A build stopped by hand was shown as a build that failed** (medium): a red dot and "the build … failed". A killed process ends with no code, and the first fix-up had made every unasked-for end a failure. The build's end is left alone when a stop has already said the watch is idle, and Ctrl+C says so before it kills. The new journey is red without either half. - **Which links the header's menu is given was decided in `index.jsx`.** It is in `site-processes.cjs`, with a unit test. - **The dev server journey's header still described a menu with one thing in it**, and **a count left over from the double click could not fail.** Rewritten and removed. One follow-up was done here: the translation journey now waits for the server and the watch to have been asked for before it speaks for them. Left as follow-ups, all in Risks above: a paused watch offering "Start"; a start that throws leaving the button dead; a boot that cannot be cancelled; how the window handles a link; the Logs tab's label; and the time's unit. **Checks, on macOS, on the head of this branch:** | Check | Result | | --- | --- | | `npx eslint . --max-warnings=0` | clean | | `npm test` | 1793 pass, 2 skipped | | `npm run test:electron` | 1794 pass, 1 skipped | | `npm run test:e2e` (journeys) | 73 of 73 | | `npm run test:e2e:packaged` | 15 of 15, on an unsigned `npm run pack:dir` build of the head | | `dev-server`, `build-watch`, `site-header`, `i18n` in a 1024×728 window, the Windows runner's | pass | **The tests' claims were audited by breaking each one** in the source and running the test that should notice. 30 breaks, each run on the head, and all 30 turn a journey red on a line that states the claim. | Break | Caught by | | --- | --- | | The header's server menu does nothing | `dev-server` | | A link of the server's section is not opened by the main process | `dev-server` | | The password is shown without being asked for | `dev-server` | | A server that is starting can be pressed | `dev-server` | | The header says the server is stopped whatever it is doing | `dev-server` | | The server's section has no links | `dev-server` | | The header's build menu does nothing | `build-watch` | | The details do not say what the watch watches | `build-watch` | | A watch that ended does not say with what code | `build-watch` | | A build that failed before the watch is said as a watch that ended | `build-watch` | | A build stopped by the watch's button is said to have failed | `build-watch` | | Ctrl+C on that build is not taken for a stop | `build-watch` | | The header's server menu has no links | `dev-server` | | The header's server menu has the database too | `dev-server` | | A link in the header's menu opens nothing | `dev-server` | | A stopped server's menu offers a link | `dev-server` | | A link takes the window with it | `dev-server` | | The starting sentence is not translatable | `i18n` | | A process menu's item is not translatable | `i18n` | | A link's name is not translatable | `i18n` | | The credentials' heading is not translatable | `i18n` | | The header's name for a process is not translatable | `i18n` | | The sentence for a watch that ended is not translatable | `i18n` | | Details taller than the page stay held in place | `site-header` | | Details that fit scroll away | `site-header` | | The processes keep their words however narrow the header | `site-header` | | The review button keeps all its words however narrow | `site-header` | | The header's review button does nothing | `review-changes` | | An update of trunk does not hold the server's button | `trunk-update-watch` | | A site in its setup is offered the server and the review | `site-header` | </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
## Why #557 rebuilds what is inside the site's page, as the [prototype](https://github.com/jameskoster/wordpress-contributor-toolkit-prototype) draws it. This is item 3 of its nine, the ticket card: where a site links the Trac ticket or the GitHub issue it is working on, and reads what already exists on it. It was about 300 lines of inline markup in the middle of the site's view, in the old look. **This PR targets `redesign/wpds`, not `trunk`.** ## What changes - **The card is drawn with the design system**, as the prototype has it: a card with a title, "Trac ticket" or "GitHub issue"; with nothing linked, a field, its button and the way to find a first ticket; with one linked, the ticket's number and summary as the way to it, what Trac said of it, and under that its pull requests and, on Trac, its attachments, each as a list hanging from its heading. - **It is a component of its own**, `ticket-card.jsx`, and what it says is decided in `ticket-card.cjs`: its words for a ticket and for an issue, what Trac said as a badge and a line of facts, each row, and what each list says besides its rows: reading, empty, or failed and with what in its place. It has unit tests of its own. - **Its strings are translatable.** A ticket and an issue get whole sentences each; before, one sentence had the noun put into it. - **A button that is held still says why**, in the design system's button: `ReasonedUiButton`, beside the old one, which the rest of the page still uses. - **The two "Refresh" buttons are named for their lists**: each is its word and then its section's heading, as both are on screen. Each shows the one word. - **The notice that trunk has moved is still said to a screen reader.** The old block was a status region. The design system's notice says what is in it, and with a held button in it that reading fails and says nothing, so it is told its sentence. - **One sentence named a button that was not there.** After Trac's window was closed the card said to click "Show Trac attachments", which is gone once a read has answered. It says "Refresh". **Deliberately not in this PR:** - the pull request's banner, the note about uncommitted changes, and what a link or a switch says back: they are made by the site's view, placed in the card, and keep their old look until items 4 and 9; - the "Apply a patch or PR" card under it (item 4) and the list of a site's other tickets (item 5). ## How to test this Platforms: any. Use the Buildkite build for the current head of this branch. The journeys carry this. From the repository root, `npm run test:e2e` runs them all; `tests/e2e/journeys/ticket-card.spec.js` is new: - with nothing linked the card asks for a ticket and its button waits for one to be typed; the way to find a first ticket, the ticket's number, its component, a pull request's title and an attachment's name each open their page through the main process; - linking reads the pull requests and the ticket once each with nothing more pressed; the card then says the ticket's summary, status and facts, each pull request's number, state and date, and each attachment's uploader, age and size; - a list that could not be read says why and keeps the rows it last had, or says there is nothing to fall back on; an empty one says it is empty; Trac's window closed early is said as that, and not as "no attachments"; - a site opened with a ticket already linked does not go to Trac until asked, and offers to in both places; asked once, it fills in both; - while a build runs, Unlink, the field and Link are held, say why as their description, can still be reached, do nothing when pressed, and come back as the same buttons. `ticket-rebase.spec.js` now also holds that the notice about trunk having moved is said, by recording what the page's live regions say. `i18n.spec.js` gains the card in the pseudo-locale, with nothing linked, with a ticket and its lists, and with a failed and an empty list. `pr-checkout`, `ticket-branches`, `gutenberg-site` and `open-pull-request` now find the linked ticket by its number in the card. What they cannot reach is how it looks, and the real Trac: **Starting state:** a Core site that has finished setup, with no ticket linked. 1. In "Trac ticket", type `62281` and click "Link ticket". Trac's window opens for its human-check; pass it. The card shows `#62281` with the ticket's summary, a badge with its status and a line of facts, then "Linked pull requests" and "Trac attachments", each row with a tick to the rule down its side. 2. Click the ticket's number. The ticket opens in the browser; the app's window does not change. 3. Click "Refresh" beside "Linked pull requests" with the network off. A yellow notice says GitHub could not be reached and, under that, when the list on screen was read; the rows stay. **What must not have happened:** - Opening the app with a ticket already linked does not open Trac's window by itself. - "Unlink" during a build or an update does nothing, and says why on hover. ## Risks and limitations - **The explicit "Open in Trac" button is gone.** The ticket's number and summary are the way to it, with "Open in Trac" as the tooltip, as the prototype has it. A newcomer has to recognise a link. - **"Unlink" is no longer red.** It is a plain button in the card's header, as in the prototype. It parks the ticket's work and loses nothing, which red overstated. - **The linking prompt is shorter.** It no longer says the ticket is stored with the site, survives restarts and can be changed or removed. The prototype's sentence is "Link the ticket you're working on." - **Three things inside the card keep the old look**: the checked-out pull request's banner, the note about uncommitted changes, and what a link or a switch says back. They are the site's view's and are redrawn with items 4 and 9. - **"Apply…" and "Show Trac attachments" are held during a build, an install, an update or an apply with no stated reason**, as before; held buttons of the design system can now be reached by keyboard, so the missing reason is met more often. Several "Apply…" buttons share one name, as before. Both go with item 4, which redraws the apply. - **Announcements can overwrite one another.** Right after a link, "Linking ticket", "Checking GitHub" and "Reading the ticket on Trac" are put in the one live region in quick succession; and a refresh that fails twice with the same sentence is said once. The first is how the design system's loading buttons speak. - **The sentence the stale-ticket notice speaks is its two sentences joined with a space**, and those two, with every reason a button is held for, are not translatable yet: they come from modules this PR does not touch. The translation journey does not reach that state. - **For a moment after a build ends, "Unlink" can be pressed and is turned away** with "A command is already running": the card's buttons come back before the terminal does. It is older than this card, and longer on a slow disk; CI's Windows runner found it. Left for its own fix on `trunk`. - **Section headings are as large as the card's title**, as in the prototype. - **The guide pages still show the old card.** #561. - **About 1,475 lines added, 680 of them tests**, against about 360 removed. It is one pull request because the card is one block of the site's view: it could not be half moved. - **The manual steps above were not run by a person**: they need Trac's human-check. I looked at screenshots of the card with nothing linked, with a ticket and its lists, with a failed read of each list, and in a 1024px window. - **Checked on macOS only**, with the journeys this touches also run in a window resized to the Windows runner's 1024×728. Windows itself is CI's. - **Pre-PR review, two passes: everything marked to fix here was fixed, nine findings, two of medium weight; nine follow-ups are left**, all in this list or in the review outcome. The medium two were a notice that was no longer said to a screen reader, and a journey whose stand-ins could be late. ## Related Part of #557 (item 3 of its nine) and #542. Fourth slice on `redesign/wpds`, after #590, #594 and #595. --- <details> <summary>Design decisions and alternatives considered</summary> **The ways out are buttons drawn as links.** The prototype has anchors. They were buttons in the old card, and the main process opens the page; an anchor would also be something the window itself could follow. **The heading is the way to the ticket.** The old card had the number in a pill and an "Open in Trac" button beside it. The prototype makes the number and the summary the link; "Open in Trac", or "Open on GitHub", is its tooltip. **"Milestone:" is labelled.** The prototype's line reads "Defect (bug) · General · 7.2 · Opened 4 weeks ago". A real ticket's milestone is as often "Awaiting Review" or "Future Release", which beside a status badge reads as a second status. **"Apply…" keeps its dots.** The prototype says "Apply" and opens a dialog. Here the button reads the patch into the card below, which item 4 redraws; the dots say that it does not apply yet. **A closed pull request has no colour, a merged one is blue.** The old pill used GitHub's own three colours. The design system's badge has no purple, and its red means that something failed, which a closed pull request has not (#227). **"Read details from Trac" stays**, where the prototype has no such button: it always has the details. Reading them opens Trac's window and may ask for a human, so the app does it once after a ticket is linked and afterwards only when asked. **`pr-state.cjs` stays**, and the card's badge takes from it which of the three a state is. The apply's preview still draws its old pill from there, until item 4. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> Run as the repository's review procedure, with the judgement pass in a fresh context that took no part in the change. First pass on `fde8836` against `5388e5d`; second pass on the fix-up `9b7a6ef`; `86bd6b5` applies what the second pass asked for, and the head, `4b0f828`, corrects a journey after CI. **First pass: five findings to fix here, eight follow-ups; two medium, none high.** It compared the removed block with the new card line by line and found every gate unchanged: when "Apply…" is hidden and held, when each "Refresh" and "Show Trac attachments" show, what holds Unlink, Link and the field, the Enter key, the next-action cue, and the spacing of what is handed into the card. Fixed: - **The notice that trunk has moved was never said** (medium). The design system's notice says its content by rendering it to a string, the held button in it cannot be rendered that way, and the failure is swallowed. The notice is told its sentence, and `ticket-rebase.spec.js` holds that it is said, red without the fix. - **The journey that reopens a site with a ticket linked installed its stand-ins after the window existed** (medium), so the first read of the pull requests could go to GitHub and a read of Trac made early would not be counted. A session can now run a journey's code before its window is waited for, and the journey checks that its stand-in answered that read. - **"A press on a held Unlink does nothing" could not fail**: the journey read the branch before a press that got through could have moved it. It now reads what the card would say at once if the press had got through. - **Two controls were named in two journey files** and are in the shared helpers; `TESTING.md` no longer counts them. - **Translation**: two sentences that quote a button now say so to translators; a failed read is two lines and not two sentences joined with a space, and so is the way to a first ticket; each Refresh is named from the two strings on screen, so its name cannot disagree with what it shows. Three follow-ups were done here too: the ticket field's helper matches a whole label; the registry's unused browse label is gone; a held button's reason is no longer also read as a sentence of its own. And the component is labelled "Component:", which following the prototype had dropped. **Second pass: four findings to fix here, three follow-ups; all low.** It read the design system's button, tooltip and notice and confirmed the names, the description and the spoken sentence reach the page as intended, and that the fourth journey's premise holds: a press that got through raises an alert in the card before anything is asked of the main process. Fixed: - **A count in that journey could not tell the two cases apart**, for the reason above: removed, with the wrap that fed it. - **The rebase journey's record of what was said kept only the last of two messages put in a region in one turn.** It keeps what each change put there. - **Two comments named one of the two places the registry's label is read.** - A comment line ran long. Two follow-ups were done here: a session checks its throwaway profile before a journey's code runs against the app, and the attachments' failure is told to a screen reader as written, since read out of its markup an "&" in an error would be said as "&". Left as follow-ups, all in Risks above except the last two: no stated reason on "Apply…" and "Show Trac attachments"; several "Apply…" with one name; announcements overwriting one another; the stale notice's spoken sentence joined in the component, to move to its module when that module is translated; heading sizes; the shorter prompt; the guide pages; a few small decisions still in the component (when a row's button is held, when "Refresh" shows); and two notices that still say what is in them the default way, which works for what they hold. **After CI: one journey corrected.** The journey that holds the card's buttons during a build was red on Windows, on both attempts, at the Unlink it presses once the build has ended. When a build ends the card's buttons come back at once and the terminal a moment later, after the site's status has been read again; a press in between is turned away with "A command is already running". On macOS that read is over before the journey presses, and on the Windows runner it is not. The journey now waits for the terminal's hint to be a link again. The failure was reproduced here by slowing the status read by a second and a half, and the corrected journey passes with it slowed. The gap itself is the app's and older than this card: see Risks. **Checks, on macOS, on `86bd6b5`; the head differs from it in that one journey, which was run again on the head, also in the small window:** | Check | Result | | --- | --- | | `npx eslint . --max-warnings=0` | clean | | `npm test` | 1811 pass, 2 skipped | | `npm run test:electron` | 1812 pass, 1 skipped | | `npm run test:e2e` (journeys) | 78 of 78 | | `npm run test:e2e:packaged` | 15 of 15, on an unsigned `npm run pack:dir` build of the head | | Eight journey files that reach the card or restart the app, in a 1024×728 window, the Windows runner's | 28 of 28 | **The tests' claims were audited by breaking each one** in the source and running the test that should notice. 36 breaks, and all 36 turn a journey red on a line that states the claim. 34 were run on the head; two, marked below, on the commit before it, whose lines the head does not touch. One claim is not shown red: that the reopened site's first read of the pull requests was answered by the stand-in. Making the stand-in late enough to show it would send that read to GitHub. | Break | Caught by | | --- | --- | | The way to find a first ticket goes nowhere | `ticket-card` | | An empty field can be linked | `ticket-card` | | The ticket's number goes nowhere | `ticket-card` | | The ticket does not say where it goes | `ticket-card`, `gutenberg-site` | | What Trac said of the ticket is not shown | `ticket-card` | | When the ticket was opened has no date behind it | `ticket-card` | | The component is not a way to its tickets | `ticket-card` | | "Read details" stays after the ticket has been read | `ticket-card` | | A pull request does not say its state | `ticket-card` | | A pull request does not say when it last moved | `ticket-card` | | A pull request's title goes nowhere | `ticket-card` | | An attachment does not say who uploaded it | `ticket-card` | | An attachment's name goes nowhere | `ticket-card` | | A patch cannot be read from its row | `ticket-card` | | A list that could not be read says nothing | `ticket-card` | | A list that could not be read loses its rows | `ticket-card` | | A failed read does not say there is nothing to fall back on | `ticket-card` | | A failed read says why and not what is shown instead (on `9b7a6ef`) | `ticket-card` | | An empty list says nothing | `ticket-card` | | A read Trac did not answer is said as "no attachments" | `ticket-card` | | A failed read of the attachments says nothing | `ticket-card` | | An unread ticket offers no way to read its attachments | `ticket-card` | | A Refresh button is named by its word alone (on `9b7a6ef`) | `ticket-card` | | A held button does not say why | `ticket-card` | | A held button cannot be reached | `ticket-card` | | A press on a held Unlink gets through | `ticket-card` | | The field is not held during a build | `ticket-card` | | The stale-ticket notice is not told what to say | `ticket-rebase` | | The checked-out pull request's banner is under the ticket's number | `pr-checkout` | | A checked-out pull request can still be applied from its row | `pr-checkout` | | A Gutenberg site's card is worded for Trac | `gutenberg-site` | | A Gutenberg site's card has Trac's attachments | `gutenberg-site` | | A section's heading is not translatable | `i18n` | | A fact's label is not translatable | `i18n` | | A state's word is not translatable | `i18n` | | A failure's sentence is not translatable | `i18n` | The audit also changed the code once: a prop on the held button turned out to do nothing the design system's button does not already do, and was removed. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…dialog (#597) ## Why #557 rebuilds what is inside the site's page, as the [prototype](https://github.com/jameskoster/wordpress-contributor-toolkit-prototype) draws it. This is item 4 of its nine, apply patch and PR checkout: the card a pull request or a patch file is brought into the checkout from, the preview of what one would change, and the banner of a pull request that is checked out. It was about 250 lines of inline markup in the site's view, in the old look. **This PR targets `redesign/wpds`, not `trunk`.** ## What changes - **The apply card is drawn with the design system**, as the prototype has it: a card that can be folded, with a tab for each way in on a project that takes patch files, "Pull request" and "Diff", and the pull request's field alone on one that does not. - **What a patch would change is shown in a dialog** before it is applied, where it was a block in the card: its files, whose work it would land on, anything that changes what applying it means, and the button that applies it. - **The applied patch, a failure with its breakdown, and the checked-out pull request are the design system's notices.** The ones that were alerts or status regions still are. - **It is a component of its own**, `apply-card.jsx`, with what it decides in `apply-card.cjs` and unit tests beside it. Its own strings are translatable. The dialog is in front of the page, and that changed four things about the flow: - **A failed apply drops its preview.** It used to stay on the card beside the error. A dialog would come back in front of the error, so the error is read on the card and the patch is chosen again to retry. - **The preview steps aside while the app asks what should become of loose edits on trunk**, and is kept for the answer that goes on with the checkout. Saying "Cancel" to that question drops it. - **A preview is shown by its own site.** Every site's view is in the window at once, and one read while the contributor had gone to another site would have opened over that one. It waits for its site. - **The dialog's button is held, with the reason, while a command runs in the terminal.** An apply is refused then with a line in the terminal, which the dialog is in front of. Also: - **The card stays open while it has something to say** (the steps of an apply, a patch that is applied, a failure) whatever was chosen, and goes back to that choice afterwards. It remembers which tab it was left on. - **The screenshot harness waits for the ticket's number**, where it waited for a heading that #596 removed. That was missed there. **Deliberately not in this PR:** - translating what the card passes through from older modules: what a pull request's preview and banner say, what an applied patch says, why a patch did not fit, and the steps of an apply; - the list of a site's other tickets under the card (item 5), and the note about uncommitted changes (item 9). ## How to test this Platforms: any. Use the Buildkite build for the current head of this branch. The journeys carry this. From the repository root, `npm run test:e2e` runs them all; `tests/e2e/journeys/apply-card.spec.js` is new: - a patch that was chosen is shown in a dialog named for it, with what it changes, and nothing is written; leaving the dialog by its button or by Escape applies nothing and gives the card back on the tab it was left on; applied from the dialog, the patch is on disk, and focus goes to the card's header; - a patch that does not fit says before anything is written whose work it would land on; refused, its preview goes, the folded card opens to say what went wrong, the ticket's rows can be read again, and once the failure is dismissed the card is folded again; - a preview read for one site is not shown over another, and is there when its own site is opened again; - a patch cannot be applied while a command runs in the terminal, and the dialog's button says why; the command over, the same dialog can apply; - saying "Cancel" to the question about loose edits leaves the checkout and the edit as they were and does not bring the preview back. `i18n.spec.js` gains the card and a patch file's preview in the pseudo-locale. `patch-apply`, `pr-checkout`, `watch-during-changes`, `review-changes`, `store-persistence`, `gutenberg-site` and `terminal` walk the same flows as before, now through the tabs and the dialog. What they cannot reach is how it looks, a real pull request from GitHub, and an install that takes minutes: **Starting state:** a Core site that has finished setup, with a ticket linked. 1. In "Apply a patch or PR", type a pull request's number and click "Apply PR". A dialog opens, titled "Apply PR #…", with the files it changes. Click "Cancel": the dialog closes and nothing has changed. 2. Click the "Diff" tab, then "Choose a .diff or .patch file…", and choose a patch. The dialog is titled for the file. Click "Apply and rebuild": the dialog closes and the card shows the steps, then a green notice with "Revert this patch". 3. Click the card's header. It does not fold while the notice is there. Click "Revert this patch", and once it is gone click the header again: it folds. **What must not have happened:** - A patch that does not apply leaves no dialog in front of the error. - Nothing is written to the checkout before "Apply and rebuild" is pressed. ## Risks and limitations - **A failed apply cannot be tried again from the same preview.** The patch is chosen again; a pull request that was typed is typed again, since the field is cleared when it is read. - **Much of what the card says is not translatable yet**: the sentences it passes through from `pr-checkout.cjs`, `applied-layer.cjs`, `apply-conflict.cjs`, `watch-activity.cjs` and `update-plan.cjs`. Two notices join two of those sentences with a space. The translation journey covers the card with nothing applied and a patch file's preview. - **In a breakdown, why a change did not fit is on its own line**, lowercase and without a full stop: it was written to follow "near … ·". - **While the question about loose edits is up, the spoken "next step" still says to apply or cancel**, though neither button is on screen. And the next-action ring is not drawn round the dialog, which is in front of everything already. - **Linking a ticket while that question is up clears it, and the preview comes back.** Only "Cancel" drops it. - **A preview that arrives while another dialog is open on the same site**, the review or the create-site dialog, opens on top of it. - **After a pull request is checked out the card is gone, and focus with it.** The focus fix holds for a patch file, and for a pull request while its apply runs. - **During an update, an install or a build the dialog's button is held with no stated reason**, as the old one was. The reason is given when something holds the terminal. - **The card's heading is inside its header's button**, as the design system's collapsible card has it. The card is a region named by that heading; moving by headings may not find it. - **Whether the card is folded, and its tab, are not remembered across restarts.** That belongs with the settings (#559). - **The guide pages still show the old card.** #561. - **About 1,400 lines added, 560 of them tests**, against about 410 removed. One pull request, because the card, its preview and the pull request's banner are one block of the site's view and one flow. - **The manual steps above were not run by a person.** I looked at screenshots of the card with nothing applied, on each tab, of the dialog for a patch file and for a pull request, of the steps, of an applied patch, of a failure with its breakdown, and of a checked-out pull request. - **Checked on macOS only**, with the journeys this touches also run in a window resized to the Windows runner's 1024×728. Windows itself is CI's. - **Pre-PR review, two passes: everything marked to fix here was fixed, five findings, two of medium weight; the follow-ups left are in this list.** The medium two were a preview that could open over another site, and an apply button that could do nothing with its reason out of sight. ## Related Part of #557 (item 4 of its nine) and #542. Fifth slice on `redesign/wpds`, after #590, #594, #595 and #596. --- <details> <summary>Design decisions and alternatives considered</summary> **The patch file is chosen with a button, not the prototype's file input.** A page is not told where a file is, and the app reads the patch in the main process. The button opens the system's own dialog, as before, under the "Diff" tab. **"Apply PR" and "Apply and rebuild" keep their words on Gutenberg**, where the prototype says "Check out PR" and "Check out and rebuild". The app's sentences about a pull request ("PR #7 is applied", "Revert this PR") come from modules this PR passes through; changing two buttons alone would leave the two vocabularies side by side. **The description is the prototype's**, "Test changes in this checkout. Your own changes are preserved." The old sentence, that a pull request is checked out with its author's commits and a patch file is applied as a removable layer, is the line of help under each tab. **A failure drops the preview rather than showing the failure in the dialog.** A failure can carry a breakdown by file and place, and up to four ways on. That is the card's to hold, and it was already drawn there. **The steps stay on the card.** The prototype's dialog shows a spinner on its button until the apply is done. An apply here can install and build, for minutes, and the card's list says which step it is on while the page stays usable. **Notices are told to say nothing by themselves.** The design system's notice reads its own content out, and #596 found that reading can fail silently. The ones here that were alerts or status regions keep those roles, which announce them; the two that said nothing before, the applied patch and the checked-out pull request, still say nothing. **`pr-state.cjs` is gone.** Its last use was the old pill in the preview; the badge's rule lives with the work-item card's since #596. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> Run as the repository's review procedure, with the judgement pass in a fresh context that took no part in the change. First pass on `ceba525` against `d5bbd47`; second pass on the fix-up `cab185f`; the head, `df655d2`, applies what the second pass asked for. **First pass: four findings to fix here, six follow-ups; two medium, none high.** It compared the removed block with the new components line by line and found every line of the applied patch's and the checked-out pull request's banners, the failure's buttons and their gates, and the alert and status roles carried over. It traced every reader of the preview after a failed apply and found none that needed it, and no path on which the dialog goes without an error or a question being shown. Fixed: - **A preview could open over another site** (medium). Every site's view renders its own dialog, and a dialog is drawn over the whole window: one read while the contributor had gone elsewhere opened there, with a button that applied to the site out of view. A preview is shown by its own site. - **"Apply and rebuild" could do nothing, with the reason behind the dialog** (medium). The refusal while something holds the terminal is older than the dialog, and was a line on the same page. The button is held and says why. - **Two decisions this change made were in the components**: when the dialog shows and when the card is open. They are in `apply-card.cjs`, with unit tests. - **Translation**: "Near" and the line it introduces are one sentence a translation can reorder; "Diff" says to translators that it is a noun; a string that was extracted and never shown is gone. Follow-ups done here too: focus goes to the card's header after an apply, where it fell to the top of the window; a place in a breakdown is keyed by where it comes; a weak assertion was moved and a helper widened; and "Cancel" on the question about loose edits drops the preview, where it would have brought the dialog back. **Second pass: one finding to fix here, five follow-ups; all low.** It read base-ui's handling of where focus goes and confirmed it always lands on the card's header and never on a field, that the reason and the terminal's lock are written together, and that the three new journeys read their absences after the page has rendered. Fixed: - **The reason named the terminal as where a command was running**, and not everything that holds the terminal prints there. It says something else is running on the site, and how to stop it. Follow-ups done here too: a dialog left without applying gives focus back to its opener whatever was pressed in it; the two-site journey waits for a positive sign before it reads that no dialog is shown; the helper that finds a way to a patch file no longer matches two unrelated buttons; an assertion that could not fail is gone. Left as follow-ups, all in Risks above: the breakdown's wording; the spoken next step and the ring during the question; linking a ticket during the question; a preview over another dialog of the same site; focus after a pull request's checkout; no stated reason during an update, install or build; and retrying from the same preview. **Checks, on macOS, on the head of this branch:** | Check | Result | | --- | --- | | `npx eslint . --max-warnings=0` | clean | | `npm test` | 1827 pass, 2 skipped | | `npm run test:electron` | 1828 pass, 1 skipped | | `npm run test:e2e` (journeys) | 84 of 84 | | `npm run test:e2e:packaged` | 15 of 15, on an unsigned `npm run pack:dir` build of the head | | Nine journey files that reach the card, in a 1024×728 window, the Windows runner's | 34 of 34 | **The tests' claims were audited by breaking each one** in the source and running the test that should notice. 27 breaks, each run on the head, and all 27 turn a journey red on a line that states the claim. | Break | Caught by | | --- | --- | | A pull request can be asked for with nothing typed | `apply-card` | | The preview is not named for its patch | `apply-card` | | The preview does not say how much the patch changes | `apply-card` | | The preview does not list the files | `apply-card` | | Leaving the preview does not drop it | `apply-card` | | The card forgets the tab it was left on | `apply-card` | | The card cannot be folded | `apply-card` | | A folded card stays folded over a failure | `apply-card` | | A failed patch keeps its preview | `apply-card` | | The preview does not say whose work the patch lands on | `apply-card`, `patch-apply` | | A failed patch does not say the checkout was left alone | `apply-card`, `patch-apply` | | A failure cannot be dismissed | `apply-card` | | A preview opens over whichever site is on screen | `apply-card` | | The dialog's button is not held while something holds the terminal | `apply-card` | | Saying no to the question leaves the preview | `apply-card` | | Focus is left on nothing when the dialog closes on an apply | `apply-card` | | The preview is in front of the question about loose edits | `pr-checkout` | | Reverting a pull request does nothing | `pr-checkout` | | The failure is not an alert | `patch-apply` | | Reverting a patch does nothing | `patch-apply` | | A Gutenberg site is offered a patch file | `gutenberg-site` | | The pull request field is not held during a build | `terminal` | | The card's title is not translatable | `i18n` | | A tab's help is not translatable | `i18n` | | The preview's button is not translatable | `i18n` | | The preview's way out is not translatable | `i18n` | | The preview's sentence is not translatable | `i18n` | The audit also changed the code once: the first version of the focus fix did nothing, which the journey written for it showed, and it was redone through the dialog's own option for it. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
## Why #542 replaces the app's interface with the one in the [prototype](https://github.com/jameskoster/wordpress-contributor-toolkit-prototype), drawn with the WordPress Design System. It was built in slices on `redesign/wpds`, so that trunk, which releases are cut from, never held a half-redesigned window. The window is whole now: the shell, the page header and details, every card and dialog, and the tray. This pull request is how the branch reaches trunk. It lands before the last three sub-issues, on purpose. The settings dialog (#559) and dark mode (#560) add to a finished window and do not need a branch to hide in, and the guide (#561) is easier to rewrite against what is on trunk. They follow as pull requests against trunk. ## What changes Everything `redesign/wpds` holds; this pull request adds nothing of its own. Each slice was reviewed in its own pull request against that branch, and says there how to test it and what it leaves out. - **The shell** (#555): a sites list, the page area and a footer, in a larger window. #590 - **The page header and the details** (#556): the site's menu in the header, and what the checkout is in a sidebar that can be put away. #594 - **The site's page** (#557): the server and the build watch in the header and the details (#595); the ticket card, the apply card with a preview dialog, and the list of a site's tickets (#596, #597, #599); the create-site, rename and delete dialogs (#600, #601); confirmations as notices in the window's corner (#602); the page's notices, update card and setup checklist (#605); the terminal, logs, mail and the review dialogs painted with the design system's tokens (#606, #608); and lint rules that keep it so (#609, #610). - **The tray** (#558): along the bottom of the window, holding the terminal, the logs or the mail, with a count of unseen debug.log lines on the footer (#612, #613, #614, #615). - Along the way: #611 (the site menu's update item says why it is held), and three that tidy up before landing: #616 (a journey's race with the page's clock), #617 (comments the finished redesign made untrue) and #618 (the server's button by its present name in `AGENTS.md`). **What a contributor will notice first:** the terminal, the logs and the mail are no longer on the page but behind three buttons at the foot of the window; the terminal is dark text on light; the server and the build watch are started from the header; a patch is previewed in a dialog before it is applied; and what worked is said in the window's corner. **Not in this pull request:** - The settings dialog (#559). Until it, nothing about how the window is arranged is remembered across restarts: the details, the tray's height, the apply card's fold. - Dark mode (#560). The terminal reads its colours once, when it is made, and will need to follow the theme. - The user guide and its screenshots (#561), which describe the old interface. They are published from release tags, so they are not wrong until a release ships this; #561 is next, ahead of #559 and #560, for that reason. - How a mail is drawn, and how the window handles a link. Both are unchanged and tracked separately. ## How to test this Platforms: macOS and Windows, both. Buildkite builds signed artifacts for this pull request; use the build for its current head, and check that it is: a push to the branch makes earlier builds stale. The suite, from the repository root, after `npm ci`: ``` npm run lint npm test npm run test:e2e ``` 113 journeys walk every slice on macOS and Windows on each push. What they do not do is run a real site: a clone of `wordpress-develop`, an install, a build and a server are stood in for. The one test that does, `real-setup.yml`, is run by hand and was run on this branch at `548c020` (https://github.com/WordPress/contributor-toolkit/actions/runs/37298667056): a Core site on macOS and on Windows and a Gutenberg site on macOS were created, installed, built and served, and passed. **The Gutenberg site on Windows did not finish.** Its first attempt was cancelled after 74 minutes inside the journey, a minute short of the limit that would have failed it with evidence, so it says nothing about where it was; a second attempt was still running when this was written and its result is not in this description. The commits since `548c020` change a journey, comments and prose. **A pass by hand, on a real machine** (any platform; Apple Silicon for the server, which no test on a pull request starts for real). It follows the order a contributor meets things in: 1. Open the app with no sites. **Create new site**, a Core site, in a folder with a space in its path. The setup checklist runs its steps; the terminal's button at the foot of the window shows what each is doing. 2. Let the checklist's last step start the server, and finish WordPress's own setup in the browser. Back in the app the header says "Server running" and "Build watching". Stop the server and start it again: from **Server running** in the header, or with **Stop** and **Start** beside "Server" in the details. 3. Press the three buttons at the left of the footer in turn. Terminal: type `help`. Logs: the server's lines, the build watch's, and Debug.log. Email: where SMTP is listening. Drag the tray's top edge. 4. Link a ticket by its number. The card shows the ticket and its pull requests and patches. 5. Apply one of them. The preview dialog names the files; **Apply and rebuild** runs the steps on the card. 6. Edit a file under `src/`, then **Review & submit changes**. The diff is the edit and nothing else. 7. Create a second site, or open one from before, and move between the two from the sites list. Each has its own terminal, logs and mail. 8. **Update to latest trunk** from the site's menu. **What must not have happened:** work lost in a link, an apply, a switch or an update that did not say so; `node_modules` rebuilt where nothing changed; a failure said only in a tray that stayed closed; one site's output in another's tray; anything on the page under the tray. **What cannot be tested by hand, and what was not:** none of the slices' manual steps were run by a person. I looked at screenshots and drove the app through the journeys; everything local was on macOS, with Windows left to CI. This pass is the first one by hand. ## Risks and limitations - **It is one commit on trunk**, since the repository allows only squash merges. The slices' pull requests are its record. `redesign/wpds` should not be branched from after this: it will have diverged from trunk. - **Reviewed in slices, not as a whole.** Twenty-two pull requests, all but a two-word change to `AGENTS.md` reviewed by a separate reviewer context before they were opened, most in two passes; see below. The branch was then swept once for what falls between slices, which found comments and one unread field (#617) and the three points that follow. - **Three questions and two failures still use the system's own dialogs**: the discards ask through its confirm box, and a path that could not be copied and a server started before there is a build are said in its alert. #557 meant to replace them and did so for deletions only. - **Some dialogs are repainted, not rebuilt.** The review dialog and its destinations, the mail dialog and the question an update asks about loose edits have the design system's colours and type on the old component library's parts. - **A real Gutenberg setup on Windows is not shown to pass on this branch.** See "How to test this". It passed on trunk before the redesign, in 35 minutes; here it was past 46 on its second attempt. Whether that is the runner or the branch is not known. - **The guide describes the old interface** until #561. Do not cut a release before it. - **The bundle roughly doubled** (#590): the sites list brings `DataViews` and what it needs. Launch time was not measured. - **Less is always in view.** A build's output, a new line in debug.log and a caught mail are in a tray that may be closed. A failure opens the terminal, or the logs if nothing else is open; unseen debug.log lines are counted on the footer; a mail that arrives shows nowhere until its tray is opened, which was decided and is not an oversight. - **Translation is about half done.** The shell, the cards, the tray and most dialogs go through the translator. Roughly 200 strings in components and hooks do not yet (the review dialog and its destinations, the setup checklist, the logs, most of what the hooks say), nor about 150 in the modules they take sentences from. Strings that are new are in English in the shipped translations until they are translated. - **Focus is lost in a few places**: when the app swaps what the tray holds from under it, when a notice with a focused button goes, when a row's own action removes the row. - **Some failures are still worded by the main process** ("Error invoking remote method…"): a failed rename, a failed setup. - **Things carried over, not made by this:** a start that throws leaves the server's button dead until a restart; erasing typed input in the terminal breaks once the line wraps; a line written to debug.log while it is being attached to can be shown twice. - **#571 and #603 will conflict with trunk after this**, in the mail's journey and helpers and in the dev server's hook. - Each slice's own list has more, and says which of its limits were chosen. ## Related Closes #557. Closes #558. Part of #542, which stays open for #559, #560 and #561. #555 and #556 are closed already. --- <details> <summary>Design decisions and alternatives considered</summary> - **An integration branch, and one landing.** A window drawn half by the old view and half by the new one could not be released, and the slices were too many to hold back one by one behind a flag. Fixes and changes with no visible effect went to trunk first and were merged across, so this diff is the redesign and little else. - **Landing before #559 to #561.** The plan was to land when all seven sub-issues were in the branch. The remaining three do not change what is there: two add to it and one documents it. Holding a finished window on a branch for them only lengthens the time trunk and the branch can drift apart. - **What the prototype lacks is kept and redrawn**, and what it has somewhere else moves to where it puts it. Its labels are adopted. Where a slice departs from it, the slice says so. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **No review of the whole diff was run. The record is the slices': twenty-two pull requests into `redesign/wpds`, each but one reviewed before it was opened by a separate, read-only reviewer context following `.github/instructions/code-review.instructions.md`, most in two passes (the change, then its fix-up), with the counts and the findings in each pull request's own review outcome. The one is #618, two words in a line of prose.** - **Review:** completed, per slice — #590, #594 to #602, #605, #606, #608 to #617 (no review was run for #618); everything marked to fix in a slice was fixed in that slice, and what was left is in its Risks. - **Review:** completed, for leftovers only — a separate agent context, read-only; `redesign/wpds` at `548c020` against trunk; not a review of correctness, which the slices had: a search for dead CSS and code, comments that waited for a slice that has landed, what is left of the old component library and of the system's dialogs, strings not yet translatable, and repository docs that name the branch. Nothing found in the stylesheet, the dependencies or the docs; thirteen comments and one unread field, fixed in #617; the rest is in Risks above. - **CodeRabbit:** not run. Its check reads "Review skipped: automatic reviews are disabled" on this pull request and on the slices, so the slices' pre-PR reviews are the reviews of record, each for the head it names. - **Since review:** nothing was added to the branch outside a reviewed slice, apart from three merges of trunk (`da19ac6`, `3344172`, `23d46e7`). </details> <details> <summary>Implementation notes</summary> - `git diff origin/trunk...origin/redesign/wpds`: 148 files, about 14,800 lines added and 3,000 removed; about 6,300 of the added lines are tests. - The site's view, which was one component of five thousand lines, is spread over the four regions the layout has: the header, the cards, the details and the tray. Decisions are in `src/renderer/*.cjs` with unit tests; components draw them. - New for the suite: journeys per region under `tests/e2e/journeys/`, lint rules of the repository's own (`eslint-rules/`), and Stylelint over the window's stylesheet. - The main process changed in one place for the redesign's sake: the debug.log tail says which lines the file already held (#614). </details> <details> <summary>Screenshots or recording</summary> Not attached: I have no way to upload images from the command line. The prototype shows the design; the signed build shows the app. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
## Why The prototype draws each site's server dot at the left of its name in the sites list, so which sites are running can be seen without opening each. The app had the dot only in the open site's header: a contributor with three sites had to open each one to learn which server was up. The dot also sat 1.5px above the lowercase letters beside it, in the header as in the list. ## What changes Every entry in the sites list now wears the header's own dot before its name: grey while the server is stopped, green running, amber starting, red for a server that went by itself or could not start, with the reason as the dot's tooltip. The attention dot for a stale or half-updated site (#94) stays, drawn after it. A server lives in its site's view, open or not, so the view is the one thing that knows. Each `SiteRow` reports its server's state to the window as it changes and takes the report back when it unmounts; the window keeps the reports by path and hands them to `sitesListRows`. What a report says, what it does to the reports and what the dot shows are all decided in `src/renderer/sites-list.cjs`, where the suite can reach them. For a screen reader the dot's words follow the name, as the attention dot's do, and only when the dot is not the grey every site starts with: said after every name, "Server stopped" would be the thing heard most and needed least. So an entry is named `my-site` while stopped and `my-site (Server running)` while running. Both dots, and the header's, are now centred on the text's x-height with `vertical-align: middle`, which centres an inline box on the parent font's x-height by definition and so holds on Windows with a different font. A flex item, which the dots were, is centred on the line box instead, 1.5px higher. Deliberately not in it: a dot for the build watch in the list, and any control on the list's dot. The dot reports; the header and the details act. ## How to test this Platforms: any. The x-height alignment is worth a look on Windows too, where the font differs: the rule is font-agnostic, but only a look confirms it. The first journey in `tests/e2e/journeys/dev-server.spec.js` walks the reported path and reads the list's dot at each turn: starting, running, stopped and crashed, with the entry's name. It is red without the change. From the repository root: ```bash npx playwright test --project=journeys tests/e2e/journeys/dev-server.spec.js ``` What the journey cannot reach is a real server, since journeys stub the handler that starts one, and what the dots look like beside real text: **Starting state:** two sites, both set up, neither server running. The Buildkite artifact for the head commit, on macOS or Windows. 1. Look at the sites list. Expected: every entry has a grey dot at the left of its name, on the same line as the lowercase letters, and a stale site has its amber dot after it. 2. On the open site, open the server's menu in the page header and choose **Start development server**. Expected: the entry's dot turns amber while the server starts and green once the site opens in the browser. The header's dot sits on its words' x-height, as the list's does. 3. Click the other site in the list. Expected: its own dot is still grey, and the first site's dot is still green while you look at the second. 4. Go back to the first site and choose **Stop development server**. Expected: its dot is grey again. 5. Narrow the window until the header's menus show only their dots. Expected: one gap between a dot and its chevron, not two. **What must not have happened:** no site's entry has lost its name or project line; a long name still ends in an ellipsis; the attention dot of a stale site is still there, after the server dot; nothing else in the header moved. ## Risks and limitations - A running stale site shows two dots side by side, a green and an amber. The prototype has no attention dot to reconcile with, so this keeps both rather than folding two meanings into one dot. - A crashed server's words are the whole sentence from the details, so that entry's accessible name is long. It says what the red dot means, which "Server stopped" would not. - The guide's sidebar screenshots were not retaken. - Pre-PR review: 2 [fix here] · 0 [follow-up], both fixed in `6229fb6`. ## Related Follow-up to #590 (the sites list) and #595 (the server and the watch in the header). --- <details> <summary>Design decisions and alternatives considered</summary> - **The view reports up, rather than the main process broadcasting.** "Starting" is a renderer state: it covers the wait for the build watch before the server is even asked for. Only the view knows it, so a main-process list of running servers would have drawn the wrong colour for the longest phase. - **One effect, with a cleanup that takes the report back.** Two effects, one to report and one to clean up on unmount, would have lost the entry if the callback's identity ever changed; one effect's cleanup and re-run land in the same commit, so the list sees a single change and no grey dot in between. - **No words for a stopped server.** The prototype announces "Server stopped" before every name. Here the grey dot is the default and says nothing, as the attention dot says nothing when there is nothing to report. - **`vertical-align: middle` over a pixel offset.** The dot was measured 1.5px high against the ink of the letters, in a screenshot of the running app. A `margin-top` would have fixed that for this font and this size; `vertical-align: middle` is the CSS definition of "centred on the x-height", so it follows the font. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 2 [fix here] · 0 [follow-up] — both fixed. - 🟡 Architecture, `[fix here]`: the words a view reports of its server, and what a report does to the window's reports, were decided inline in `index.jsx`, where the suite cannot reach them. Fixed: `serverReport` and `withServerReport` in `src/renderer/sites-list.cjs`, with unit tests. - 🔵 Layout, `[fix here]`: the space between a process's dot and its words was the dot's margin, which stayed when the words gave way to the dot in a short header and doubled the gap before the chevron. Fixed: the margin is the words' own and goes with them. - **Review:** completed — a separate agent context running `.github/instructions/code-review.instructions.md`; reviewed head `aad1b72` / base `0cd70e4` (origin/trunk); `npm test` 2047 pass, `eslint src tests scripts` and Stylelint clean (a repo-wide `eslint .` fails only on stale checkouts under `.claude/worktrees`, which are not part of the branch); outcome above. - **Since review:** `aad1b72 → 6229fb6` checked; the one commit applies the two findings and nothing else. `npm test` 2049 pass, lint clean, and the dev-server, site-header, auto-start and logs journeys pass on the new head. - **CodeRabbit:** not yet requested. </details> <details> <summary>Implementation notes</summary> - `src/renderer/sites-list.cjs`: `serverDot` (what the row draws; unknown or missing reports are offline), `serverReport` (what a view reports from `serverProcess`), `withServerReport` (the reducer). `sitesListRows` takes `servers` by path. - `src/renderer/index.jsx`: `serverReports` state and the `onServerStatus` callback in `App`; the reporting effect in `SiteRow`. - `src/renderer/components/sites-sidebar.jsx`: draws `ProcessStatus` before the name and the hidden words after it. `ProcessStatus` in `site-header-actions.jsx` gained an optional `title`. - `src/renderer/shell.css`: the name span is one nowrap line with the ellipsis, the dots inline boxes; the header's dot and words are one inline span inside the button. - Journeys: `dev-server.spec.js` reads the list's dot at each state; `logs.spec.js` and `auto-start.spec.js` name a running site's entry with its dot's words, since the accessible name carries them. - Measured alignment, in the ink of a window screenshot at 1x: list dot centre 93.5 against the letters' 95.5 before, 95.5 against 95.5 after; header dot 32.5 against 34 before, 33.5 against 34 after. </details> <details> <summary>Screenshots or recording</summary> The list before the alignment fix (dots centred on the line box) and after (dots on the x-height), and the header's menu, are to be attached from the measuring run; `gh` cannot upload images, so they go in from the web UI. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Why
The app's interface was built from design-system components without a design review, and #542 replaces it with the one in the prototype. #555 is the first piece: the shell everything else sits in. Today that is a dark sidebar of buttons beside one scrolling column in a 1000×700 window; the prototype's is a "My sites" list beside the open site's page, with a footer under it.
This PR targets
redesign/wpds, nottrunk. The redesign is built on that branch and reaches trunk in one piece at the end, so trunk never holds a half-redesigned window.What changes
DataViewslist inside anadmin-uiPage, as in the prototype. The open site's page is aPagewhose header carries the site's name, its project and the button that hides the list. With no site there is no list: a centred "No sites" and one button.sites-list.cjs: its name, the line under it, what its dot reports, and which site a selection opens.@wordpress/admin-uiand@wordpress/dataviews. They and what they bring in are bundled by esbuild, so they are excluded from the package, like the renderer-only packages before them.A bug this would have introduced, found and fixed here:
DataViewsbuilds element ids from an item's id and pointsaria-labelledbyat them, and that attribute is split on spaces. With the site's path as the id, a site in a folder whose path has a space, which is any Windows user with two names, lost its accessible name. A row's id is now the path encoded.shell.spec.jshas a journey for it that was red before the fix.Deliberately not in this PR, because each needs per-site state that the list and the header cannot reach yet, or a region that does not exist yet:
How to test this
Platforms: any. Use the Buildkite build for the current head of this branch.
The journeys carry this. From the repository root,
npm run test:e2eruns them all;tests/e2e/journeys/shell.spec.jsis new and is the one that walks the shell:i18n.spec.js);What they cannot reach is how it looks:
Starting state: the app with two or more sites, one of them Gutenberg.
What must not have happened:
Risks and limitations
DataViewsbrings@wordpress/data,@wordpress/datewithmoment-timezone, and@base-ui/react. It is read from disk once at launch; I did not measure launch time.DataViews' own strings are not translated. The app loads only its own catalogue. The list as configured here shows none of them, but anything a later slice turns on will be in English.buttonan 8px right margin. The shell's own buttons are taken out of it; the rule itself stays until the old site view, whose rows of buttons lean on it, is rebuilt (Server section and process menus, ticket card, apply patch and others #557).empty-state,create-site-modal,site-menu,stale-site-notice); the images were not committed. Retake all screenshots and update guide text #561./x/fixand/x/fix-2), andDataViewsmatches some of its element ids by prefix. Nothing that reads those matches is on screen until rows have an actions menu, so the fix, a terminator on the id, belongs to Page header with the site menu and the collapsible details sidebar #556.DataViewsstylesheet is loaded, as for the components already.h1headings in one window: "My sites" and the open site's name, as in the prototype.npm run pack:dirstops at signing in a checkout without the provisioning profile, after the app is assembled.macOS signing removes its temporary key when Buildkite terminates the shell, which waits five seconds for a shell to exit. Nothing in this PR touches signing.Related
Part of #555 and #542. First PR on the
redesign/wpdsintegration branch.Design decisions and alternatives considered
DataViewsfor a list of a handful of sites. It is heavy for what the list does today, and a dozen lines of markup would draw the same rows. It is what the prototype uses, #542 asks for the design system's own components, and the rows gain actions in #556, which is whatDataViewsis for.The list hides; it does not collapse to a rail of initials. The prototype's hides too, and the old 56px rail of single letters told two sites named alike apart by nothing.
"Deleting site…" replaces the project in the row, and the entry's name ends with "(Deleting)". The prototype has no such state. The old entry was a disabled button; a
DataViewsrow cannot be disabled, sositeToOpenrefuses a site that is being deleted, and the name says why pressing it does nothing.The footer is there with no site too. The prototype's first screen has none. Feedback is about the app, and a newcomer who cannot create their first site is the person most likely to want to say so.
The three notices with no place in the prototype stay at the top of the page area: a setup in flight, the Playground web server where a build ships one, and a ticket that arrived from a link when there is no site to put it in.
#rootis not made a stacking context, which the prototype does. The toast region has to rise over a dialog, which is drawn onbody, so that what was done inside one is still confirmed.The stylesheet is a file,
shell.css, imported by the entry point and bundled intoindex.css. It is excluded from the package by name, like the.jsxsources it is bundled with.Review outcome (required — see AGENTS.md)
Run as the repository's review procedure, with the judgement pass in a fresh context that took no part in the change. First pass on
eeef3ccagainste1b17dd; second pass on the fix-up81fbeb4; the head,63723c4, applies what the second pass asked for.First pass: sixteen findings, two of medium weight, none high. It compared the old
Appwith the new one prop by prop and found everySiteRowprop, the selection effect and the hidden views carried over, and one real loss, the page's maximum width.Fixed:
app.asar(medium), about 18 MB. They are excluded like the renderer-only packages before them, and the re-packaged archive holds none of them.catcharound reading the screen increateWindow. Removed: every caller runs after the app is ready. The unit tests' Electron had noscreenand had been passing because of that catch; it has one now, with a test that the window is given what it reports.button { margin-right: 8px }reached every new button, so a row's click area stopped short of its edge. The shell's own buttons are taken out of it.TESTING.mdasks for, "Deleting site…" read from the page and not from its row, two helpers only one journey uses sitting inui.cjs, and a class selector where text would do.Second pass: four findings, all low, two to fix here. Fixed: three of the list's strings were not translatable (what a dot reports, the brackets around it, "Deleting"), in an area whose other strings are; they are wrapped, and
i18n.spec.jsgains a journey for the list. And nothing asserted what the hide button'saria-controlspoints at;shell.spec.jsnow does.Left as follow-ups, all in Risks above or here: row ids that can be prefixes of each other; the window sized from the primary display; the left-to-right
DataViewsstylesheet only; the design system's body type reaching the old view and dialogs; twoh1headings;ui.cjsexporting two more part-of-the-screen locators than the six its header counts, which predates this change;i18n.spec.jsfinding the feedback popover by its class, which it needs a container for; and two small unit-test gaps (rowIdbeing prefix-free, which goes with the first follow-up, andsitesListRowsgivendeleting: null, which the app never does).Checks, on macOS, on the head of this branch:
npx eslint . --max-warnings=0npm testnpm run test:electronnpm run test:e2e(journeys)npm run test:e2e:packagednpm run pack:dirbuild of81fbeb4; the head differs from it in the renderer's strings and in testsapp.asarof that buildshell.cssThe tests' claims were audited by breaking each one in the source and running the test that should notice. 32 breaks, all 32 turn a test red on the line that states the claim.
shellshellshellshellshellshellshellshellshellshellshellshellshellshellshellshellshellengineengineengineenginecreate-sitei18ni18nshellshellipc-wiring(unit)i18ni18ni18nshellshell🤖 Generated with Claude Code