From 7ab68b0b58d81dc18c8bba0f6d0725b6b21b0154 Mon Sep 17 00:00:00 2001
From: Kristian Wind <kw@KW-MacBook.local>
Date: Mon, 17 Aug 2026 15:07:15 +0200
Subject: [PATCH] fix(projects): keep the heading, and make the sidebar test
 mean what it says
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 web/e2e/smoke.spec.js                    | 12 +++++-
 web/src/routes/projekt/[id]/+page.svelte | 49 ++++++++++++++++++------
 2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/web/e2e/smoke.spec.js b/web/e2e/smoke.spec.js
index 9855b8e..8aace00 100644
--- a/web/e2e/smoke.spec.js
+++ b/web/e2e/smoke.spec.js
@@ -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();
diff --git a/web/src/routes/projekt/[id]/+page.svelte b/web/src/routes/projekt/[id]/+page.svelte
index 64c7606..7943362 100644
--- a/web/src/routes/projekt/[id]/+page.svelte
+++ b/web/src/routes/projekt/[id]/+page.svelte
@@ -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;