fix(projects): keep the heading, and make the sidebar test mean what it says
Renaming replaced the project page's h1 with an input, so a page an owner opened had no heading at all. The sidebar smoke test caught it on CI — and it was right to: every other view here has a heading, and it is what a screen reader announces on arrival. The heading stays and becomes a field when clicked. Same rename, one more click, and the page keeps its outline. The more interesting half is why that passed locally. The test clicked a link and immediately asserted a heading was visible — and `toBeVisible` was being satisfied by the heading of the page it had just left, before the SPA swapped it out. On this laptop the old h1 lingered long enough to answer; on a slower runner it did not. So the test did not check that the new page rendered, it checked that some page had at some point. waitForURL closes that. Verified by putting the broken heading back with the strengthened test in place: it now fails locally with the same message CI gave. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
committed by
Kristian Wind
co-authored by
Claude Opus 5
parent
8a6e1bc39e
commit
7ab68b0b58
+10
-2
@@ -82,6 +82,12 @@ test('hvert link i sidebjælken fører et sted hen', async ({ page }) => {
|
||||
for (const href of hrefs) {
|
||||
await sidebar.locator(`a[href="${href}"]`).first().click();
|
||||
|
||||
// Wait for the navigation before looking at the page. Without this the
|
||||
// assertion can be satisfied by the heading of the page you just left —
|
||||
// which is not a hypothetical: a change that removed a project page's
|
||||
// heading entirely passed here on a fast machine and failed on CI.
|
||||
await page.waitForURL(`**${href}`);
|
||||
|
||||
// Something with a heading rendered. A route written outside the routes
|
||||
// tree lands on the SPA shell and renders nothing — which is exactly how
|
||||
// the label route broke, and it answered 200 the whole time.
|
||||
@@ -196,9 +202,11 @@ test('et projekt kan omdøbes, slettes og hentes tilbage', async ({ page }) => {
|
||||
await sidebar.getByLabel('Projektnavn').fill('Ferie');
|
||||
await sidebar.getByLabel('Projektnavn').press('Enter');
|
||||
|
||||
const title = page.getByLabel('Projektets navn');
|
||||
await expect(title).toHaveValue('Ferie');
|
||||
await expect(page.getByRole('heading', { name: 'Ferie' })).toBeVisible();
|
||||
|
||||
// The heading is the rename affordance: click it, and it becomes a field.
|
||||
await page.getByRole('button', { name: 'Ferie' }).click();
|
||||
const title = page.getByLabel('Projektets navn');
|
||||
await title.fill('Sommerferie');
|
||||
await title.press('Enter');
|
||||
await expect(sidebar.getByRole('link', { name: 'Sommerferie' })).toBeVisible();
|
||||
|
||||
@@ -66,19 +66,19 @@
|
||||
let open = $derived(app.tasks.filter((t) => !t.completed && !t.parent_id));
|
||||
let unsectioned = $derived(open.filter((t) => !t.section_id));
|
||||
|
||||
let editing = $state(false);
|
||||
|
||||
/** Renames on blur. A no-op when nothing changed, so tabbing through is free. */
|
||||
async function rename(input) {
|
||||
const name = input.value.trim();
|
||||
if (!name || name === project.name) {
|
||||
input.value = project.name;
|
||||
return;
|
||||
}
|
||||
editing = false;
|
||||
if (!name || name === project.name) return;
|
||||
|
||||
try {
|
||||
project = await api.updateProject(project.id, { name });
|
||||
// The sidebar reads its own copy, so it has to be told.
|
||||
await app.refreshProjects();
|
||||
} catch (e) {
|
||||
input.value = project.name;
|
||||
app.toast(humanMessage(e));
|
||||
}
|
||||
}
|
||||
@@ -123,11 +123,16 @@
|
||||
<div class="view">
|
||||
{#if project}
|
||||
<header>
|
||||
{#if isOwner}
|
||||
<!-- A heading that happens to be editable, rather than a field that
|
||||
looks like one. Saves on blur; Escape puts the old name back. -->
|
||||
<!-- A heading first, a field second. Swapping the h1 out for an input
|
||||
outright cost the page its heading — which the sidebar smoke test
|
||||
caught, and which is a real loss: every view here has one, and it is
|
||||
what a screen reader announces on arrival. So the heading stays and
|
||||
becomes editable on click. -->
|
||||
{#if editing}
|
||||
<!-- svelte-ignore a11y_autofocus -->
|
||||
<input
|
||||
class="title"
|
||||
autofocus
|
||||
value={project.name}
|
||||
aria-label="Projektets navn"
|
||||
onblur={(e) => rename(e.currentTarget)}
|
||||
@@ -135,12 +140,20 @@
|
||||
if (e.key === 'Enter') e.currentTarget.blur();
|
||||
if (e.key === 'Escape') {
|
||||
e.currentTarget.value = project.name;
|
||||
e.currentTarget.blur();
|
||||
editing = false;
|
||||
}
|
||||
}}
|
||||
/>
|
||||
{:else}
|
||||
<h1>{project.name}</h1>
|
||||
<h1 class:renameable={isOwner}>
|
||||
{#if isOwner}
|
||||
<button onclick={() => (editing = true)} title="Klik for at omdøbe">
|
||||
{project.name}
|
||||
</button>
|
||||
{:else}
|
||||
{project.name}
|
||||
{/if}
|
||||
</h1>
|
||||
{/if}
|
||||
<div class="views" role="group" aria-label="Visning">
|
||||
{#each [['list', 'Liste'], ['board', 'Board'], ['calendar', 'Kalender']] as [value, label]}
|
||||
@@ -287,7 +300,21 @@
|
||||
overflow-wrap: anywhere;
|
||||
}
|
||||
|
||||
/* Reads as the heading it replaces until you put the cursor in it. */
|
||||
/* The heading's button carries no button-ness: it is the heading, and the only
|
||||
hint that it does anything is the underline on hover. */
|
||||
h1.renameable button {
|
||||
font: inherit;
|
||||
letter-spacing: inherit;
|
||||
color: inherit;
|
||||
text-align: left;
|
||||
border-bottom: 1px solid transparent;
|
||||
}
|
||||
|
||||
h1.renameable button:hover {
|
||||
border-bottom-color: var(--line-strong);
|
||||
}
|
||||
|
||||
/* Matches the heading it stands in for, so the swap does not move the page. */
|
||||
.title {
|
||||
flex: 1;
|
||||
min-width: 0;
|
||||
|
||||
Reference in New Issue
Block a user