Skip to content

Commit 022ea60

Browse files
zaerlclaude
zaerl
and
claude
authored
Move the rename dialog out of SiteRow into a component of its own (#568)
## Why `SiteRow` is about 5,000 lines and holds the state of everything a site's view can do. The redesign in #542 needs its dialogs as components it can restyle one at a time, and #553 asks for them to leave `index.jsx` one per pull request. This is the first: the rename dialog, chosen because nothing else in `SiteRow` reads its state. ## What changes `src/renderer/components/rename-site-modal.jsx` is new. It holds what was inline in `SiteRow`: the value being typed, the refusal, the busy flag, two effects, the handlers and the markup. `SiteRow` keeps the one thing that is its business, whether the dialog is open. One thing is different in structure. **The component is mounted while the dialog is open and not otherwise.** Opening used to reset the value to the site's name and clear the last refusal by hand; a fresh mount does both, so those two lines are gone rather than moved. Everything else is the code that was there, with `onClose()` where it set `SiteRow`'s flag. **No journey drove this dialog**, so one does now. `rename-site.spec.js` was written against the inline version first and passes on both: the dialog opens on the site's name with it selected, refuses an empty one, writes nothing when cancelled, and puts the saved name in the sidebar and the heading. `siteHeading` joins the shared locators in `ui.cjs`, now that two files use it. Deliberately not in this PR: the other four dialogs, and the two things listed under Risks that would change what the dialog does. ## How to test this Platforms: any. Use the Buildkite build for the current head of this branch. The journey carries this: `npx playwright test --project=journeys -g "rename dialog"`, from the repository root. It passes on `trunk`'s inline dialog and on the component, and it is red when Save does not rename, when an empty name is accepted, when Cancel saves what was typed, and when the name is not selected on opening. What it does not reach is the dialog while a rename is pending, which is too brief to stage against a local store: **Starting state:** the app open on any site. 1. Click the pencil beside the site's name. The "Rename site" dialog opens with the name selected. 2. Type a new name and press Enter. The dialog closes and the name changes in the sidebar and in the heading. 3. Open it again, clear the field, and click "Save". "Site name cannot be empty." appears under the field and the dialog stays open. 4. Press Escape. The dialog closes and the name is unchanged. **What must not have happened:** - The name typed and then abandoned in step 4's dialog is not in the field the next time it opens. - No other site's name changed. ## Risks and limitations - **A failed rename still closes the dialog.** `App`'s rename handler catches every error, shows it in a native alert and resolves, so the dialog's own place for a refusal can never be reached and what was typed is lost. This is how `trunk` behaves; the move keeps it, and fixing it changes behaviour. - **The empty-name check is still in a `.jsx` file**, where the unit suite cannot reach it. It was moved, not added. The rule in the review standard says it belongs in a `.cjs` module with a test; the journey covers it at layer 4 until then. - **The journey costs one app launch** on every pull request, on both platforms: about a second and a half locally on macOS. - **Two of its assertions could not be shown to be load-bearing on their own.** The saved name's surrounding spaces are removed by the main process as well as the dialog, and what was typed is forgotten both by the fresh mount and by an effect; breaking one of each pair leaves the test green. - **Checked on macOS only.** Windows is covered by CI. - Review: 1 [fix here], fixed; 2 [follow-up], the first two items above. ## Related Part of #553, which has four more dialogs. Part of #542. Follows #567. --- <details> <summary>Design decisions and alternatives considered</summary> **Mounted while open, rather than always mounted with an `isOpen` prop.** An always-mounted component would have to reset its own state each time it opens, from an effect, which is the code this removes. Mounting it fresh is the reset. **`SiteRow` still owns the open flag.** The button that opens the dialog is in the site's header, which `SiteRow` renders, so the flag has to live where both can reach it. **The state keeps its names**, `renameValue`, `renameError`, `renaming`, although inside a component called `RenameSiteModal` the prefix says nothing. Keeping them makes the diff readable as a move. **A journey of its own rather than a step in `engine.spec.js`.** That file's restart test renames through the handler directly, deliberately, because what it proves is the write reaching disk. The dialog is a different claim. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **1 [fix here] · 2 [follow-up]** — the one fixed, the two recorded under Risks. - Tests, low, [fix here], fixed: TESTING.md and the helper's header listed what `ui.cjs` holds as the shared controls and four locators, and `siteHeading` is neither. Both say five. - Architecture, low, [follow-up], deferred: the empty-name check and its sentence are in JSX. Moved, not added; extracting it to a module changes nothing a contributor sees but is a change to the code's shape beyond a move. - Architecture, low, [follow-up], deferred: `App.onRename` swallows a failed write, so the dialog closes on failure. It predates this change, and fixing it is a behaviour change. The review went through every piece of state, effect, handler and prop against `trunk` and found the behaviour the same, including a state update after unmount on success, closing while a rename is pending, and focus and selection on opening. - **Review:** completed — fresh agent context, local, before the PR existed, read-only; head b6c996d / base 2b6a69a; committed work only, working tree clean; outcome above. CodeRabbit: not run yet. - **Since review:** `b6c996d → 655a5ed` checked. The difference is the fix and one comment in the component. It was not reviewed again by a fresh context. Checks, macOS, local: | Check | On | Result | |---|---|---| | `eslint . --max-warnings=0` | 655a5ed | clean | | unit suite on Node | b6c996d | 1691 pass, 2 skipped, 0 fail | | unit suite on Electron's Node | b6c996d | 1693 pass, 0 fail | | `npm run test:e2e` | b6c996d | 35 of 35 pass | | the two journeys that use what the second commit touched | 655a5ed | 2 of 2 pass | | `npm run test:e2e:packaged` | b6c996d | 15 of 15 pass | Lint and the unit suites were run on this checkout's files only: a worktree left by another local session sits under `.claude/worktrees/`, and the bare commands collect it. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 2b6a69a commit 022ea60

6 files changed

Lines changed: 208 additions & 89 deletions

File tree

‎TESTING.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ Every matrix uses `fail-fast: false`, so a Windows failure never hides the macOS
221221
- **`npm test` is separated from the end-to-end suite by filename, not by directory.** `node --test` is run with no path at all, so it walks the whole repository and collects by name: `*.test.cjs` and four other shapes Node treats as test files — `*-test.cjs`, `*_test.cjs`, `test-*.cjs`, `test.cjs`. The journeys are `*.spec.js`, which matches none of them. Now that both suites live under `tests/`, that naming is the only thing keeping them apart, and a file under `tests/e2e/` named any of the five would silently join the fast suite and cost it an app launch. **End-to-end files are `.spec.js`.** The separation is worth defending: the fast suite has to stay something you run without thinking.
222222
- End-to-end selectors read the text and roles already on screen. No `data-testid` — the visible copy is the contract, and renaming a button is a change worth noticing.
223223
- Journeys run in the `en-GB` locale and read English text. The exception is `tests/e2e/journeys/i18n.spec.js`, which launches with `lang: 'en-XA'` (Chromium's pseudo-locale) and fails on any visible text that did not go through `__()`. See [Translatable strings](CONTRIBUTING.md#translatable-strings).
224-
- What is on screen is named once, in `tests/e2e/helpers/ui.cjs`, for the journeys, the other end-to-end specs and `scripts/screenshots/` alike, so renaming a button or rebuilding a card is one change. It holds every control that more than one of those files reaches, and four locators that find a part of the screen by how it is built rather than by what it says: the rendered app, a sidebar entry, a card and a ticket's row. A sentence one journey asserts, a button only one journey presses, and a selector only one file needs stay in that file: they are what it is about.
224+
- What is on screen is named once, in `tests/e2e/helpers/ui.cjs`, for the journeys, the other end-to-end specs and `scripts/screenshots/` alike, so renaming a button or rebuilding a card is one change. It holds every control that more than one of those files reaches, and five locators that find a part of the screen rather than something to press: the rendered app, a sidebar entry, the open site's heading, a card and a ticket's row. A sentence one journey asserts, a button only one journey presses, and a selector only one file needs stay in that file: they are what it is about.
225225
- No test reads a component's source, the `.jsx` under `src/renderer/`, as text. A test that greps the component for a call, a literal or the order of two strings pins the file rather than the screen: it goes red when the code moves and stays green when the screen is wrong. What is on the page and where, a journey asserts, with order read as document order rather than from bounding boxes (the page scrolls as cards appear). How a component is allowed to be written, a lint rule asserts: `react-hooks/refs` is on for the renderer for that reason.
226226
- Journeys mark each assertion as an **invariant** (must hold under any model of how work is stored) or a **characterisation** (true because of how the app stores things today). A red invariant is a bug; a red characterisation is a prompt to read it and update it deliberately.
227227
- A bugfix's test must fail on the old code. A test written after the fix pins whatever the current behaviour is, rather than the correction.

‎src/renderer/components/rename-site-modal.jsx‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { useCallback, useEffect, useState } from 'react';
2+
import { Button, Modal, TextControl } from '@wordpress/components';
3+
4+
const RENAME_INPUT_ID = 'rename-site-name-input';
5+
6+
// The dialog that renames a site. Mounted while it is open and not otherwise,
7+
// so every opening starts from the name the site has now, with no error left
8+
// over from the last one: the value, the refusal and the busy flag are this
9+
// component's and go when it does. Whoever renders it owns only whether it is
10+
// open.
11+
//
12+
// `onRename(sitePath, name)` does the renaming and is awaited; the dialog
13+
// closes itself when it resolves. While it is pending nothing closes the
14+
// dialog, neither Cancel nor Escape nor a click outside it, so the answer has
15+
// somewhere to arrive.
16+
export function RenameSiteModal({ sitePath, displayName, onRename, onClose }) {
17+
const [renameValue, setRenameValue] = useState(displayName);
18+
const [renameError, setRenameError] = useState('');
19+
const [renaming, setRenaming] = useState(false);
20+
// Follows the site's name if it changes under an open dialog. On mount it
21+
// sets the value the state already starts with, which does nothing.
22+
useEffect(() => { setRenameValue(displayName); }, [displayName]);
23+
useEffect(() => {
24+
const input = document.getElementById(RENAME_INPUT_ID);
25+
if (input) {
26+
input.focus();
27+
if (typeof input.select === 'function') input.select();
28+
}
29+
}, []);
30+
31+
const closeRenameModal = useCallback(() => {
32+
if (renaming) return;
33+
onClose();
34+
}, [renaming, onClose]);
35+
36+
const submitRename = useCallback(async () => {
37+
const trimmed = renameValue.trim();
38+
if (!trimmed) {
39+
setRenameError('Site name cannot be empty.');
40+
return;
41+
}
42+
try {
43+
setRenaming(true);
44+
setRenameError('');
45+
if (onRename) await onRename(sitePath, trimmed);
46+
onClose();
47+
} catch (err) {
48+
setRenameError(String(err));
49+
} finally {
50+
setRenaming(false);
51+
}
52+
}, [onRename, onClose, renameValue, sitePath]);
53+
54+
const handleRenameSubmit = useCallback((event) => {
55+
event.preventDefault();
56+
submitRename();
57+
}, [submitRename]);
58+
59+
const handleRenameFormKeyDown = useCallback((event) => {
60+
if (event.key === 'Escape') {
61+
event.preventDefault();
62+
event.stopPropagation();
63+
closeRenameModal();
64+
}
65+
}, [closeRenameModal]);
66+
67+
return (
68+
<Modal
69+
title="Rename site"
70+
onRequestClose={closeRenameModal}
71+
shouldCloseOnClickOutside={!renaming}
72+
>
73+
{/* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions -- Escape-to-close/Enter-to-submit on the modal form is standard, intentional behavior. */}
74+
<form
75+
onSubmit={handleRenameSubmit}
76+
onKeyDown={handleRenameFormKeyDown}
77+
style={{ display: 'flex', flexDirection: 'column', gap: 12 }}
78+
>
79+
<TextControl
80+
id={RENAME_INPUT_ID}
81+
label="Site name"
82+
value={renameValue}
83+
onChange={(value) => setRenameValue(value)}
84+
disabled={renaming}
85+
// eslint-disable-next-line jsx-a11y/no-autofocus -- intentional: this is the only field of a just-opened modal.
86+
autoFocus
87+
/>
88+
{renameError ? <div style={{ color: '#d63638', fontSize: 12 }}>{renameError}</div> : null}
89+
<div style={{ display: 'flex', justifyContent: 'flex-end', gap: 8 }}>
90+
<Button type="button" variant="secondary" onClick={closeRenameModal} disabled={renaming}>Cancel</Button>
91+
<Button type="submit" variant="primary" isBusy={renaming} disabled={renaming}>Save</Button>
92+
</div>
93+
</form>
94+
</Modal>
95+
);
96+
}

‎src/renderer/index.jsx‎

Lines changed: 6 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ import { DiffText } from './components/diff-text.jsx';
7272
import { LogText } from './components/log-text.jsx';
7373
import { Destination, DestinationGroup } from './components/destination.jsx';
7474
import { TerminalCommandLink } from './components/terminal-command-link.jsx';
75+
import { RenameSiteModal } from './components/rename-site-modal.jsx';
7576
import { useDetectedEditors } from './hooks/use-detected-editors.jsx';
7677
import { useContributorProvenance } from './hooks/use-contributor-provenance.jsx';
7778
import { useNextActionCue } from './hooks/use-next-action-cue.jsx';
@@ -133,7 +134,6 @@ const UPDATE_STEP_MARKS = {
133134
const FILE_MANAGER_LABELS = { darwin: 'Show in Finder', win32: 'Show in Explorer' };
134135
const FILE_MANAGER_NAMES = { darwin: 'Finder', win32: 'File Explorer' };
135136
const TERMINAL_INSTALL_ALIASES = ['npm install', 'npm i', 'install'];
136-
const RENAME_INPUT_ID = 'rename-site-name-input';
137137
const CREATE_SITE_NAME_INPUT_ID = 'create-site-name-input';
138138
const CREATE_SITE_LOCATION_INPUT_ID = 'create-site-location-input';
139139
const CREATE_SITE_LOCATION_HELP_ID = 'create-site-location-help';
@@ -1343,61 +1343,11 @@ function SiteRow({ sitePath, initialized, createdAt, label, projectType = null,
13431343

13441344
const siteName = pathBasename(sitePath);
13451345
const displayName = (label && label.trim()) || siteName;
1346+
// Whether the rename dialog is up. Everything else about it, the value being
1347+
// typed, the refusal, the busy flag, is the dialog's own (#553).
13461348
const [renameModalOpen, setRenameModalOpen] = useState(false);
1347-
const [renameValue, setRenameValue] = useState(displayName);
1348-
const [renameError, setRenameError] = useState('');
1349-
const [renaming, setRenaming] = useState(false);
1350-
useEffect(() => { setRenameValue(displayName); }, [displayName]);
1351-
useEffect(() => {
1352-
if (!renameModalOpen) return;
1353-
const input = document.getElementById(RENAME_INPUT_ID);
1354-
if (input) {
1355-
input.focus();
1356-
if (typeof input.select === 'function') input.select();
1357-
}
1358-
}, [renameModalOpen]);
1359-
1360-
const openRenameModal = useCallback(() => {
1361-
setRenameValue(displayName);
1362-
setRenameError('');
1363-
setRenameModalOpen(true);
1364-
}, [displayName]);
1365-
1366-
const closeRenameModal = useCallback(() => {
1367-
if (renaming) return;
1368-
setRenameModalOpen(false);
1369-
}, [renaming]);
1370-
1371-
const submitRename = useCallback(async () => {
1372-
const trimmed = renameValue.trim();
1373-
if (!trimmed) {
1374-
setRenameError('Site name cannot be empty.');
1375-
return;
1376-
}
1377-
try {
1378-
setRenaming(true);
1379-
setRenameError('');
1380-
if (onRename) await onRename(sitePath, trimmed);
1381-
setRenameModalOpen(false);
1382-
} catch (err) {
1383-
setRenameError(String(err));
1384-
} finally {
1385-
setRenaming(false);
1386-
}
1387-
}, [onRename, renameValue, sitePath]);
1388-
1389-
const handleRenameSubmit = useCallback((event) => {
1390-
event.preventDefault();
1391-
submitRename();
1392-
}, [submitRename]);
1393-
1394-
const handleRenameFormKeyDown = useCallback((event) => {
1395-
if (event.key === 'Escape') {
1396-
event.preventDefault();
1397-
event.stopPropagation();
1398-
closeRenameModal();
1399-
}
1400-
}, [closeRenameModal]);
1349+
const openRenameModal = useCallback(() => setRenameModalOpen(true), []);
1350+
const closeRenameModal = useCallback(() => setRenameModalOpen(false), []);
14011351
const createdLabel = createdAt ? new Date(createdAt).toLocaleString() : '';
14021352
const [pathCopied, setPathCopied] = useState(false);
14031353
const copyTimeoutRef = useRef(null);
@@ -5712,33 +5662,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, projectType = null,
57125662
</Modal>
57135663
) : null}
57145664
{renameModalOpen ? (
5715-
<Modal
5716-
title="Rename site"
5717-
onRequestClose={closeRenameModal}
5718-
shouldCloseOnClickOutside={!renaming}
5719-
>
5720-
{/* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions -- Escape-to-close/Enter-to-submit on the modal form is standard, intentional behavior. */}
5721-
<form
5722-
onSubmit={handleRenameSubmit}
5723-
onKeyDown={handleRenameFormKeyDown}
5724-
style={{ display: 'flex', flexDirection: 'column', gap: 12 }}
5725-
>
5726-
<TextControl
5727-
id={RENAME_INPUT_ID}
5728-
label="Site name"
5729-
value={renameValue}
5730-
onChange={(value) => setRenameValue(value)}
5731-
disabled={renaming}
5732-
// eslint-disable-next-line jsx-a11y/no-autofocus -- intentional: this is the only field of a just-opened modal.
5733-
autoFocus
5734-
/>
5735-
{renameError ? <div style={{ color: '#d63638', fontSize: 12 }}>{renameError}</div> : null}
5736-
<div style={{ display: 'flex', justifyContent: 'flex-end', gap: 8 }}>
5737-
<Button type="button" variant="secondary" onClick={closeRenameModal} disabled={renaming}>Cancel</Button>
5738-
<Button type="submit" variant="primary" isBusy={renaming} disabled={renaming}>Save</Button>
5739-
</div>
5740-
</form>
5741-
</Modal>
5665+
<RenameSiteModal sitePath={sitePath} displayName={displayName} onRename={onRename} onClose={closeRenameModal} />
57425666
) : null}
57435667
{isPatchOpen && (
57445668
<Modal

‎tests/e2e/helpers/ui.cjs‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@
1010
// is a change to this file and to nothing that uses it.
1111
//
1212
// What is here: every control, something typed into or clicked, that more
13-
// than one of those files reaches; and four locators that find a part of the
14-
// screen by how it is built rather than by what it says, which are the
15-
// rendered app, a sidebar entry, a card and a ticket's row.
13+
// than one of those files reaches; and five locators that find a part of the
14+
// screen rather than something to press, which are the rendered app, a sidebar
15+
// entry, the open site's heading, a card and a ticket's row.
1616
//
1717
// What is not here: a sentence one journey asserts, a button only one journey
1818
// presses, and a selector only one file needs even when it reads the markup
@@ -65,6 +65,16 @@ const createSiteDialog = ( page ) => page.getByRole( 'dialog', { name: 'Create a
6565
*/
6666
const renderedApp = ( page ) => page.locator( '#root > * > *' );
6767

68+
/**
69+
* The open site's heading. The site's name is in the sidebar too, so this is
70+
* the half of the pair `sidebarEntry` is not.
71+
*
72+
* @param {Object} page
73+
* @param {string} label The site's name.
74+
* @return {Object} The locator.
75+
*/
76+
const siteHeading = ( page, label ) => page.getByRole( 'heading', { name: label, exact: true } );
77+
6878
const siteMenuButton = ( page ) => page.getByRole( 'button', { name: 'More', exact: true } );
6979
const updateTrunkMenuItem = ( page ) => page.getByRole( 'menuitem', { name: 'Update to latest trunk', exact: true } );
7080
const deleteSiteMenuItem = ( page ) => page.getByRole( 'menuitem', { name: 'Delete this site', exact: true } );
@@ -206,6 +216,7 @@ module.exports = {
206216
createSiteButton,
207217
createSiteDialog,
208218
renderedApp,
219+
siteHeading,
209220
siteMenuButton,
210221
updateTrunkMenuItem,
211222
deleteSiteMenuItem,

‎tests/e2e/journeys/engine.spec.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ const ui = require( '../helpers/ui.cjs' );
2424
// fixture's problem; these are the fake sites and patch files pointed at from it.
2525
// The site's name is on screen twice — the sidebar entry and the heading of the
2626
// open site — so neither can be reached by text alone. Roles tell them apart, and
27-
// say which half of the app the assertion is about. The entry is `ui.sidebarEntry`.
28-
const openSiteHeading = ( page, label ) => page.getByRole( 'heading', { name: label, exact: true } );
27+
// say which half of the app the assertion is about: `ui.sidebarEntry` and
28+
// `ui.siteHeading`.
2929

3030
const scratch = [];
3131
test.afterEach( () => {
@@ -160,7 +160,7 @@ test( 'state written by the app survives closing and reopening it', async ( { se
160160

161161
const { page: reopened } = await session.restart();
162162
await expect( ui.sidebarEntry( reopened, 'after-restart' ) ).toBeVisible();
163-
await expect( openSiteHeading( reopened, 'after-restart' ) ).toBeVisible();
163+
await expect( ui.siteHeading( reopened, 'after-restart' ) ).toBeVisible();
164164
expect( session.readSettings().siteMeta[ site.dir ].label ).toBe( 'after-restart' );
165165
} );
166166

‎tests/e2e/journeys/rename-site.spec.js‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/**
2+
* Renaming a site, through the dialog a contributor uses (#553).
3+
*
4+
* The name is what tells two checkouts of the same repository apart, and it is
5+
* shown in two places that have to agree: the sidebar and the heading of the
6+
* open site. That the name survives a restart is `engine.spec.js`'s claim, made
7+
* there through the same handler; this is the half only the interface can
8+
* show, that the dialog writes the name a contributor typed, where they will
9+
* look for it, and nothing when they back out.
10+
*
11+
* Assertions are marked INVARIANT or CHARACTERISATION; see
12+
* ticket-branches.spec.js for why.
13+
*/
14+
15+
const { test, expect } = require( '../helpers/app.cjs' );
16+
const ui = require( '../helpers/ui.cjs' );
17+
const { makeSite } = require( '../helpers/git-site.cjs' );
18+
19+
/**
20+
* The site's name as the main process holds it, asked for after whatever the
21+
* test just did. A write the dialog should not have made would have been sent
22+
* before this question, and the main process answers in the order it is asked,
23+
* so the name read here is the name after that write. Reading the settings
24+
* file straight after a click would only show that the write had not landed
25+
* yet.
26+
*
27+
* @param {Object} page
28+
* @param {string} dir
29+
* @return {Promise<string>} The label.
30+
*/
31+
const storedName = ( page, dir ) => page.evaluate( async ( sitePath ) => ( await window.api.getSitesWithMeta() ).siteMeta[ sitePath ].label, dir );
32+
33+
test( 'the rename dialog changes the name in the sidebar and the heading, refuses an empty one, and writes nothing when cancelled', async ( { session } ) => {
34+
const site = await makeSite( session, { label: 'first-name' } );
35+
const { page } = await session.start( site.settings );
36+
await expect( ui.siteHeading( page, 'first-name' ) ).toBeVisible( { timeout: 30_000 } );
37+
38+
const openDialog = page.getByRole( 'button', { name: 'Rename site', exact: true } );
39+
const dialog = page.getByRole( 'dialog', { name: 'Rename site' } );
40+
const field = dialog.getByLabel( 'Site name' );
41+
42+
// INVARIANT — the dialog opens on the name the site has, ready to be typed
43+
// over: the field holds it, has the focus, and the whole name is selected,
44+
// so the first key pressed replaces it.
45+
await openDialog.click();
46+
await expect( field ).toHaveValue( 'first-name' );
47+
await expect( field ).toBeFocused();
48+
expect( await field.evaluate( ( input ) => [ input.selectionStart, input.selectionEnd ] ) ).toEqual( [ 0, 'first-name'.length ] );
49+
50+
// INVARIANT — a name of nothing but spaces is refused where it was typed, and
51+
// the dialog stays open to be corrected. The site behind it cannot be read
52+
// by role while a dialog is up; that it kept its name is asserted once this
53+
// one is closed, below.
54+
await field.fill( ' ' );
55+
await dialog.getByRole( 'button', { name: 'Save', exact: true } ).click();
56+
await expect( dialog.getByText( 'Site name cannot be empty.', { exact: true } ) ).toBeVisible();
57+
expect( await storedName( page, site.dir ) ).toBe( 'first-name' );
58+
59+
// INVARIANT — backing out writes nothing, and what was typed is not kept for
60+
// the next time the dialog opens.
61+
await field.fill( 'abandoned-name' );
62+
await dialog.getByRole( 'button', { name: 'Cancel', exact: true } ).click();
63+
await expect( dialog ).toHaveCount( 0 );
64+
expect( await storedName( page, site.dir ) ).toBe( 'first-name' );
65+
await expect( ui.siteHeading( page, 'first-name' ) ).toBeVisible();
66+
await openDialog.click();
67+
await expect( field ).toHaveValue( 'first-name' );
68+
await expect( dialog.getByText( 'Site name cannot be empty.', { exact: true } ) ).toHaveCount( 0 );
69+
70+
// INVARIANT — saving closes the dialog and the new name is in both places a
71+
// contributor reads it. It is typed with spaces around it, which neither the
72+
// dialog nor the main process keeps.
73+
await field.fill( ' second-name ' );
74+
await dialog.getByRole( 'button', { name: 'Save', exact: true } ).click();
75+
await expect( dialog ).toHaveCount( 0 );
76+
await expect( ui.siteHeading( page, 'second-name' ) ).toBeVisible();
77+
await expect( ui.sidebarEntry( page, 'second-name' ) ).toBeVisible();
78+
await expect( ui.siteHeading( page, 'first-name' ) ).toHaveCount( 0 );
79+
80+
// CHARACTERISATION — the name is the site's label in the store.
81+
expect( session.readSettings().siteMeta[ site.dir ].label ).toBe( 'second-name' );
82+
83+
// INVARIANT — the next time the dialog opens it starts from the new name.
84+
await openDialog.click();
85+
await expect( field ).toHaveValue( 'second-name' );
86+
await page.keyboard.press( 'Escape' );
87+
await expect( dialog ).toHaveCount( 0 );
88+
} );

0 commit comments

Comments
 (0)