Repository navigation
Move the logs into the bottom tray, and bring them up for a watch or a server that fails - #613
Merged
Merged
Conversation
Second part of #558. The footer has a second button, "Toggle Logs", and the tray holds one thing at a time: the terminal or the logs, each in the other's place, for the site that is open. The logs are the three tabs they were, drawn with the design system's tabs under the tray's heading: Server, Build watch and Debug.log, with the pane taking the room the tray has and debug.log's file and its three buttons along the bottom. A pane that was following its last line is put back there when the logs come back on screen. The tabs now follow the selection the app makes. They did not before: the watch said "show my tab" when it ran a build, the old tab panel kept its own idea of which tab was open, and the two could differ. So starting the watch on a site with no build now shows the watch's tab, as it was written to. A build watch that ends by itself, or is never started because the build ahead of it failed, is said on the page with "Its last lines are in the Logs." The tray opens on the logs, on the watch's tab, when that happens, as it opens on the terminal for a failure that is printed there. A watch stopped by its button brings nothing up. The tab that was "Build watcher" is "Build watch", as the header, the details and the prototype have it, and "debug.log" is "Debug.log". Four messages that sent the reader to "the Build watcher tab" now name the logs or the header. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… a server that fails Review fix-ups for the logs' move into the tray (#558). The logs open by themselves only in a tray that is closed or already showing them. They do not replace the terminal, where someone may be typing; the terminal still takes the tray for a failure or a refusal printed there. The tab the failure is on is selected either way. A dev server that could not start, or went by itself, says so only in its tab of the logs: the logs come up on that tab. The build watch refused because a command is running now says so in the terminal as well, like every other action a running command turns away. A site whose deletion fails no longer keeps a reveal waiting for a watch the deletion ended. Two messages no longer quote the header's label, which is hidden in a narrow window and translated where the sentence is not. The screenshot script finds the renamed tab, the real-setup journey tells a watch that never started from one that went, and the logs panel takes its panes from the object it is given. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…st by the tray's rule With the logs kept out of the terminal's way, a development server that stopped by itself or could not start under an open terminal said so nowhere on screen. The server's details now say it, with where its last lines are, until the server is started again. A request for a tray made while the site is not the open one is kept by the same rule the tray follows, so the logs asked for after the terminal do not take its place there either. The sentences that told a contributor to wait for the build watch "to finish" say what to wait for. The comment about a deletion's endings says what is not covered: a process that takes longer to end than the deletion waits for. Journeys: the watch's and the server's tab is read as selected, not only present; the terminal is read as still there; a failed server's sentence is read as gone after the next run. The header journey waits for the site's status before it reads where the page is, and has the app jump instead of glide, which a slowed status read showed it racing. 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 configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The header journey asked for 40px to spare under the details with the tray at its smallest, and the runners' screens leave 16px (Windows) and 6px (macOS). It puts the tray away where that is so. Its second half read a site whose details are now the taller column, where being held and being let go look the same, so it passed with the rule broken; it now makes the same site's details too tall for the page with the tray at its largest. The logs journey expected the pane to grow by one key step. It now reads the pane's growth against the tray's own, and first that the tray is as tall as its edge says, so a failure names which of the two was off. The macOS runner measured a pane 42.5px shorter than the tray's height accounts for before the key press, which I could not reproduce. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
zaerl
added a commit
that referenced
this pull request
Oct 5, 2026
## Why Part of #558, after #613 moved the logs into the tray. The count of debug.log lines nobody has looked at is on the Debug.log tab, and the tab is on screen only while the tray shows the logs. With the tray closed, or showing the terminal, a PHP error arrived with nothing to show for it. On the page the count was always in view. ## What changes **The footer's Logs button counts them.** A number on the button's corner, for the site that is open, and the words that say what it counts ("3 unseen lines in Debug.log") as the button's description. The button's name is still "Toggle Logs". Past 99 the number reads "99+" and the words still say how many. **What counts as seen.** A line was unseen while another tab was selected. It is now unseen unless debug.log's pane is in front of someone: the Logs are what the tray shows, for the open site, with Debug.log the tab selected. A tab left selected in a closed tray is not being read. The tab's own count follows the same rule. **What the file already held is not news.** Starting the server shows what debug.log held, with the app's line under it that says where the earlier run ends, and that was counted: every start of the server on a site with history would have lit the button before anything had happened. The main process now says which lines the file held before the server started, and those are shown and not counted. Two things follow from doing that properly: - The file is watched from before the server starts, where it was watched once the server was up. A line WordPress logs while it boots is this run's, and is counted. - A debug.log that first appears while the server runs, the common case, is news from its first line, and the "earlier run" line is no longer put under it. That line was untrue there before this change too. **A tab's panel has its room from the moment its tab is selected** (its own commit). The design system's tabs keep the panel that is leaving in the document until the next frame is drawn, and for that long it shared the room with the panel coming in: a log that opened 42.5px short and then grew, and on the apply card fields two tabs tall and then one. This is what the macOS runner measured in #613's logs journey, three attempts in four. Every panel but the selected one is now out of the layout. **Not in this pull request:** - Unseen lines of a site that is not the open one. They are counted, and the button says so when the site is opened; nothing in the sites list says it before. - The words on hover. The tooltip says "Toggle Logs"; the number is drawn and its words are for a screen reader. - The user guide, whose page on the logs describes the old count: #561. ## How to test this Platforms: any. The journeys drive this on macOS and Windows. From the repository root, after `npm ci` and `npm run build:once`: ``` npx playwright test --project=journeys logs.spec apply-card.spec ``` - `logs.spec.js`, "debug.log shows what the file holds, counts what arrived unseen on its tab and on the footer…": what the file held at the start is told to the window and counted as nothing; lines written afterwards are counted on the tab and on the footer, one line and then two with the tray closed and under the terminal; another site's page shows none of it; coming back to the tab clears both. - "a debug.log that first appears while the server runs is news from its first line": counted, and no "earlier run" line under it. - "what WordPress logs while the server is starting is news…": the stand-in server is held mid-start while a line is written. Red with the file watched only once the server is up. - Both files check that one panel is taking room straight after a tab is selected, and again when the tab just left is gone back to within the same frame. I broke 17 behaviours one at a time and each turned a journey red. `tests/unit/debug-log.test.cjs` holds the decisions (when the pane is in front of someone, what a chunk adds to the count, what the button says) and `tests/unit/preload-listeners.test.cjs` the bridge's part. **Worth a look by hand** (any platform, the current head; I have not done this, the journeys stand in for the server): on a site that is set up, press **Start development server** with the tray closed. The Logs button shows no number, even if `debug.log` has lines from before. Make WordPress log something, for example by adding `error_log( 'seen?' );` on a new line after the opening `<?php` of `src/wp-login.php`, waiting for the build watch, and opening the site's login page. The button shows **1**. Open the Logs and select **Debug.log**: the number goes. **What must not have happened:** a number on the button straight after a server start; a line losing its place in the count because the tray was closed, the terminal was showing, or another site was open when it arrived; the log pane or the apply card's fields jumping in height as a tab is selected. **What cannot be tested by hand here:** what a real WordPress logs while it boots. The journey holds a stand-in server mid-start and writes the line itself. ## Risks and limitations - Review: 2 passes, 7 findings fixed here, 1 decided by the maintainer and done, 1 follow-up applied here, 4 left as follow-ups. Details below. - **The Debug.log tab's count changes too**, not only the footer: it no longer counts what the file held at the start, and it counts lines that arrive while the tab is selected in a tray nobody is looking at. - **The file is watched before the server starts.** A start that fails now empties the pane and shows the file again, where it used to leave the pane as it was. - **The "earlier run" line's fix is here and not on trunk first**, because the flag that makes it possible is introduced here. Trunk keeps the old line until the redesign lands. - A line that arrives in the instant the tray is being closed or switched by the app itself (a failure bringing the terminal up) can go uncounted. One task wide; the same instant for a tab the app selects is closed. - A line written while the file is being attached to can be shown twice, as before, and is now counted once. A file replaced in place is sent again in full and counted again. Both are in the tail's reads, on trunk as well, and flagged separately. - A third button in the footer, the mail's, would sit under a "99+". The mail's pull request places it. - `toasts.spec.js:52` failed once in a full local run with "Cannot fast-forward to the past" and passed on the next. It is the second time I have seen it and it is not from this change; flagged separately. ## Related Part of #558. Follows #613. Mail HTML rendering is unchanged and tracked separately. --- <details> <summary>Design decisions and alternatives considered</summary> - **A description, not a longer name.** The button is found by its name, by people and by the journeys, and a name that changes with every line is a button that keeps being announced. The count is state, so it is the description. - **A slot in the footer that the open site fills**, as the tray's contents and the header's actions are. The alternative, the count lifted into the window's state, redraws every site's view for every line. - **The main process says what is backlog.** The renderer could have looked for the app's marker line in the text, which ties the count to a sentence. Main knows which read is the one made as the tail starts. - **The backlog is not counted at all**, the maintainer's choice of two. Counting it, as the tab did, makes the button mean "debug.log is not empty" on most starts. - **`inert` and not the tabs' own mark for a leaving panel.** A panel gone back to before the next frame still carries that mark, and was hidden for the frame when I used it. `inert` is on every panel but the selected one and on nothing else that is a tab's panel here: the old tab panel in the mail dialog never has it, and the app's own `inert` is on ancestors. - **One rule for every set of tabs**, by role, and not one per place they are used: the mail's tabs in the tray are next. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **Pass 1: 3 [fix here] · 3 [follow-up]. Pass 2 (on the fix-up): 4 [fix here] · 3 [follow-up]. All 7 [fix here] fixed. Of the follow-ups: 1 was the maintainer's decision and is done, 1 is applied here, 4 are left.** - **Review:** completed — a separate agent context, read-only; the change as staged on base `6be1ee1`, before it could be committed (signing was unavailable; the commits are `324b00e` and `381be54`); lint and the unit tests of the module run, the journeys read and not run; 3 [fix here], 3 [follow-up]. - Fixed: the journey's tab check chose tabs by name across every site's copy in the tray; one "stays empty" check ran before the line it was about could have arrived; a line arriving as the app itself selects another tab went uncounted. - Decided by the maintainer and done: whether what the file held at the start counts (it does not). - Applied here though marked follow-up: the apply card's tabs have the same frame of two panels (one rule for both). - Deferred to #561: the user guide. - **Review:** completed — a second separate agent context, read-only; the whole working change on base `6be1ee1`; lint and two unit files run, every path into the tail's attach and every way the pane leaves the screen traced; 4 [fix here], 3 [follow-up]. - Fixed: the tail started after the server, so what WordPress logged while booting was marked as the earlier run's and not counted; the "earlier run" line was sent under a file that appeared during the run; the new journey waited for that line and could miss it when the file was seen empty; the count's rule was in the hook and is now in `debug-log.cjs` with a unit test. - Follow-ups: the tail's reads have no end, so a line can be sent twice; a file replaced in place is sent again; a one-task window when the app itself takes the logs off screen. - Nothing found: the wider CSS selector (every element with the role, and everything that can be `inert`), the bridge's new argument, the journey helper. - **Since review:** the second pass's fixes are not reviewed again. One journey was added after it, for a line logged while the server starts, because the stand-in server logged nothing and moving the tail back did not turn anything red. Lint, `npm test`, `npm run test:electron`, all the journeys, the affected journeys at three window heights and with a slowed status read, and the packaged smoke were run on the tree that became `381be54`. </details> <details> <summary>Implementation notes</summary> - `src/renderer/debug-log.cjs`: `debugLogOnScreen`, `unseenIn`, `unseenLinesNote`. `src/renderer/hooks/use-site-logs.jsx` keeps whether the pane is on screen in a ref the tail's callback reads; it is cleared at once when another tab is selected and set by a layout effect. - `src/main.js` `startWpDebugTail`: `send( data, backlog )`; `attachFileWatcher( fromBefore )` is true only for the attempt made as the tail starts. `src/preload.js` hands the callback `{ backlog }` as a second argument. - `src/renderer/hooks/use-dev-server.jsx`: `startDebugTail()` before `startServer`, and `stopDebugTail()` on the one way out that does not go through `stopDevServer`. - `src/renderer/components/app-footer.jsx`: `LogsToggleNote` and the slot, which is the element `aria-describedby` names, so an empty slot is an empty description and nothing dangles. - `tests/e2e/helpers/ui.cjs` `panelsTakingRoom`: selects tabs and counts panels inside the page in one turn, since a frame is what must not come between. - The logs journeys keep what the window is told of debug.log (`debugTold`), because a pane that is not on screen cannot say a line has arrived. - Local checks on macOS: `npm run lint` clean, `npm test` and `npm run test:electron` pass (1943 tests), `npm run test:e2e` 113 pass, `npm run test:e2e:packaged` 15 pass. </details> <details> <summary>Screenshots or recording</summary> Not attached: I have no way to upload images from the command line. The steps under "Worth a look by hand" show everything that changed. </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 Third and last part of #558. The terminal (#612) and the logs (#613) are in the tray along the bottom of the window; the mail a site's WordPress sent was still a section at the foot of each site's page. This moves it into the tray, as the prototype has it. ## What changes **A third button in the footer, "Toggle Email".** The tray holds one thing at a time: the terminal, the logs or the mail. **The mail in the tray.** The open site's list, newest first, filling the tray, and under it where the mail server is listening. A row opens its mail in the dialog, as before. The Mail section is gone from the page. Each site keeps its own list, as each keeps its own terminal and logs. **"Clear emails" is in the tray's heading**, beside the button that closes it, and cannot be pressed while the list is empty. That is the prototype's, and it takes one thing away: the list is empty until the dev server has first run, whatever the app holds from before, so mail kept from an earlier session can be cleared only once the server has been started. Before, the button could always be pressed. **The logs take nothing's place when the app brings them up.** They gave way to the terminal alone, where someone may be typing; with the mail open someone may be about to open one. They come up by themselves only in a tray that is closed or already showing them, and the page says where a failure's lines are, as it does under the terminal. The terminal still takes the tray whatever is in it. **The count on the Logs button** starts at the footer's top edge and is one line tall, so that a wide one is clear of the envelope beside it, and is drawn over the buttons. **Labels follow the prototype:** the tray is "Email", and an empty list says "No mail yet. WordPress transactional email will appear here once the site sends any." **Not in this pull request:** - A sign that mail has arrived while the tray is closed or showing something else. On the page the list was always in view; now such a mail shows nowhere until the tray is opened. Left out on purpose, by the maintainer's decision. - The dialog a mail opens in. Mail HTML rendering is unchanged and tracked separately. - The user guide and the README, which still describe a Mail panel on the page: #561. ## How to test this Platforms: any. The journeys drive this on macOS and Windows. From the repository root, after `npm ci` and `npm run build:once`: ``` npx playwright test --project=journeys mail.spec tray.spec site-header.spec ``` - `mail.spec.js`: the mail is in the Email tray, named and headed for it, and with the tray put away no row is on screen; a row opens the mail that was clicked; mail that arrives joins the list for its own site only; "Clear emails" is disabled with nothing to clear, empties the list and what the app keeps, and is disabled again. - `tray.spec.js`: each of the three footer buttons puts its own in the tray in the others' place; with two sites the heading has one "Clear emails"; a build that fails with its output in the logs leaves an open Email tray where it is, and the page says where the lines are. - `site-header.spec.js`: two journeys used the Mail section as the end of the page and now give the page its height with the tray. I broke seven behaviours of the mail tray one at a time and each turned a journey red, and the details' two rules (always let go, never let go) at three window sizes. `tests/unit/site-mail.test.cjs` and `tests/unit/tray.test.cjs` hold the decisions. **Worth a look by hand** (any platform, the current head; I have not done this, the journeys stand in for the server): on a site that is set up, press **Start development server**, then press the envelope in the footer. The tray says where SMTP is listening. Make WordPress send a mail, for example **Lost your password?** on the site's login page: the mail is in the list. Press its row: the dialog opens. Close it and press **Clear emails**: the list is empty and the button cannot be pressed. **What must not have happened:** a site's mail in another site's tray; the list losing mail when the tray is closed, shows something else, or another site is opened; the logs coming up over an open mail list; anything on the page ending up under the tray. **What cannot be tested by hand here:** I had no real server to send a mail through. The journeys say what the main process says when one is caught, on the channels it says it on. ## Risks and limitations - Review: 2 passes; 10 findings fixed here, 2 settled by the maintainer's decisions (below), 4 left as follow-ups. Details below. - **Mail that arrives unseen shows nowhere** until the tray is opened. See "Not in this pull request". - **Mail kept from an earlier session cannot be cleared until the server has run once.** See "What changes". - Focus drops to the page when the app itself puts the terminal in place of the mail while the focus is in the mail's tray. The logs and the terminal already behave this way when one replaces the other. - A row's hover is faint on the list's ground, the same weak surface the logs are on. - A clear that fails says nothing, as before. - `toasts.spec.js:52` failed once in a full local run with "Cannot fast-forward to the past" and passes on its own; it is the third time I have seen it, it is not from this change, and it is flagged separately. ## Related Part of #558, and the last of its three. Follows #614. Mail HTML rendering is unchanged and tracked separately. --- <details> <summary>Design decisions and alternatives considered</summary> - **What the prototype lacks is kept and redrawn.** Its tray has a title, "Clear emails" and a placeholder text: no list, no line for the mail server, no dialog. The app has all three and keeps them, in the tray. - **A slot in the tray's heading**, filled by the open site while the mail is what the tray shows, for what is done to the whole of what the tray holds. The tray's heading is the window's, and the list is a site's. - **The dialog is left exactly as it is**, old components and all, so that this change and the one that redraws how a mail is shown do not meet in the same file. - **The logs give way to everything, not only to the terminal.** One rule for "the app asks for the logs" that does not have to know what each tray is for: they never take a tray from under someone. - **The count stays the design system's badge**, one line tall by losing its vertical padding, and not a smaller drawing of my own. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **Pass 1: 7 [fix here] · 5 [follow-up]. Pass 2 (on the fix-up): 3 [fix here] · 1 [follow-up]. 10 fixed here (7 and 3, one of pass 1's being a follow-up applied here), 2 settled by the maintainer's decisions, 4 left as follow-ups.** - **Review:** completed — a separate agent context, read-only; head `b9b7daa` / base `351d34b`; lint and the two unit files run, the journeys read and not run; 7 [fix here], 5 [follow-up]. - Fixed: the check that the mail is nowhere but the tray ran before the list had loaded and could not fail; the details journey guarded on a margin of my own and not on the app's boundary, and read it before the page was at its top; the count on the Logs button was drawn under the next button when that was pressed, and its comment was wrong; the step that leaves an open mail list where it is did not read that the page says where the lines are; one journey needed a page longer than the room above a tray at its first height; the check that the tray covers nothing read the last card and not what ends the page. - Applied here though marked follow-up: who a mail is from now gives way before its subject in a narrow window. - Decided by the maintainer: "Clear emails" stays disabled on an empty list, as the prototype has it (one [fix here], recorded above as what it takes away); no sign for mail that arrives unseen (one [follow-up], not wanted). - Follow-ups left: focus when the app swaps the tray from under it; the row's faint hover; a failed clear that says nothing. - **Review:** completed — a second separate agent context, read-only; `b9b7daa` plus the fix-up as staged, before it could be committed (it is `09ab94d`), base `351d34b`; lint and the two unit files run, each fix-up traced, every journey that depends on the page's length listed; 3 [fix here], 1 [follow-up]. - Fixed: a rule and its comment left from the heading the mail had on the page; a comment that said who a mail is from gives way first, which the rules did not do (they do now); a comment in the details journey that did not say what its loop does. - Follow-up left: a comment in the mail dialog's file still says the list is under "Mail". The dialog is not touched here. - Nothing found: the seven fix-ups, the journeys' absences, the other journeys against a shorter page, the screenshot script. - **Since review:** `b9b7daa → 09ab94d` is the two passes' fixes; the second pass's three are not reviewed again, and are a deleted rule, one flex value and comments. Lint, `npm test`, `npm run test:electron`, all 113 journeys, the affected journeys at three smaller window heights and with a slowed status read, and the packaged smoke were run on the tree that became `09ab94d`, the journey suite before the second pass's three fixes and the affected journeys after them. </details> <details> <summary>Implementation notes</summary> - New: `src/renderer/site-mail.cjs` (`smtpNote`, `noMailNote`, `mailRow`), `src/renderer/components/mail-panel.jsx` (`MailPanel`, `MailTrayActions`), `tests/unit/site-mail.test.cjs`. - `src/renderer/components/bottom-tray.jsx`: `SiteTrayActionsFill` and its slot in the heading. `src/renderer/tray.cjs`: the third tray and `trayAfterReveal`'s rule. - A mail's row is a native button in a list item, where it was a `div` with a role and key handlers. Who a mail is from now gives way before its subject does in a narrow window. - `scripts/screenshots/shots.cjs`: the mail's shot is taken of the tray. I ran that one shot to check the script and did not commit the image; #561 retakes them all. - The details journey reads both of its halves part of the way down the page, where a column that is held is not yet pushed by the end of the cards, and guards on the app's own boundary: the details fit until they are taller than the page, which is 24px less than nothing to spare under them. - Local checks on macOS: `npm run lint` clean, `npm test` and `npm run test:electron` pass (1948 tests), `npm run test:e2e` 113 pass, `npm run test:e2e:packaged` 15 pass. </details> <details> <summary>Screenshots or recording</summary> Not attached: I have no way to upload images from the command line. The steps under "Worth a look by hand" show everything that changed. </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
Second part of #558. #612 built the tray along the bottom of the window and moved the terminal into it. This pull request moves the logs there, as the prototype has them; the mail follows in its own pull request and stays on the page until then.
What changes
A second button in the footer, "Toggle Logs". The tray holds one thing at a time, the terminal or the logs, and each button puts its own in the other's place or closes the tray.
The three logs are tabs in the tray, drawn with the design system's tabs: Server, Build watch, Debug.log. The selected log fills the tray and follows its height; debug.log's path and its three buttons (Show in folder, Copy, Clear) are along the bottom of its tab. The Logs section is gone from the page. Each site keeps its own logs and its own selected tab, as each keeps its own terminal.
The tabs follow the app's choice of tab (a fix). The old tab panel kept its own idea of which tab was open, so the app's "show the watch's tab" never changed what was on screen. The tabs are controlled now: starting the build watch on a site with no build shows the watch's tab, as the code always meant.
When the tray opens by itself. On the page the logs were always in view, and two failures have their only words there. So the tray opens on the Logs:
The logs never take the terminal's place: someone may be typing there. With the terminal open the tab is still selected, and the page says what happened and where its last lines are, which for the server is new: "The development server stopped by itself. Its last lines are in the Logs." and "The development server could not start. Its last lines are in the Logs.", under the server's details until it is started again. The terminal may still take the logs' place for a failure or a refusal printed there. A watch or a server stopped by its own button brings nothing up, and a failure on another site than the open one waits until that site is opened.
Labels follow the prototype: "Build watcher" is "Build watch" and "debug.log" is "Debug.log" on the tabs. Two sentences that quoted the header's label ("Wait for the header to say Build watching") no longer quote anything: the label is hidden in a narrow window.
Not in this pull request:
How to test this
Platforms: any. The journeys below drive this on macOS and Windows. From the repository root, after
npm ciandnpm run build:once:tray.spec.js: the tray holds one thing at a time; a watch that fails brings the logs up, one stopped by its button brings nothing, and neither takes the terminal's place; a refusal by the watch is said in the terminal; a site whose deletion fails brings nothing up later.logs.spec.js: the Logs tray shows debug.log's lines, the pane grows with the tray, lines that arrive while the tray is away are there when it comes back, and the pane is at its end after a reopen.build-watch.spec.jsanddev-server.spec.js: the watch's tab is selected when the watch ends; a server that crashes with the tray closed opens the Logs on the Server tab and the page says so; a start that fails under the terminal leaves the terminal, and the page says so; the sentence goes when the server is started again.I broke 20 behaviours one at a time; 19 turned a journey red, and the one that did not was a guard nothing needed, which is gone.
tests/unit/tray.test.cjsandtests/unit/site-processes.test.cjshold the decisions (which request takes the tray, what the server's details say).Worth a look by hand (any platform, the current head; the journeys stand in for the server and the watch, so this is the part they cannot reach): on a site that is set up, press Start development server, open the Logs from the footer, and look at the three tabs while the site is used: the server's lines under Server, the watch's under Build watch, and under Debug.log whatever
wp-content/debug.loggets, with Show in folder, Copy and Clear along the bottom. Drag the tray's top edge: the log follows it. Open another site and come back: the tab that was selected still is.What must not have happened: a log losing lines while the tray was closed, was showing the terminal, or another site was open; the logs taking the place of a terminal someone is typing in; the tray opening for a watch or a server the contributor stopped.
What cannot be tested by hand here: a real server that fails to start or dies, which I could not stage. Main tells the window of a failed start twice, with an answer and with the server's exit, so which of the two new sentences a real failed start gets depends on which arrives first; both say where the lines are. The journeys cover each message on its own.
Risks and limitations
Related
Part of #558. Follows #612. Mail HTML rendering is unchanged and tracked separately.
Design decisions and alternatives considered
trayAfterRevealinsrc/renderer/tray.cjs, and a request made while the site is not the open one is kept by the same rule.Review outcome (required — see AGENTS.md)
Pass 1: 6 [fix here] · 3 [follow-up]. Pass 2 (on the fix-up): 6 [fix here] · 2 [follow-up], counted from its list; its own headline said 5 and 3. 12 fixed (7 and 5), 1 answered only in part, 4 left as follow-ups.
3afcd88/ base2ef8177; lint and the unit tests run, the journeys read and not run; 6 [fix here], 3 [follow-up].1c0d06f/ base2ef8177; lint and the tray's and the watch's unit tests run, every request to show the tray traced to what ends up on screen; 6 [fix here], 2 [follow-up].1c0d06f → 637d29ais those fixes, not reviewed again. The fix for the silent failure is the one the reviewer proposed, and a journey pins that the sentence goes when the server is started again (red without the line that clears it). The run with a slowed status read then showed one journey of the header's racing the site's status, one time in five: it read where the page was while the app was still bringing the site's next step into view. It now waits for the status and has the app jump instead of glide; ten slowed runs pass. Lint,npm test,npm run test:electron, all 111 journeys, the small-window and slowed runs of the affected journeys and the packaged smoke re-run on637d29a.Implementation notes
src/renderer/components/logs-panel.jsx.trayAfterRevealis insrc/renderer/tray.cjs; the server's failed state isserverProcess's insrc/renderer/site-processes.cjs, fed byserverFailureinuse-dev-server.jsx, which is cleared when a start is asked for.useSiteLogstakesshownand hands out callback refs for the three panes, since a pane is in the document only while its tab is selected and the tray's slot fills a render after the site's view mounts.outputInTerminal: false), so their failure opens the logs and not the terminal.npm run lintclean,npm testandnpm run test:electronpass (1937 tests),npm run test:e2e111 pass,npm run test:e2e:packaged15 pass.Screenshots or recording
Not attached: I have no way to upload images from the command line. The steps under "Worth a look by hand" show everything that changed.
🤖 Generated with Claude Code