Fix #1303: wrap admin/admins advanced-search form in collapsible <details>

AGENTS.md

@@ -1271,7 +1271,8 @@ audit (#1207) locked in. New CTAs:
 | Render sub-views inside a Pattern A section (e.g. protests / submissions current-vs-archive) | `?view=<slug>` query param + a server-rendered `.chip-row` of real anchors (each carries `data-active="true|false"` + `aria-selected`). Reference: the protests / submissions chip rows in `web/pages/admin.bans.php` (`?section=protests&view=archive` / `?section=submissions&view=archive`). Pre-#1275 the chips called `Swap2ndPane()` — a `web/scripts/sourcebans.js` helper deleted at #1123 D1, leaving them dead — and the page rendered both views simultaneously. The new shape only renders the active view's data path; back/forward and link sharing both work. |
 | Lay out a sub-paged admin route's chrome (the 14rem vertical sidebar at `>=1024px`, the `<details open>` accordion at `<1024px`) | `web/themes/default/core/admin_sidebar.tpl` (the partial) + the `.admin-sidebar-shell` / `.admin-sidebar` / `.admin-sidebar__details` / `.admin-sidebar__summary` / `.admin-sidebar__nav` / `.admin-sidebar__link` / `.admin-sidebar-content` rules in `web/themes/default/css/theme.css` (#1259). The active link reuses the shared `.sidebar__link[aria-current="page"]` rule from the main app shell so the dark-pill-in-light / brand-orange-in-dark treatment is single-source. |
 | Render the trailing "Back" link on edit-* admin pages (the only surface that calls `new AdminTabs([], …)`) | `web/themes/default/core/admin_tabs.tpl` is the back-link-only partial (it still has a defensive `{foreach}` for legacy themes, but `web/includes/View/AdminTabs.php` only routes here when `$tabs === []`). Page handlers like `admin.edit.ban.php` / `admin.rcon.php` / `admin.email.php` call `new AdminTabs([], $userbank, $theme)` and the partial emits the right-aligned Back anchor (`.admin-tabs__back` in theme.css). |
-| Add or rename an admin-admins advanced-search filter | `web/pages/admin.admins.php` (filter-building loop + active-filter map for pagination) + `web/pages/admin.admins.search.php` (DTO population) + `web/includes/View/AdminAdminsSearchView.php` (`active_filter_*` properties) + `web/themes/default/box_admin_admins_search.tpl` (input + pre-fill). The form is single-submit AND-semantics with a backward-compat shim for legacy `advType=…&advSearch=…` URLs (#1207 ADM-4); cover new filters in `web/tests/integration/AdminAdminsSearchTest.php`. |
+| Add or rename an admin-admins advanced-search filter | `web/pages/admin.admins.php` (filter-building loop + active-filter map for pagination) + `web/pages/admin.admins.search.php` (DTO population + `$active_filter_count` increment for the new slot) + `web/includes/View/AdminAdminsSearchView.php` (`active_filter_*` properties) + `web/themes/default/box_admin_admins_search.tpl` (input + pre-fill). The form is single-submit AND-semantics with a backward-compat shim for legacy `advType=…&advSearch=…` URLs (#1207 ADM-4); cover new filters in `web/tests/integration/AdminAdminsSearchTest.php`. |
+| Wrap a filter `<form>` in a default-collapsed `<details>` disclosure (admin-admins advanced search; the public banlist / commslist filter bars are candidates for the same shape per #1303's notes) | `.filters-details` rules in `web/themes/default/css/theme.css` + reference shape in `web/themes/default/box_admin_admins_search.tpl` (`<details class="card filters-details" {if $has_active_filters}open{/if}>` with a `<summary data-testid="…-toggle">` carrying the title + chevron + optional "N active" count badge). The View carries paired `int $active_filter_count` + `bool $has_active_filters` properties (#1303); the page handler increments the count once per populated value slot, NEVER per match-mode toggle. The disclosure auto-expands on a post-submit paint so the Clear-filters affordance stays one click away. Visual vocabulary mirrors `core/admin_sidebar.tpl`'s mobile `<details open>` accordion (chevron + `prefers-reduced-motion: reduce` override). |
 | Add a shared "1 of these required" badge for an either/or input pair | `web/themes/default/page_submitban.tpl` (`data-required-group="…"` + the inline guard script — vanilla JS `// @ts-check`, blocks submit when both are empty) |
 | Bootstrap (paths, autoload, theme)     | `web/init.php`                                           |
 | Routing (`?p=…&c=…&o=…`)               | `web/includes/page-builder.php` — unrecognised admin `c=…` returns the 404 page slot via `web/pages/page.404.php` + `Sbpp\View\NotFoundView` (#1207 ADM-1) |

web/includes/View/AdminAdminsSearchView.php

@@ -39,6 +39,18 @@
  * (admin.admins.search.php) is the only place that knows whether a
  * given $_GET shape came from a modern submit, a legacy
  * `advType=…&advSearch=…` URL, or nothing at all.
+ *
+ * #1303 — collapsible disclosure
+ * ------------------------------
+ * The form is wrapped in a `<details class="card filters-details">`
+ * default-collapsed disclosure so the unfiltered admin list paints
+ * above the fold. `$has_active_filters` (derived from the nine
+ * `active_filter_*` value slots — match-mode toggles don't count
+ * because they always carry a default) drives the `[open]` attribute,
+ * so any post-submit page paints with the form expanded. The chrome
+ * mirrors `core/admin_sidebar.tpl`'s mobile `<details open>` pattern
+ * (chevron + label + `prefers-reduced-motion: reduce` override). The
+ * count badge ("Filters · N active") rides `$active_filter_count`.
  */
 final class AdminAdminsSearchView extends View
 {
@@ -74,6 +86,16 @@ final class AdminAdminsSearchView extends View
      *     to mark the matching `<option selected>` rows.
      * @param list<string> $active_filter_admsrvflag Pre-filled
      *     `admsrvflag[]` values.
+     * @param int $active_filter_count Number of non-empty filter
+     *     value slots — drives the `<summary>` count badge ("Filters
+     *     · N active") and `$has_active_filters`. Match-mode toggles
+     *     (`name_match` / `steam_match` / `admemail_match`) are NOT
+     *     counted: they always carry a default ('0' or '1') and only
+     *     refine the matching filter, they don't filter on their own.
+     * @param bool $has_active_filters Convenience boolean derived from
+     *     `$active_filter_count > 0`. The template uses it to decide
+     *     whether the disclosure paints `<details open>` (post-submit
+     *     paint) vs default-collapsed (first-paint).
      */
     public function __construct(
         public readonly bool $can_editadmin,
@@ -96,6 +118,8 @@ public function __construct(
         public readonly string $active_filter_server = '',
         public readonly array $active_filter_admwebflag = [],
         public readonly array $active_filter_admsrvflag = [],
+        public readonly int $active_filter_count = 0,
+        public readonly bool $has_active_filters = false,
     ) {
     }
 }

web/pages/admin.admins.search.php

@@ -178,32 +178,73 @@
     }
 }
 
+// Match-mode defaults differ per filter (#1231):
+//   - steam_match defaults to '0' (exact) — typical SteamID
+//     queries are "find this one admin by their full ID".
+//   - name_match / admemail_match default to '1' (partial) so
+//     pre-#1231 URLs (`?name=alice`) keep their substring
+//     behaviour. Adding the toggle widens the UI without
+//     regressing the default.
+$activeFilterName        = is_string($_GET['name']        ?? null) ? (string) $_GET['name']        : '';
+$activeFilterSteamid     = is_string($_GET['steamid']     ?? null) ? (string) $_GET['steamid']     : '';
+$activeFilterAdmemail    = is_string($_GET['admemail']    ?? null) ? (string) $_GET['admemail']    : '';
+$activeFilterWebgroup    = is_scalar($_GET['webgroup']    ?? null) ? (string) $_GET['webgroup']    : '';
+$activeFilterSrvadmgroup = is_string($_GET['srvadmgroup'] ?? null) ? (string) $_GET['srvadmgroup'] : '';
+$activeFilterSrvgroup    = is_scalar($_GET['srvgroup']    ?? null) ? (string) $_GET['srvgroup']    : '';
+$activeFilterServer      = is_scalar($_GET['server']      ?? null) ? (string) $_GET['server']      : '';
+
+// #1303 — the `admemail` filter is permission-gated by
+// `$can_editadmin` in both the rendering template AND the page
+// handler (`admin.admins.php` ignores `?admemail=` from a user without
+// `EditAdmins | Owner`). For URL-forgery cases where a non-admin
+// passes `?admemail=foo`, the input is hidden in the form and the
+// server narrows nothing; the count must mirror that — otherwise the
+// "N active" badge would say "1 active" while every visible filter
+// row reads empty. Mirror the gate locally so the count stays an
+// honest summary of what the visible form actually filters on.
+$canFilterByEmail = $userbank->HasAccess(WebPermission::mask(WebPermission::EditAdmins, WebPermission::Owner));
+
+// #1303 — count populated filter slots so the disclosure can paint a
+// "Filters · N active" badge on the <summary> and auto-expand on
+// post-submit. Match-mode selects (`name_match` / `steam_match` /
+// `admemail_match`) deliberately don't count: they always carry a
+// default ('0' or '1') and only refine the matching filter, they
+// don't filter on their own. Empty multi-select arrays count as zero
+// even though the array itself "exists" — the user hasn't picked a
+// permission. The `admemail` slot only counts when the user can
+// actually filter by it (see `$canFilterByEmail` above).
+$activeFilterCount =
+      ($activeFilterName        !== '' ? 1 : 0)
+    + ($activeFilterSteamid     !== '' ? 1 : 0)
+    + ($canFilterByEmail && $activeFilterAdmemail !== '' ? 1 : 0)
+    + ($activeFilterWebgroup    !== '' ? 1 : 0)
+    + ($activeFilterSrvadmgroup !== '' ? 1 : 0)
+    + ($activeFilterSrvgroup    !== '' ? 1 : 0)
+    + ($activeFilterServer      !== '' ? 1 : 0)
+    + (count($activeWebFlags) > 0 ? 1 : 0)
+    + (count($activeSrvFlags) > 0 ? 1 : 0);
+
 \Sbpp\View\Renderer::render($theme, new \Sbpp\View\AdminAdminsSearchView(
-    can_editadmin:             $userbank->HasAccess(WebPermission::mask(WebPermission::EditAdmins, WebPermission::Owner)),
+    can_editadmin:             $canFilterByEmail,
     server_list:               $servers,
     server_script:             $serverscript,
     webgroup_list:             $webgroups,
     srvadmgroup_list:          $srvadmgroups,
     srvgroup_list:             $srvgroups,
     admwebflag_list:           $webflag,
     admsrvflag_list:           $serverflag,
-    // Match-mode defaults differ per filter (#1231):
-    //   - steam_match defaults to '0' (exact) — typical SteamID
-    //     queries are "find this one admin by their full ID".
-    //   - name_match / admemail_match default to '1' (partial) so
-    //     pre-#1231 URLs (`?name=alice`) keep their substring
-    //     behaviour. Adding the toggle widens the UI without
-    //     regressing the default.
-    active_filter_name:           is_string($_GET['name']           ?? null) ? (string) $_GET['name']           : '',
+    active_filter_name:           $activeFilterName,
     active_filter_name_match:     is_scalar($_GET['name_match']     ?? null) ? (string) $_GET['name_match']     : '1',
-    active_filter_steamid:        is_string($_GET['steamid']        ?? null) ? (string) $_GET['steamid']        : '',
+    active_filter_steamid:        $activeFilterSteamid,
     active_filter_steam_match:    is_scalar($_GET['steam_match']    ?? null) ? (string) $_GET['steam_match']    : '0',
-    active_filter_admemail:       is_string($_GET['admemail']       ?? null) ? (string) $_GET['admemail']       : '',
+    active_filter_admemail:       $activeFilterAdmemail,
     active_filter_admemail_match: is_scalar($_GET['admemail_match'] ?? null) ? (string) $_GET['admemail_match'] : '1',
-    active_filter_webgroup:       is_scalar($_GET['webgroup']       ?? null) ? (string) $_GET['webgroup']       : '',
-    active_filter_srvadmgroup:    is_string($_GET['srvadmgroup']    ?? null) ? (string) $_GET['srvadmgroup']    : '',
-    active_filter_srvgroup:       is_scalar($_GET['srvgroup']       ?? null) ? (string) $_GET['srvgroup']       : '',
-    active_filter_server:         is_scalar($_GET['server']         ?? null) ? (string) $_GET['server']         : '',
+    active_filter_webgroup:       $activeFilterWebgroup,
+    active_filter_srvadmgroup:    $activeFilterSrvadmgroup,
+    active_filter_srvgroup:       $activeFilterSrvgroup,
+    active_filter_server:         $activeFilterServer,
     active_filter_admwebflag:     $activeWebFlags,
     active_filter_admsrvflag:     $activeSrvFlags,
+    active_filter_count:          $activeFilterCount,
+    has_active_filters:           $activeFilterCount > 0,
 ));

web/tests/e2e/pages/admin/AdminAdmins.ts

@@ -74,6 +74,21 @@ export class AdminAdminsPage extends BasePage {
         return this.page.locator('[data-testid="search-admins-reset"]');
     }
 
+    /** #1303 — the `<details>` disclosure wrapping the search form. */
+    get searchDisclosure(): Locator {
+        return this.page.locator('[data-testid="search-admins-disclosure"]');
+    }
+
+    /** #1303 — the `<summary>` toggle that opens / closes the disclosure. */
+    get searchToggle(): Locator {
+        return this.page.locator('[data-testid="search-admins-toggle"]');
+    }
+
+    /** #1303 — the "N active" badge (only present when count > 0). */
+    get searchActiveCount(): Locator {
+        return this.page.locator('[data-testid="search-admins-active-count"]');
+    }
+
     /** ADM-4 — pre-filled inputs by name. */
     searchInput(field: 'name' | 'steamid' | 'admemail' | 'webgroup' | 'srvadmgroup' | 'srvgroup' | 'admwebflag' | 'admsrvflag' | 'server'): Locator {
         return this.page.locator(`[data-testid="search-admins-${field}"]`);

web/tests/e2e/specs/flows/admin-admins-density.spec.ts

@@ -102,9 +102,11 @@ test.describe('flow: admin/admins density rework (#1207 ADM-3, ADM-4 / #1275)',
         const p = new AdminAdminsPage(page);
         await p.goto();
 
-        // Exactly one Search submit + one reset link, sitting at the
-        // bottom of the form (collapsing 8 per-row buttons was the
-        // headline of ADM-4).
+        // Exactly one Search submit + one reset link in the rendered
+        // DOM (collapsing 8 per-row buttons was the headline of
+        // ADM-4). Count assertion works whether the #1303 disclosure
+        // is open or closed — browsers parse <details> children
+        // either way.
         await expect(p.searchSubmit).toHaveCount(1);
         await expect(p.searchReset).toHaveCount(1);
 
@@ -115,6 +117,12 @@ test.describe('flow: admin/admins density rework (#1207 ADM-3, ADM-4 / #1275)',
         const baselineCount = await p.adminRows.count();
         expect(baselineCount).toBeGreaterThanOrEqual(1);
 
+        // #1303 — the form ships inside a default-collapsed
+        // `<details>` disclosure, so open it before driving the
+        // inputs. Native `<details>` makes `[open]` flip
+        // synchronously on click; no animation-driven sentinel.
+        await p.searchToggle.click();
+
         // Two filters at once: a deliberately not-matching login
         // ("zzznoadminmatchesthis") + a steam_match=1 (partial). The
         // login filter narrows the result list to zero — locking the