From 59a17565294926aff3a88dacf18e614a7df015e1 Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Tue, 8 Sep 2026 23:08:26 +0200
Subject: [PATCH] style(admin): lay the admin panel out in two columns and
 match the banner buttons
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The User Defaults and Notifications tabs were one narrow column of cards on a
page that has no max width, so the right two thirds stayed empty while the page
scrolled for ages. Both now use the two-column grid the Settings tab already
has, grouped by subject rather than by height.

User Defaults splits into "Default User Settings" (colour mode, units, time
format, currency, booking codes) and a "Map" card holding template, keys,
routing base, preview and engine. The columns are deliberately uneven — the left
one never needs more than its widest option row, the right one holds four fields
whose values are 60-character tile URLs.

Notifications keeps the three channel switches together on the left, because they
all write the same notification_channels list, and puts the operator's own
targets on the right. The admin preference matrix moves in with them instead of
running full width underneath: it decides which of those channels each admin-only
event goes out over. Its label column gets minmax(0,1fr) so it wraps rather than
overflowing in the narrower column.

The transit provider was the one native <select> left on the Settings tab. The
option list of a native select is drawn by the OS, so it followed neither the
colour scheme nor the radius nor the user's text-size setting; it now uses the
CustomSelect the rest of the app uses, and skips the request when the option
already selected is picked again.

In the update banner, "View on GitHub" was a size smaller than "How to Update" on
every axis — 12px against 14px, a 14px icon against 16px, less padding, and 2px
shorter. Two buttons side by side read as one group, so that reads as a mistake
rather than as hierarchy. Same box now; the fill, the colour and the weight still
mark the primary action.
---
 .../Admin/DefaultUserSettingsTab.tsx          | 24 ++++++--
 client/src/pages/AdminPage.test.tsx           | 58 ++++++++++++++++---
 .../pages/admin/AdminNotificationsPanel.tsx   |  4 +-
 .../admin/AdminNotificationsTab.test.tsx      | 14 ++++-
 .../src/pages/admin/AdminNotificationsTab.tsx | 22 ++++++-
 client/src/pages/admin/AdminSettingsTab.tsx   | 28 ++++++---
 client/src/pages/admin/AdminUpdateBanner.tsx  | 15 +++--
 7 files changed, 134 insertions(+), 31 deletions(-)

diff --git a/client/src/components/Admin/DefaultUserSettingsTab.tsx b/client/src/components/Admin/DefaultUserSettingsTab.tsx
index f5a619252..b800c3026 100644
--- a/client/src/components/Admin/DefaultUserSettingsTab.tsx
+++ b/client/src/components/Admin/DefaultUserSettingsTab.tsx
@@ -1,5 +1,5 @@
 import React, { useEffect, useMemo, useState } from 'react'
-import { Settings2 } from 'lucide-react'
+import { Map as MapIcon, Settings2 } from 'lucide-react'
 import { adminApi } from '../../api/client'
 import { useTranslation } from '../../i18n'
 import { useToast } from '../shared/Toast'
@@ -223,11 +223,23 @@ export default function DefaultUserSettingsTab(): React.ReactElement {
   }
 
   return (
-    <Section title={t('admin.defaultSettings.title')} icon={Settings2}>
-      <p className="text-sm text-content-faint" style={{ marginTop: -8 }}>
+    <div>
+      <p className="text-sm text-content-faint mb-4">
         {t('admin.defaultSettings.description')}
       </p>
 
+      {/* Two columns from xl up, the same idea as the Settings tab: as one flat list
+          the map fields sat a screen below the units while the right half of the page
+          stayed empty. Grouped by subject rather than by height — what a user ends up
+          seeing on the left, everything that configures the map on the right.
+          The columns are deliberately uneven: the left one never needs more than the
+          widest option row, while the right one holds four text fields whose values
+          are 60-character tile URLs. Only the column gap is set, because Section
+          carries its own bottom margin and a row gap would double it once the grid
+          collapses. Two cards can sit straight in the grid; a third would need the
+          explicit per-column stacks the Settings tab uses, or it leaves a hole. */}
+      <div className="grid grid-cols-1 gap-x-6 xl:grid-cols-[minmax(0,1fr)_minmax(0,1.35fr)] xl:items-start">
+      <Section title={t('admin.defaultSettings.title')} icon={Settings2}>
       {/* Color Mode */}
       <OptionRow label={<>{t('settings.colorMode')} <ResetButton field="dark_mode" /></>}>
         {([
@@ -325,7 +337,9 @@ export default function DefaultUserSettingsTab(): React.ReactElement {
           </OptionButton>
         ))}
       </OptionRow>
+      </Section>
 
+      <Section title={t('settings.map')} icon={MapIcon}>
       {/* Map Tile URL */}
       <div>
         <label className="block text-sm font-medium mb-1.5 text-content-secondary">
@@ -520,6 +534,8 @@ export default function DefaultUserSettingsTab(): React.ReactElement {
           </div>
         )}
       </div>
-    </Section>
+      </Section>
+      </div>
+    </div>
   )
 }
diff --git a/client/src/pages/AdminPage.test.tsx b/client/src/pages/AdminPage.test.tsx
index 1feb33220..f7792e158 100644
--- a/client/src/pages/AdminPage.test.tsx
+++ b/client/src/pages/AdminPage.test.tsx
@@ -517,6 +517,16 @@ describe('AdminPage', () => {
     });
   });
 
+  /**
+   * The transit-provider trigger. CustomSelect renders a plain button whose accessible
+   * name is the selected option, so it is found through its own card rather than by
+   * role and label the way the native select was.
+   */
+  function transitTrigger(): HTMLElement {
+    const block = screen.getByText('Transit Provider').closest<HTMLElement>('.rounded-xl');
+    return within(block!).getByRole('button');
+  }
+
   describe('FE-PAGE-ADMIN-023b: Transit provider select in Settings tab (#1699)', () => {
     it('choosing Google calls PUT /api/admin/transit-provider', async () => {
       let capturedBody: Record<string, unknown> | null = null;
@@ -535,12 +545,47 @@ describe('AdminPage', () => {
       await waitFor(() => expect(screen.getByRole('button', { name: /^users$/i })).toBeInTheDocument());
       fireEvent.click(screen.getByRole('button', { name: /settings/i }));
 
-      const select = await screen.findByRole('combobox', { name: /transit provider/i });
-      await waitFor(() => expect(select).toHaveValue('transitous'));
-      fireEvent.change(select, { target: { value: 'google' } });
+      await screen.findByText('Transit Provider');
+      await waitFor(() => expect(transitTrigger()).toHaveTextContent('Transitous'));
+      fireEvent.click(transitTrigger());
+      fireEvent.click(screen.getByText('Google'));
 
       await waitFor(() => expect(capturedBody).toEqual({ provider: 'google' }));
-      expect(select).toHaveValue('google');
+      expect(transitTrigger()).toHaveTextContent('Google');
+    });
+
+    it('FE-PAGE-ADMIN-023d: re-picking the provider already selected sends no request', async () => {
+      let puts = 0;
+      server.use(
+        http.get('/api/admin/transit-provider', () =>
+          HttpResponse.json({ provider: 'transitous', googleKeySource: 'instance' })),
+        http.put('/api/admin/transit-provider', () => {
+          puts += 1;
+          return HttpResponse.json({ provider: 'google', googleKeySource: 'instance' });
+        })
+      );
+
+      seedStore(useAuthStore, { isAuthenticated: true, user: buildAdmin() });
+      render(<AdminPage />);
+
+      await waitFor(() => expect(screen.getByRole('button', { name: /^users$/i })).toBeInTheDocument());
+      fireEvent.click(screen.getByRole('button', { name: /settings/i }));
+      await screen.findByText('Transit Provider');
+
+      // A real change first, so the counter is proven to move at all.
+      fireEvent.click(transitTrigger());
+      fireEvent.click(screen.getByText('Google'));
+      await waitFor(() => expect(puts).toBe(1));
+
+      // Picking the same option again must not send a second PUT. Trigger and menu
+      // entry carry the same label, so the entry is the one that is not the trigger.
+      const trigger = transitTrigger();
+      fireEvent.click(trigger);
+      const entry = screen.getAllByRole('button', { name: /^google$/i }).find(b => b !== trigger);
+      fireEvent.click(entry!);
+
+      await waitFor(() => expect(transitTrigger()).toHaveTextContent('Google'));
+      expect(puts).toBe(1);
     });
   });
 
@@ -554,7 +599,8 @@ describe('AdminPage', () => {
       render(<AdminPage />);
       await waitFor(() => expect(screen.getByRole('button', { name: /^users$/i })).toBeInTheDocument());
       fireEvent.click(screen.getByRole('button', { name: /settings/i }));
-      await screen.findByRole('combobox', { name: /transit provider/i });
+      await screen.findByText('Transit Provider');
+      await waitFor(() => expect(transitTrigger()).toHaveTextContent('Google'));
     }
 
     it('warns that Google is selected but no key is configured', async () => {
@@ -569,8 +615,6 @@ describe('AdminPage', () => {
 
     it('stays quiet when an instance-wide key resolves', async () => {
       await openSettingsWith('instance');
-      await waitFor(() =>
-        expect(screen.getByRole('combobox', { name: /transit provider/i })).toHaveValue('google'));
       expect(screen.queryByText(/no google api key is configured/i)).not.toBeInTheDocument();
       expect(screen.queryByText(/only your own google key is set/i)).not.toBeInTheDocument();
     });
diff --git a/client/src/pages/admin/AdminNotificationsPanel.tsx b/client/src/pages/admin/AdminNotificationsPanel.tsx
index a62072acd..50c6cbd41 100644
--- a/client/src/pages/admin/AdminNotificationsPanel.tsx
+++ b/client/src/pages/admin/AdminNotificationsPanel.tsx
@@ -70,7 +70,7 @@ export default function AdminNotificationsPanel({ t, toast }: { t: (k: string) =
         <div className="p-6">
           {saving && <p className="text-content-faint" style={{ fontSize: 'calc(11px * var(--fs-scale-caption, 1))', marginBottom: 8 }}>Saving…</p>}
           {/* Header row */}
-          <div className="border-b border-edge" style={{ display: 'grid', gridTemplateColumns: `1fr ${visibleChannels.map(() => '80px').join(' ')}`, gap: 4, paddingBottom: 6, marginBottom: 4 }}>
+          <div className="border-b border-edge" style={{ display: 'grid', gridTemplateColumns: `minmax(0, 1fr) ${visibleChannels.map(() => '80px').join(' ')}`, gap: 4, paddingBottom: 6, marginBottom: 4 }}>
             <span />
             {visibleChannels.map(ch => (
               <span key={ch} className="text-content-faint" style={{ fontSize: 'calc(11px * var(--fs-scale-caption, 1))', fontWeight: 600, textAlign: 'center', textTransform: 'uppercase', letterSpacing: '0.04em' }}>
@@ -82,7 +82,7 @@ export default function AdminNotificationsPanel({ t, toast }: { t: (k: string) =
           {matrix.event_types.map((eventType: string) => {
             const implementedForEvent = matrix.implemented_combos[eventType] ?? []
             return (
-              <div key={eventType} className="border-b border-edge" style={{ display: 'grid', gridTemplateColumns: `1fr ${visibleChannels.map(() => '80px').join(' ')}`, gap: 4, alignItems: 'center', padding: '8px 0' }}>
+              <div key={eventType} className="border-b border-edge" style={{ display: 'grid', gridTemplateColumns: `minmax(0, 1fr) ${visibleChannels.map(() => '80px').join(' ')}`, gap: 4, alignItems: 'center', padding: '8px 0' }}>
                 <span className="text-content" style={{ fontSize: 'calc(13px * var(--fs-scale-body, 1))' }}>
                   {t(ADMIN_EVENT_LABEL_KEYS[eventType]) || eventType}
                 </span>
diff --git a/client/src/pages/admin/AdminNotificationsTab.test.tsx b/client/src/pages/admin/AdminNotificationsTab.test.tsx
index 35ce31c0e..929fc87db 100644
--- a/client/src/pages/admin/AdminNotificationsTab.test.tsx
+++ b/client/src/pages/admin/AdminNotificationsTab.test.tsx
@@ -1,4 +1,4 @@
-// FE-ADMNOT-001 to FE-ADMNOT-041
+// FE-ADMNOT-001 to FE-ADMNOT-043
 import { http, HttpResponse } from 'msw';
 import React from 'react';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
@@ -559,4 +559,16 @@ describe('AdminNotificationsTab', () => {
 
     await waitFor(() => expect(body).toEqual({ smtp_host: 'mail.example.com', smtp_pass: 'hunter2' }));
   });
+
+  it('FE-ADMNOT-043: a managed install keeps the user channels and drops the operator cards', () => {
+    // The second column is nothing but !managed cards, so the layout falls back to a
+    // single stack there instead of leaving half a grid row empty.
+    renderTab({ managed: true });
+
+    expect(screen.getByRole('heading', { name: /^webhook$/i })).toBeInTheDocument();
+    expect(screen.getByRole('heading', { name: 'Trip Reminders' })).toBeInTheDocument();
+    expect(screen.queryByRole('heading', { name: 'Email (SMTP)' })).not.toBeInTheDocument();
+    expect(screen.queryByRole('heading', { name: 'Admin Webhook' })).not.toBeInTheDocument();
+    expect(screen.queryByRole('heading', { name: 'Admin Ntfy' })).not.toBeInTheDocument();
+  });
 });
diff --git a/client/src/pages/admin/AdminNotificationsTab.tsx b/client/src/pages/admin/AdminNotificationsTab.tsx
index 536bd3a3e..7f792d64c 100644
--- a/client/src/pages/admin/AdminNotificationsTab.tsx
+++ b/client/src/pages/admin/AdminNotificationsTab.tsx
@@ -57,7 +57,19 @@ export default function AdminNotificationsTab({ admin, t }: AdminNotificationsTa
   }
 
   return (<>
-    <div className="space-y-4">
+    {/* Two columns from xl up, the same shape as the Settings tab. Most of these
+        cards are a title with a switch on the far right, so on a wide screen the
+        middle stayed empty while the page still scrolled past the three tall ones.
+        Two explicit columns rather than a grid over the flat list: a plain grid
+        pairs cards row by row and leaves a hole under the shorter one, and the SMTP
+        card is taller than all the toggle rows together. Grouped by audience, not by
+        height — the channels a user can receive on the left, everything the operator
+        sends or receives themselves on the right. The three channel switches stay
+        together because they all write the same notification_channels list.
+        On a managed install the two admin-target cards are gone and the right column
+        is the matrix alone — still a column, so the grid keeps working. */}
+    <div className="grid grid-cols-1 gap-6 xl:grid-cols-2 xl:items-start">
+      <div className="space-y-6">
       {/* The relay is the operator's: their host, their credential, their sending
           reputation. An instance that could point it elsewhere would send under a
           domain it does not own. */}
@@ -218,7 +230,9 @@ export default function AdminNotificationsTab({ admin, t }: AdminNotificationsTa
           </button>
         </div>
       </div>
+      </div>
 
+      <div className="space-y-6">
       {/* Admin alerts are about running the instance (version notices, and what else
           lands there later). On a managed install those go to whoever runs it. */}
       {!managed && (<>
@@ -375,9 +389,11 @@ export default function AdminNotificationsTab({ admin, t }: AdminNotificationsTa
       </div>
       </>)}
 
-    </div>
-    <div className="mt-6">
+      {/* The matrix decides which of those channels each admin-only event goes out
+          over, so it belongs under the targets it routes to rather than across the
+          full width below both columns. */}
       <AdminNotificationsPanel t={t} toast={toast} />
+      </div>
     </div>
   </>)
 }
diff --git a/client/src/pages/admin/AdminSettingsTab.tsx b/client/src/pages/admin/AdminSettingsTab.tsx
index 5a27f89ca..b5e2cd7c1 100644
--- a/client/src/pages/admin/AdminSettingsTab.tsx
+++ b/client/src/pages/admin/AdminSettingsTab.tsx
@@ -3,6 +3,7 @@ import { adminApi, authApi } from '../../api/client'
 import { getApiErrorMessage } from '../../types'
 import { Eye, EyeOff, Save, CheckCircle, XCircle, Loader2, RefreshCw, AlertTriangle } from 'lucide-react'
 import ToggleSwitch from '../../components/Settings/ToggleSwitch'
+import CustomSelect from '../../components/shared/CustomSelect'
 import GoogleOptions from './GoogleOptions'
 import ProviderBlock from './ProviderBlock'
 import TrekApiCard from './TrekApiCard'
@@ -553,11 +554,18 @@ export default function AdminSettingsTab({ admin, t }: AdminSettingsTabProps): R
           <ProviderBlock title={t('admin.transitProvider.title')}>
             <div>
               <p className="text-xs text-content-faint">{t('admin.transitProvider.subtitle')}</p>
-              <select
+              {/* The app's own select rather than the browser's: a native option list
+                  is drawn by the OS, so it ignores the scheme, the radius and the
+                  text-size setting the rest of this card follows. The menu portals to
+                  the body, which is what lets it escape the card's overflow-hidden. */}
+              <CustomSelect
                 value={transitProvider}
-                aria-label={t('admin.transitProvider.title')}
-                onChange={async e => {
-                  const next = e.target.value === 'google' ? 'google' : 'transitous'
+                onChange={async value => {
+                  // A native select stayed silent when the option already selected was
+                  // picked again; this one reports every pick, and a no-op PUT is still
+                  // a write on an audited settings route.
+                  if (value === transitProvider) return
+                  const next = value === 'google' ? 'google' : 'transitous'
                   const previous = transitProvider
                   setTransitProviderState(next)
                   try {
@@ -565,11 +573,13 @@ export default function AdminSettingsTab({ admin, t }: AdminSettingsTabProps): R
                     setTransitGoogleKeySource(saved.googleKeySource)
                   } catch { setTransitProviderState(previous) }
                 }}
-                className="mt-2 w-full px-3 py-2 border border-edge rounded-lg text-sm bg-surface-input text-content focus:ring-2 focus:ring-accent focus:border-transparent"
-              >
-                <option value="transitous">{t('admin.transitProvider.transitous')}</option>
-                <option value="google">{t('admin.transitProvider.google')}</option>
-              </select>
+                options={[
+                  { value: 'transitous', label: t('admin.transitProvider.transitous') },
+                  { value: 'google', label: t('admin.transitProvider.google') },
+                ]}
+                size="sm"
+                style={{ marginTop: 8 }}
+              />
               <p className="text-xs text-content-faint mt-1.5">
                 {transitProvider === 'google' ? t('admin.transitProvider.googleHint') : t('admin.transitProvider.transitousHint')}
               </p>
diff --git a/client/src/pages/admin/AdminUpdateBanner.tsx b/client/src/pages/admin/AdminUpdateBanner.tsx
index 79c042d2c..eb20ad6c8 100644
--- a/client/src/pages/admin/AdminUpdateBanner.tsx
+++ b/client/src/pages/admin/AdminUpdateBanner.tsx
@@ -10,7 +10,7 @@ interface AdminUpdateBannerProps {
 }
 
 // The "new version available" banner shown at the top of the admin page.
-// Purely presentational — extracted from AdminPage with identical markup.
+// Purely presentational — extracted from AdminPage.
 export default function AdminUpdateBanner({ updateInfo, t, onHowTo }: AdminUpdateBannerProps): React.ReactElement {
   return (
     <div className="mb-6 p-4 rounded-xl border flex flex-col sm:flex-row items-start sm:items-center gap-4 bg-amber-50 dark:bg-amber-950/40 border-amber-300 dark:border-amber-700">
@@ -25,23 +25,28 @@ export default function AdminUpdateBanner({ updateInfo, t, onHowTo }: AdminUpdat
           </p>
         </div>
       </div>
+      {/* Both controls carry the same box metrics — padding, text size, icon, and a
+          border the filled one only needs to stop the outlined one from ending up
+          2px shorter. Two buttons side by side read as one group, so a size gap
+          reads as a mistake rather than as hierarchy; the fill, the colour and the
+          weight are what mark the primary action. */}
       <div className="flex items-center gap-2 flex-shrink-0">
         {updateInfo.release_url && (
           <a
             href={updateInfo.release_url}
             target="_blank"
             rel="noopener noreferrer"
-            className="flex items-center gap-1.5 px-3 py-2 rounded-lg text-xs font-medium transition-colors text-amber-800 dark:text-amber-300 border border-amber-300 dark:border-amber-600 hover:bg-amber-100 dark:hover:bg-amber-900/50"
+            className="flex items-center gap-1.5 px-4 py-2 rounded-lg text-sm font-medium transition-colors text-amber-800 dark:text-amber-300 border border-amber-300 dark:border-amber-600 hover:bg-amber-100 dark:hover:bg-amber-900/50"
           >
-            <ExternalLink className="w-3.5 h-3.5" />
+            <ExternalLink className="w-4 h-4 flex-shrink-0" />
             {t('admin.update.button')}
           </a>
         )}
         <button type="button"
           onClick={onHowTo}
-          className="flex items-center gap-1.5 px-4 py-2 rounded-lg text-sm font-semibold transition-colors bg-slate-900 dark:bg-white text-white dark:text-slate-900 hover:bg-slate-700 dark:hover:bg-gray-200"
+          className="flex items-center gap-1.5 px-4 py-2 rounded-lg text-sm font-semibold transition-colors border border-transparent bg-slate-900 dark:bg-white text-white dark:text-slate-900 hover:bg-slate-700 dark:hover:bg-gray-200"
         >
-          <Download className="w-4 h-4" />
+          <Download className="w-4 h-4 flex-shrink-0" />
           {t('admin.update.howTo')}
         </button>
       </div>