From 34d8271521b9121c50cb478fc9c2cb8262deae5f Mon Sep 17 00:00:00 2001 From: Mao Nakamoto <41178744+maonakamoto@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:38:19 +0200 Subject: [PATCH] =?UTF-8?q?fix(projects):=20one=20element=20per=20anchor?= =?UTF-8?q?=20=E2=80=94=20tab=20panels=20own=20the=20hash?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Left over from the tab conversion. The sections had ids from when they were scroll anchors (`id="feedback"`, `id="settings"`, ...), and the new panels took ids of their own — so both existed for the same concept: a panel `panel-feedback` wrapping a section still called `feedback`. That is not cosmetic. Every panel stays mounted, because they hold unsaved drafts and poll while work is in flight. So loading /projects/#feedback handed the browser a real element to scroll to that was inside a HIDDEN panel — native anchor behaviour racing the tab logic over the same name. ControlInbox and FeedbackItemRow both link exactly that way, so it is the common path, not an edge case. The panel is the anchor now: `id={tab.id}`, `aria-controls={tab.id}`, `scroll-mt-28` to clear the sticky tab bar. The six nested ids are gone. What the browser scrolls to is what becomes visible. Pinned by scripts/test/project-tab-anchors.ts rather than left to review: it reads the tab ids out of ProjectWorkspaceView, asserts the panel uses them, asserts no component under components/projects re-declares one as an element id, and asserts ControlInbox still deep-links to #feedback — the link that makes the invariant matter in the first place. Proven by mutation: reintroducing id="feedback" on the section turns exactly that one assertion red. --- scripts/test/project-tab-anchors.ts | 82 +++++++++++++++++++ .../projects/ProjectContextEditor.tsx | 2 +- .../projects/ProjectFeedbackSection.tsx | 2 +- .../projects/ProjectPlanSection.tsx | 2 +- .../projects/ProjectSettingsPanel.tsx | 2 +- src/components/projects/ProjectTabs.tsx | 15 +++- .../projects/ProjectWorkspaceView.tsx | 12 +-- 7 files changed, 100 insertions(+), 17 deletions(-) create mode 100644 scripts/test/project-tab-anchors.ts diff --git a/scripts/test/project-tab-anchors.ts b/scripts/test/project-tab-anchors.ts new file mode 100644 index 000000000..a1bb7bda0 --- /dev/null +++ b/scripts/test/project-tab-anchors.ts @@ -0,0 +1,82 @@ +// One element per anchor on the project page. +// +// The project page used to be a scroll of sections with `id="feedback"`, +// `id="settings"` and so on, linked from a jump-nav. Those sections became TAB +// PANELS, and the panels took ids of their own — so for a while BOTH existed: +// a panel `panel-feedback` and, inside it, a section still carrying the legacy +// `id="feedback"`. +// +// That is not cosmetic. Every panel stays mounted (they hold unsaved drafts), +// so loading /projects/#feedback handed the browser a real element to +// scroll to that lived inside a HIDDEN panel — native anchor behaviour racing +// the tab logic over the same name. ControlInbox and FeedbackItemRow both link +// that way, so it is the common path, not an edge case. +// +// The rule this pins: a tab id may exist exactly once in the DOM, on the panel. +// No section inside a panel may re-declare it. +// Run: npx tsx scripts/test/project-tab-anchors.ts +import { readFileSync, readdirSync } from "fs"; +import { join, dirname } from "path"; +import { fileURLToPath } from "url"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "..", ".."); +const PROJECTS_DIR = join(ROOT, "src/components/projects"); + +let pass = 0; +let fail = 0; +function ok(cond: boolean, label: string) { + if (cond) { + pass++; + } else { + fail++; + console.error(`✗ ${label}`); + } +} + +const view = readFileSync(join(PROJECTS_DIR, "ProjectWorkspaceView.tsx"), "utf8"); +const tabsSrc = readFileSync(join(PROJECTS_DIR, "ProjectTabs.tsx"), "utf8"); + +// The tab ids are declared in the ProjectTabs call in ProjectWorkspaceView. +const tabIds = [...view.matchAll(/^\s*id:\s*"([a-z-]+)",$/gm)].map((m) => m[1]); +ok(tabIds.length >= 5, `found the tab ids in ProjectWorkspaceView (got ${tabIds.length})`); +ok(tabIds.includes("feedback"), "feedback is a tab — it is what ControlInbox deep-links to"); + +// The panel must BE the anchor, so the element the browser scrolls to is the +// element that becomes visible. +ok( + /id=\{tab\.id\}/.test(tabsSrc), + "the tab panel uses id={tab.id} — the panel owns the hash, not a nested section", +); +ok( + !/id=\{`panel-\$\{tab\.id\}`\}/.test(tabsSrc), + "no `panel-` prefix: that is what created a second element for the same concept", +); +ok(/aria-controls=\{tab\.id\}/.test(tabsSrc), "aria-controls points at the panel's real id"); + +// No component rendered inside a panel may re-declare a tab id. +const files = readdirSync(PROJECTS_DIR).filter( + (f) => f.endsWith(".tsx") && f !== "ProjectTabs.tsx", +); +for (const id of tabIds) { + const offenders: string[] = []; + for (const f of files) { + const src = readFileSync(join(PROJECTS_DIR, f), "utf8"); + // Match a literal element id, not a string inside a comment or a template. + if (new RegExp(`\\sid="${id}"`).test(src)) offenders.push(f); + } + ok( + offenders.length === 0, + `no section re-declares id="${id}"${offenders.length ? ` — found in ${offenders.join(", ")}` : ""}`, + ); +} + +// The deep links that make this matter must still exist and still be bare +// hashes, since that is what the tablist reads on mount. +const inbox = readFileSync(join(ROOT, "src/components/control/ControlInbox.tsx"), "utf8"); +ok( + /\/projects\/\$\{[^}]+\}#feedback/.test(inbox), + "ControlInbox still deep-links to #feedback (the reason this invariant exists)", +); + +console.log(`${pass} passed, ${fail} failed`); +process.exit(fail === 0 ? 0 : 1); diff --git a/src/components/projects/ProjectContextEditor.tsx b/src/components/projects/ProjectContextEditor.tsx index f4d9e8fbe..86a44a1af 100644 --- a/src/components/projects/ProjectContextEditor.tsx +++ b/src/components/projects/ProjectContextEditor.tsx @@ -139,7 +139,7 @@ export function ProjectContextEditor({ const hasRepo = getProjectLinks(attrs, gitUrl).repo !== null; return ( -
+
diff --git a/src/components/projects/ProjectFeedbackSection.tsx b/src/components/projects/ProjectFeedbackSection.tsx index f9e039bb6..ad242c8cd 100644 --- a/src/components/projects/ProjectFeedbackSection.tsx +++ b/src/components/projects/ProjectFeedbackSection.tsx @@ -143,7 +143,7 @@ export function ProjectFeedbackSection({ } } return ( -
+

diff --git a/src/components/projects/ProjectPlanSection.tsx b/src/components/projects/ProjectPlanSection.tsx index 3238772bf..a1beef5cb 100644 --- a/src/components/projects/ProjectPlanSection.tsx +++ b/src/components/projects/ProjectPlanSection.tsx @@ -37,7 +37,7 @@ export function ProjectPlanSection({ const nextStep = answer(attrs.next_step); return ( -
+
+ {/* The panel owns the hash id, and the sections inside it no longer carry + one. Before this, both existed: the panel was `panel-feedback` while + the section inside kept the legacy `id="feedback"` from when these + were scroll anchors. Every panel stays mounted, so loading + /projects/#feedback gave the browser a real element to scroll to + that was inside a HIDDEN panel — native anchor behaviour racing the + tab logic over the same name. One element per anchor removes the race, + and what the browser scrolls to is now what becomes visible. + `scroll-mt-28` clears the sticky tab bar. */} {tabs.map((tab) => (