[richshaw]

Fixes #644.

The problem

On a signup's Build, Responses and Settings tabs, a keyboard user's first Tab skipped the whole header (breadcrumb, Help, account, Preview, Publish and share, ⋯, the public link's buttons) and landed on the tab after the current one. TabsNav keeps the current tab in view on a phone by calling scrollIntoView on it when the page loads and when the tab changes. In Chrome that also moves where the next Tab starts from, as a click on the tab would.

The fix

• TabsNav (src/components/signup/TabsNav.tsx) now scrolls the strip itself. A ref on the role="tablist" element, and the effect sets strip.scrollLeft = tab.offsetLeft - strip.offsetLeft - (strip.clientWidth - tab.offsetWidth) / 2, which centres the current tab as inline: 'center' did. The issue's outline gave this formula. The strip isn't positioned, so the strip and its tabs share an offsetParent and the two offsetLefts compare. On a computer, where nothing overflows, the browser clamps the value to 0 and nothing moves.
• BuildBridge.test.tsx no longer stubs Element.prototype.scrollIntoView, since nothing calls it now.
• CHANGELOG entry under Fixed.
• No help page names this behaviour, so no help change.

Tests

• New: src/components/signup/TabsNav.test.tsx, with phone-sized layout stubbed in jsdom:

  • scrollIntoView is never called;
  • the current tab (Responses) is centred in the strip;
  • the strip scrolls again when the tab changes.

  All three failed before the fix, each for the expected reason.

• Passed on 89e7c58: pnpm lint && pnpm typecheck && pnpm knip && pnpm test (1782), and pnpm build && pnpm budgets (Build at 159.4 kB, as on main). Client-only, so no pnpm test:db.

• Not run here: the first Tab in a real browser. The tophat checks that at 1280 and 390.

---

Generated by Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

[richshaw]

Tophat: passed

Ran c82ad8f locally with pnpm dev, Postgres 16, and Chromium at 1280×900 and 390×844 (phone with touch on), plus 280×844 so the tab strip actually overflows. At 390 the three tabs fit, so nothing scrolls there. I didn't run the worker, since nothing here touches jobs. The data was made up: a draft "Snack duty — Spring season" with one slot. For the "before" rows I ran the same Playwright script on the PR's base, main at d093d0d. For each tab the script loads the page fresh, reads the strip's scrollLeft, checks the current tab sits fully inside the strip, then presses Tab three times and reads document.activeElement after each.

Check	Result	What I saw
Before (main): first Tab on each tab	✅ bug reproduced	Build → first stop was the Responses tab. Responses → Settings tab. Settings → the title box under the tabs. The header was skipped every time, at all three widths
After: first Tab on Build, Responses, Settings	✅	The first three stops were OpenSignup, Help and the account link on every tab, at 1280, 390 and 280
After: current tab kept in view on a narrow phone (280)	✅	scrollLeft was 0 on Build, 30 on Responses (31 on main), 51 = max on Settings (as on main). The current tab was fully visible in the strip each time. The page itself never scrolled (scrollY 0)
After: strip follows an in-app tab change	✅	Clicking Settings from Build scrolled the strip to 51 at 280, as on main. Focus stayed on the clicked tab, as before
Neighbour: Build tab edits and tab round trip	✅	Opened the slot, set Spots 2 → 3, saw "Saved", Done, went to Responses and back to Build by the tabs, and the row read "0 of 3 taken"
Browser console and server log	✅	No console errors in the scripted runs. One driver run logged a single "Failed to load resource: 404"; I couldn't reproduce it on the PR or on main with a 404 listener on the same steps, and the server logged no 404, so I put it down to dev-server compiling. The server log's only errors were the health check's pgboss.job probe, which fails because the worker wasn't running

For the owner (UX): for keyboard users, the first Tab on a signup's Build, Responses and Settings tabs now starts at the top of the page (the OpenSignup link, Help, the account link, then the header's buttons) instead of jumping past the header to the next tab. Nothing changes for mouse or touch users: on a narrow phone the current tab is still centred in the strip as before, to within a pixel. Screenshots: 672-before-desktop-responses (on main, focus has run off the end after Settings), 672-after-desktop-responses and 672-after-phone-responses (focus ring on the header's account link after three Tabs), 672-before-narrow-responses / 672-after-narrow-responses and 672-after-narrow-settings (the strip scrolled to the current tab at 280), 672-after-desktop-build.

Screenshots are in https://claude.ai/code/session_01GSt2N12HQ5GfdLLtfoQui2.

---

Generated by Claude Code

[richshaw]

Review: passed (round 1)

TabsNav now centres the current tab by setting the strip's scrollLeft instead of calling scrollIntoView on the tab, so loading a signup's page no longer moves where the browser starts the next Tab from. It follows the issue's outline exactly. The strip and its tabs share an offsetParent (the strip isn't positioned), so the two offsetLefts compare. Negative values (Build) and values on a desktop where nothing overflows are clamped to 0 by the browser. The tophat measured the same scroll positions as main to within a pixel at 280px. The three new tests cover "no scrollIntoView", centring on load and re-scrolling on a tab change. Dropping the scrollIntoView stub from BuildBridge.test.tsx is safe because nothing in that tree calls it now. Every "Done when" item is met: the first Tab lands in the header on all three tabs at 1280 and 390, the current tab stays in view on a narrow phone, and CI ran the checks.

Blocking (0)
Optional (2):

• src/components/signup/TabsNav.test.tsx:35-41: the first test sets Element.prototype.scrollIntoView and deletes it at the end of the test body, so a failing assertion would leave the stub in place for the rest of the file. vi.restoreAllMocks in afterEach doesn't undo a direct assignment. Moving the delete into afterEach (or using vi.stubGlobal-style setup) would make it tidy. It's harmless today, since the other tests don't call it.
• The issue's "Watch for" asks about other scrollIntoView calls. The two left (BuildWysiwyg.tsx:256 when a slot editor opens, commit-dialog.tsx:177 when an error shows) only run after the user acts, not on page load, so they don't skip anything. Nothing to do.

Earlier threads: none. Copilot couldn't review (quota used up).
Checks: CI green on c82ad8f (lint-type-test, e2e, CodeQL). I read the diff and the surrounding component locally. The tophat passed on c82ad8f, and no runtime code has changed since (shared check: 0).

Why it waits for the owner (UX): it changes what a keyboard user gets on a signup's Build, Responses and Settings tabs. The first Tab now lands on the header (OpenSignup, Help, account, then Preview and Publish and share) instead of on the next tab. Mouse and touch users see no change. The tophat's screenshots show it: 672-before-desktop-responses against 672-after-desktop-responses and 672-after-phone-responses, plus 672-after-narrow-responses / 672-after-narrow-settings for the strip still scrolled to the current tab at 280px.

Session: https://claude.ai/code/session_01QaCVc9YbQkaTfPmA9c3K1j

---

Generated by Claude Code

[richshaw]

Approved by the owner at c82ad8f.

https://claude.ai/code/session_014LCbuCxaCSh9MudLd8Pz1H

---

Generated by Claude Code