Let a signup page's first Tab start at the top of the page

CHANGELOG.md

@@ -76,6 +76,7 @@ All notable changes to OpenSignup are documented here. Format follows [Keep a Ch
 - CI's actions now run on Node.js 24 (#658): `actions/checkout`, `actions/setup-node` and `actions/upload-artifact` move from v4 to v7, and `pnpm/action-setup` from v4 to v6, so the "Node.js 20 is deprecated" warning no longer shows on every run and the workflow keeps working once GitHub drops Node 20. The steps, their inputs, the Node version and pnpm's version (still read from `packageManager`) are unchanged. GitHub-hosted runners need nothing; if you run this workflow on your own runners, they need Actions Runner v2.327.1 or later.
 
 ### Fixed
+- On a signup's Build, Responses and Settings tabs, a keyboard user's first Tab now lands at the top of the page, in the header, as it does on the dashboard (#644). It used to skip the whole header and land on the tab after the current one, so reaching **Publish and share** took two Shift+Tabs back. Keeping the current tab in view on a phone scrolled it into view in a way that also moved where the browser starts the next Tab; the tab row now scrolls itself instead.
 - A new signup's web address, and the ref of a new field or slot, now spell out letters that have no accent to strip (#641). ø and æ, œ, ß, ł, đ, þ and ð were dropped or split the word, so "Smørrebrød" became `sm-rrebr-d`, "Æblekage" `blekage` and "Straße" `stra-e`. They now read `smorrebrod`, `aeblekage` and `strasse`. Existing links and refs don't change. Titles in other scripts, such as Cyrillic or Chinese, still fall back to `signup`, `field` or `slot`.
 - A signup can no longer be created or renamed with a title that is only spaces (#425). The title is trimmed before its 2-character minimum is checked, so "   " is refused and "  Bake sale  " is saved as "Bake sale", on the New signup form, through `POST /api/signups` and `PATCH /api/signups/[id]`, and through the AI connector's `create_signup` and `update_signup`. The 120-character limit still counts the title as sent. Before, a create saved it as an empty title, and a rename saved the spaces as they were. A title that is too short now comes back with "Title must be at least 2 characters.", the Build tab's words, on the form, from the API and to a connected assistant, rather than "String must contain at least 2 character(s)". A Magic Compose draft whose title is blank is refused as other drafts that don't fit are, rather than saved with no title.
 - A keyboard-focused Build, Responses or Settings tab on a signup's page now shows its whole focus ring (#586). The tab row scrolls sideways on a narrow screen, and that cut off the ring's top and bottom edges, and on a computer the Build tab's left edge too, so a keyboard user saw two short lines instead of a box. The ring is now drawn just inside each tab. Nothing else about the tabs changes.

src/components/signup/BuildBridge.test.tsx

@@ -11,9 +11,6 @@ vi.mock('next/navigation', () => ({ useRouter: () => router, usePathname: () =>
 
 const BUILD = '/app/signups/sig_1/build';
 
-// TabsNav scrolls its tab into view, which jsdom can't.
-Element.prototype.scrollIntoView = () => {};
-
 type Bridge = ReturnType<typeof useBuildBridge>;
 type Tab = Parameters<typeof useRegisterBuildTab>[0];
 

src/components/signup/TabsNav.test.tsx

@@ -0,0 +1,60 @@
+// @vitest-environment jsdom
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { render, screen } from '@testing-library/react';
+import { TabsNav } from './TabsNav';
+
+const nav = vi.hoisted(() => ({ pathname: '/app/signups/sig_1/build' }));
+vi.mock('next/navigation', () => ({ usePathname: () => nav.pathname }));
+
+// jsdom lays nothing out, so each test gives the strip and its tabs the sizes
+// a phone would: a 300px strip whose tabs are 120px wide, side by side.
+const STRIP = { offsetLeft: 16, clientWidth: 300 };
+const TAB_WIDTH = 120;
+const TAB_LEFT: Record<string, number> = { Build: 16, Responses: 136, Settings: 256 };
+
+function layOut() {
+  vi.spyOn(HTMLElement.prototype, 'offsetLeft', 'get').mockImplementation(function (
+    this: HTMLElement,
+  ) {
+    if (this.getAttribute('role') === 'tablist') return STRIP.offsetLeft;
+    return TAB_LEFT[this.textContent?.replace(/\d+$/, '') ?? ''] ?? 0;
+  });
+  vi.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockReturnValue(TAB_WIDTH);
+  vi.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockReturnValue(STRIP.clientWidth);
+}
+
+afterEach(() => {
+  vi.restoreAllMocks();
+  nav.pathname = '/app/signups/sig_1/build';
+});
+
+describe('TabsNav', () => {
+  it('scrolls the strip, not the page, so the first Tab still starts at the top', () => {
+    // A scrollIntoView moves where the browser's next Tab starts from, so the
+    // first Tab after the page loads would skip the whole header.
+    const scrollIntoView = vi.fn();
+    Element.prototype.scrollIntoView = scrollIntoView;
+    layOut();
+    render(<TabsNav signupId="sig_1" counts={{ build: 2, responses: 0 }} />);
+    expect(scrollIntoView).not.toHaveBeenCalled();
+    // @ts-expect-error -- jsdom has none; put back as it was.
+    delete Element.prototype.scrollIntoView;
+  });
+
+  it('centres the current tab in the strip', () => {
+    nav.pathname = '/app/signups/sig_1/responses';
+    layOut();
+    render(<TabsNav signupId="sig_1" counts={{ build: 2, responses: 0 }} />);
+    // Responses starts 120px into the strip; centring it in 300px leaves 90px
+    // either side.
+    expect(screen.getByRole('tablist').scrollLeft).toBe(136 - 16 - (300 - 120) / 2);
+  });
+
+  it('scrolls again when the tab changes', () => {
+    layOut();
+    const { rerender } = render(<TabsNav signupId="sig_1" counts={{ build: 2, responses: 0 }} />);
+    nav.pathname = '/app/signups/sig_1/settings';
+    rerender(<TabsNav signupId="sig_1" counts={{ build: 2, responses: 0 }} />);
+    expect(screen.getByRole('tablist').scrollLeft).toBe(256 - 16 - (300 - 120) / 2);
+  });
+});

src/components/signup/TabsNav.tsx

@@ -23,6 +23,7 @@ type TabId = (typeof TABS)[number]['id'];
 
 export function TabsNav({ signupId, counts }: TabsNavProps) {
   const pathname = usePathname();
+  const stripRef = useRef<HTMLDivElement | null>(null);
   const activeRef = useRef<HTMLAnchorElement | null>(null);
   // A plain primary click on a tab other than Build, even the one showing,
   // means the organizer has moved on from a check's fix still on its way to
@@ -36,12 +37,21 @@ export function TabsNav({ signupId, counts }: TabsNavProps) {
     return match ? match.id : 'build';
   })();
 
+  // Keeps the current tab in view when the strip scrolls sideways on a phone.
+  // This scrolls the strip itself: scrollIntoView would also move where the
+  // browser starts the next Tab from, so a keyboard user's first Tab would
+  // skip the whole header and land on the tab after this one.
   useEffect(() => {
-    activeRef.current?.scrollIntoView({ inline: 'center', block: 'nearest' });
+    const strip = stripRef.current;
+    const tab = activeRef.current;
+    if (!strip || !tab) return;
+    strip.scrollLeft =
+      tab.offsetLeft - strip.offsetLeft - (strip.clientWidth - tab.offsetWidth) / 2;
   }, [activeTab]);
 
   return (
     <div
+      ref={stripRef}
       role="tablist"
       aria-label="Signup sections"
       className="-mx-4 mb-2 flex overflow-x-auto border-b border-surface-sunk px-4 sm:mx-0 sm:px-0 [scrollbar-width:none] [&::-webkit-scrollbar]:hidden"