From 11ce3cfb86149d00ffc92a27c916673929a31e37 Mon Sep 17 00:00:00 2001
From: Knacky <knckydev@gmail.com>
Date: Sun, 21 Jun 2026 22:20:51 +0200
Subject: [PATCH] =?UTF-8?q?fix(frontend):=20code-review=20polish=20?=
 =?UTF-8?q?=E2=80=94=20replaceState,=20arrow-key=20nav,=20TabId,=20comment?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fix 4: useHashTab navigate() uses history.replaceState instead of
window.location.hash assignment — no spurious history entries, no
anchor-jump scroll side-effect.
Fix 5: Tabs ArrowLeft/ArrowRight keyboard nav (WAI-ARIA tabs pattern).
Fix 6: TabId union type in EngagementDetailPage, cast from string for
type-safe switch on activeTab without breaking Tabs.onChange signature.
Fix 7: intentional comment on UsersAdminPage reset-password aerated row.
Tests: 236/236 (+2 arrow-key assertions in Tabs.test.tsx).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 frontend/src/components/Tabs.tsx            | 15 ++++++++++++++-
 frontend/src/hooks/useHashTab.ts            |  3 ++-
 frontend/src/pages/EngagementDetailPage.tsx |  5 ++++-
 frontend/src/pages/UsersAdminPage.tsx       |  1 +
 frontend/tests/components/Tabs.test.tsx     | 14 ++++++++++++++
 frontend/tests/hooks/useHashTab.test.tsx    |  2 +-
 6 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/frontend/src/components/Tabs.tsx b/frontend/src/components/Tabs.tsx
index 3bf5831..55d70f8 100644
--- a/frontend/src/components/Tabs.tsx
+++ b/frontend/src/components/Tabs.tsx
@@ -1,3 +1,5 @@
+import type { KeyboardEvent } from 'react';
+
 interface TabItem {
   id: string;
   label: string;
@@ -11,12 +13,22 @@ interface TabsProps {
 }
 
 export function Tabs({ items, activeId, onChange }: TabsProps): JSX.Element {
+  function handleKeyDown(e: KeyboardEvent<HTMLButtonElement>, index: number) {
+    if (e.key === 'ArrowRight') {
+      e.preventDefault();
+      onChange(items[(index + 1) % items.length].id);
+    } else if (e.key === 'ArrowLeft') {
+      e.preventDefault();
+      onChange(items[(index - 1 + items.length) % items.length].id);
+    }
+  }
+
   return (
     <div
       role="tablist"
       className="flex items-end border-b border-hairline gap-xs"
     >
-      {items.map((item) => {
+      {items.map((item, index) => {
         const isActive = item.id === activeId;
         return (
           <button
@@ -27,6 +39,7 @@ export function Tabs({ items, activeId, onChange }: TabsProps): JSX.Element {
             aria-controls={`tabpanel-${item.id}`}
             type="button"
             onClick={() => onChange(item.id)}
+            onKeyDown={(e) => handleKeyDown(e, index)}
             className={`tab-underline${isActive ? ' tab-underline-active' : ''} pb-xs`}
           >
             {item.label}
diff --git a/frontend/src/hooks/useHashTab.ts b/frontend/src/hooks/useHashTab.ts
index e7330bd..106b161 100644
--- a/frontend/src/hooks/useHashTab.ts
+++ b/frontend/src/hooks/useHashTab.ts
@@ -17,7 +17,8 @@ export function useHashTab(defaultId: string): [string, (id: string) => void] {
   }, [defaultId]);
 
   const navigate = useCallback((id: string) => {
-    window.location.hash = id;
+    // replaceState: no history entry (Back button unaffected), no anchor-jump scroll
+    history.replaceState(null, '', '#' + id);
     setActiveId(id);
   }, []);
 
diff --git a/frontend/src/pages/EngagementDetailPage.tsx b/frontend/src/pages/EngagementDetailPage.tsx
index ad9ce65..47ca46c 100644
--- a/frontend/src/pages/EngagementDetailPage.tsx
+++ b/frontend/src/pages/EngagementDetailPage.tsx
@@ -13,6 +13,8 @@ import { BackLink } from '@/components/BackLink';
 import { Tabs } from '@/components/Tabs';
 import { Link } from 'react-router-dom';
 
+type TabId = 'schedule' | 'description' | 'simulations';
+
 export function EngagementDetailPage(): JSX.Element {
   const { id } = useParams<{ id: string }>();
   const numericId = id ? Number(id) : undefined;
@@ -21,7 +23,8 @@ export function EngagementDetailPage(): JSX.Element {
   const detail = useEngagement(numericId);
   const simsQuery = useEngagementSimulations(numericId);
 
-  const [activeTab, setActiveTab] = useHashTab('schedule');
+  const [activeTabRaw, setActiveTab] = useHashTab('schedule');
+  const activeTab = activeTabRaw as TabId;
 
   if (detail.isLoading) return <LoadingState label="Loading engagement…" />;
   if (detail.isError) {
diff --git a/frontend/src/pages/UsersAdminPage.tsx b/frontend/src/pages/UsersAdminPage.tsx
index d03fe16..46e9eb1 100644
--- a/frontend/src/pages/UsersAdminPage.tsx
+++ b/frontend/src/pages/UsersAdminPage.tsx
@@ -248,6 +248,7 @@ export function UsersAdminPage(): JSX.Element {
                       </tr>
                       {resetOpen === u.id ? (
                         <tr className="border-b border-hairline last:border-0 bg-cloud">
+                          {/* Aerated row — inline form needs room; compact-density tradeoff intentional */}
                           <td colSpan={4} className="px-xl py-md">
                             <form
                               onSubmit={(e) => onResetPassword(u, e)}
diff --git a/frontend/tests/components/Tabs.test.tsx b/frontend/tests/components/Tabs.test.tsx
index d0c660b..927e107 100644
--- a/frontend/tests/components/Tabs.test.tsx
+++ b/frontend/tests/components/Tabs.test.tsx
@@ -59,6 +59,20 @@ describe('Tabs', () => {
     expect(simsBtn).toHaveAttribute('aria-controls', 'tabpanel-simulations');
   });
 
+  it('ArrowRight moves focus to the next tab', () => {
+    const onChange = vi.fn();
+    render(<Tabs items={ITEMS} activeId="schedule" onChange={onChange} />);
+    fireEvent.keyDown(screen.getByRole('tab', { name: /Schedule/i }), { key: 'ArrowRight' });
+    expect(onChange).toHaveBeenCalledWith('description');
+  });
+
+  it('ArrowLeft wraps around to the last tab', () => {
+    const onChange = vi.fn();
+    render(<Tabs items={ITEMS} activeId="schedule" onChange={onChange} />);
+    fireEvent.keyDown(screen.getByRole('tab', { name: /Schedule/i }), { key: 'ArrowLeft' });
+    expect(onChange).toHaveBeenCalledWith('simulations');
+  });
+
   it('has no rounded-md, transition-*, or shadow-* on tab buttons (brutalism)', () => {
     render(<Tabs items={ITEMS} activeId="schedule" onChange={vi.fn()} />);
     for (const btn of screen.getAllByRole('tab')) {
diff --git a/frontend/tests/hooks/useHashTab.test.tsx b/frontend/tests/hooks/useHashTab.test.tsx
index 5a8ca5f..21a37e2 100644
--- a/frontend/tests/hooks/useHashTab.test.tsx
+++ b/frontend/tests/hooks/useHashTab.test.tsx
@@ -22,7 +22,7 @@ describe('useHashTab', () => {
     expect(result.current[0]).toBe('simulations');
   });
 
-  it('navigate() updates activeId and sets location.hash', () => {
+  it('navigate() updates activeId and sets hash via replaceState (no history entry)', () => {
     const { result } = renderHook(() => useHashTab('schedule'));
     act(() => {
       result.current[1]('description');