Repository navigation
Put the site's menu in the page header and its details in a sidebar - #594
Merged
Merged
Conversation
The second slice of the redesign (#542, #556), following the prototype. The page's header gains the open site's actions: a button that shows and hides the site's details, and the site's menu, which now holds everything done to a site as a whole. Renaming and opening the folder join copying the path, showing it in the file manager, updating to the latest trunk and deleting. The window leaves a slot in the header and the open site's view fills it, so the buttons are drawn where the design puts them and still call that site's own functions. What the menu offers is decided in site-menu.cjs. The site's view becomes two columns: its cards, and its details beside them. The details are what the old view's 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 the button that copies the path, and what it is a checkout of. Putting them away is the window's choice, kept as another site is opened, and they go under the cards where the page is too narrow for two columns. 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 toasts are therefore drawn on `body` as well, so that they still rise over a dialog. And the terminal scrolls sideways: at the default window size, beside the details, its column is narrower than its eighty columns of text. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The applications a folder can be opened in move into a menu of their own, "Open in", under the site's menu. Detection answers after the menu is on screen, and in the menu's own list the rows it adds pushed "Delete site" down under a pointer aimed at something else. The menu's own list is now the same whatever detection finds. A refused open is brought into view. Its notice is at the top of the site's cards, and the header's menu can be used from anywhere down the page: said out of sight, a refusal looked like a button that did nothing. The details stay in view while the cards scroll: the cell they sit in was only as tall as they are, so the sticky rule had nowhere to hold them. What they say is decided in site-details.cjs, with unit tests, and the component draws the rows. A copy from the menu says so in a toast and leaves the details' button alone, so a screen reader hears it once. In the journeys: the header's says what its stand-ins leave out of the main process, asserts the four things above, and reads the menu without needing a parent item while the applications are open; the translation journey scans the applications once the one it expects has arrived. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The second review pass. Under "Open in", the way to choose an application is now first, the applications detection found follow it, and the row that says detection is still running is last: the rows that arrive and go are under the one that is always there, so it no longer moves under a pointer aimed at it. A unit test walks the lists before and after an answer. A row is keyed by what it is, so it keeps the focus on it when the list around it changes. A refused open is brought into view once, when it is said, and not again each time its site is opened while it is still there. The details' button no longer announces itself: it was a live region, so a screen reader heard "Copied" and then, a moment later, "Copy". A status region beside it holds the word only while it is true. 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 |
The header journey claimed that a refusal still on the page does not take the page back to itself when its site is opened again. It cannot show that. Opening a site moves the page anyway: the next-action cue centres the next step each time a site becomes the open one, and has done since #252. On macOS that scroll is animated and the step read the page before it had moved; on the Windows runner it had finished, and the step was red. The step is removed, and the comment that made the same claim says whose the page's position is when a site is opened. The journey also puts the sites list away first, so that on the smallest screen the suite runs on the page still has two columns and "the details stay in view" is about the details staying, not about where one column puts them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
zaerl
added a commit
that referenced
this pull request
Oct 2, 2026
#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>
zaerl
added a commit
that referenced
this pull request
Oct 2, 2026
## 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>
zaerl
added a commit
that referenced
this pull request
Oct 2, 2026
…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>
zaerl
added a commit
that referenced
this pull request
Oct 5, 2026
…eld nothing reads (#617) ## Why The redesign was built as about twenty slices on `redesign/wpds`, each reviewed on its own. Before the branch is merged to trunk (#591) I had a separate reviewer sweep it for what falls between slices: what one left "until a later slice" and the later slice did not clean up. It found no dead CSS, no unused dependency and no repository doc that names the branch. It found thirteen comments the finished redesign made untrue, and one field nothing reads. ## What changes No behaviour changes. **Comments corrected**, in ten files: - `src/renderer/index.jsx`: the note that the system's alert and confirm are "replaced … as its part of the window is redrawn (#557)" now says what is left of them (three discards that ask through `window.confirm`, two failures said in an alert) and that moving them is a change of its own; two notes that said "yet" of things that are settled. - `src/renderer/hooks/use-dev-server.jsx`: "the one button" that starts the server is the header's menu, the details' button and the setup checklist's; a failed server is said on the page as well as in the logs; the button's old names. - `src/project-type.cjs`: where a project's tag is drawn, and that nothing upper-cases it any more. - `tests/e2e/journeys/build-watch.spec.js`: its header called it a bug that the watch's button cannot select the watch's tab. It can, since the logs' tabs follow the app's choice (#613), and the same file asserts it. - `tests/e2e/journeys/tray.spec.js`, `logs.spec.js`: the tray's two buttons are three; the logs are in the tray. - `TESTING.md` and `scripts/screenshots/shots.cjs`: the server's button by its present name; the instructions for one screenshot no longer describe the old three buttons. **One field removed.** `trunkAgeInfo` in `src/renderer/update-plan.cjs` returned `label`, the sentence "trunk as of …" that the old header drew. Nothing has read it since the header was replaced (#594). Its unit tests now pin the date it is built from. **Deliberately not here:** - The three questions and two alerts that still use the system's dialogs. Replacing them changes what a contributor is shown. - The roughly 350 strings that are not yet translatable, and the dialogs still drawn with the old component library. Both are recorded in #591. - One stale comment in `src/renderer/components/email-modal.jsx`, a file another open pull request is changing. - `AGENTS.md`, which names the server's button by its old words twice, in the note about the Electron pin. I left the project's instructions alone. ## How to test this Platforms: any. There is nothing on screen to drive: the change is comments, one unread field and its tests. From the repository root, after `npm ci`: ``` npm run lint npm test ``` That the field is unread: `git grep -n "trunkAgeInfo" src` lists its two callers, `src/renderer/index.jsx` and `src/renderer/sites-list.cjs`, and neither they nor what they hand the result to (the details, the review dialog) read `label`. **What must not have happened:** a code line changed along with a comment. The diff outside `update-plan.cjs` and its test is comments, one string shown to a person taking a screenshot, and prose. ## Risks and limitations - Review: 1 pass, 5 findings fixed here. Details below. - A comment is only as true as the day it was checked. Each reworded one was checked against the code on this branch by the reviewer, by name, and the list is below. ## Related Part of #542. Before #591. --- <details> <summary>Review outcome (required — see AGENTS.md)</summary> **5 [fix here] · 0 [follow-up]. All 5 fixed.** - **Review:** completed — a separate agent context, read-only; the change as it stood uncommitted on base `548c020` (it is `197d21a` with the fixes); the diff read line by line for anything that is not a comment, every consumer of `trunkAgeInfo` followed through the props it is handed on in, each reworded comment checked against the code it describes. - Fixed: a new assertion on the date's day of the month failed in any time zone behind UTC, where midnight UTC is the day before (removed; the suite pins no time zone); the comment about the server's controls left out the setup checklist's; three more comments in the same file still described one button, by its old names; a comment in the same block said a failed server is said "nowhere on the page", which stopped being true in #613; "the Logs panel" once more in a file already touched. - Nothing found: no code line changed outside the field; the field has no reader; `TESTING.md`'s paragraph is still one line. - **Since review:** the five fixes. `npm run lint` and `npm test` re-run after them, the unit file for the field in three time zones (`America/Los_Angeles`, `Pacific/Kiritimati`, `UTC`). Before them, on the same code but for comments and that one assertion: `npm run test:electron`, all 113 journeys and the packaged smoke. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
zaerl
added a commit
that referenced
this pull request
Oct 5, 2026
## 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
#556 is the second slice of the redesign (#542): the open site's header and its details, as the 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, nottrunk.What changes
site-menu.cjs.site-details.cjs.Two things this needed:
bodywith no z-index, and the page's header, which has one, covered them: the menu opened under the page. Add the new shell: sites list, page area and footer, in a larger window #590 had removed this so that toasts could rise over dialogs; the toasts are now drawn onbodythemselves, which keeps that.Deliberately not in this 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:e2eruns them all;tests/e2e/journeys/site-header.spec.jsis new:Renaming, updating and deleting through the menu are walked by
rename-site.spec.js,trunk-update.spec.jsandengine.spec.js, which now reach them there.i18n.spec.jsgains 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.
What must not have happened:
Risks and limitations
Related
Part of #556 and #542. Second slice on
redesign/wpds, after #590.Design decisions and alternatives considered
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
Pagedoes 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.
useIdfor 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.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
6cb7dd5against3344172; second pass on the fix-upf1420bb;538bc8aapplies 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_modulesand 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:
site-details.cjs, with unit tests.Second pass: five findings to fix here, one medium.
After CI: one journey step removed.
538bc8aadded 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.mdandmanaging-sites.mddescribe "More", "Open directory in" and the copy icon. The screenshot harness still works: itssite-menushot was run.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 off1420bb; the head differs from it in the renderer and in tests, and CI's packaged smoke passed on538bc8aon both platformsThe 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.
site-headersite-headersite-headersite-headersite-headersite-headersite-headersite-headersite-headersite-headersite-headersite-headersite-headerrename-siteenginetrunk-updatei18ni18nsite-headersite-headersite-headersite-headersite-header, andsite-menu(unit)site-headersite-headerNothing 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.
🤖 Generated with Claude Code