style(admin): lay the admin panel out in two columns and match the banner buttons

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.
2026-10-18 21:16:47 +00:00 · 2026-09-08 23:08:26 +02:00
parent 4611f0ab7a
commit 59a1756529
7 changed files with 134 additions and 31 deletions
@@ -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>
  )
 }
@@ -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();
    });
@@ -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>
@@ -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();
+  });
 });
@@ -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>
  </>)
 }
@@ -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>
@@ -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>