Fix #1315: public banlist / commslist v1.x → v2.0 regressions (unban-meta + Re-apply + collapsible adv search)

AGENTS.md

@@ -1443,6 +1443,9 @@ audit (#1207) locked in. New CTAs:
 | Reuse the moderation-queue card layout (admin submissions / protests, mobile-stacked summary rows) | `web/themes/default/css/theme.css` (`.queue-row`, `.queue-row__body`, `.queue-row__date` — #1207 PUB-2). Apply by adding `class="queue-row …"` to the outer `<details>` and dropping the inline `flex` / `flex-shrink:0` styles from the summary children. |
 | Add visible row actions to a table-rendered admin list (Edit / Unmute / Remove buttons + responsive mobile-card mirror) | `web/themes/default/page_comms.tpl` (#1207 ADM-5) is the canonical reference: `<button class="btn btn--secondary btn--sm">` / `<a class="btn btn--ghost btn--sm">` inside a `.row-actions` cell, plus `.ban-card__actions` row of identical-data-action buttons in the mobile card. Wire destructive / state-changing buttons via `data-action="…"` + `data-bid` + `data-fallback-href`; the inline page-tail JS calls `sb.api.call(Actions.PascalName)` and falls back to the GET URL if the JSON dispatcher is absent. |
 | Add a confirm + reason modal for an irreversible row-level action (unban, lift comm block, …) | `web/themes/default/page_bans.tpl` (`#bans-unban-dialog`, `Actions.BansUnban`) and `web/themes/default/page_comms.tpl` (`#comms-unblock-dialog`, `Actions.CommsUnblock`) are the canonical reference (#1301). Shape: a `<dialog hidden>` with a `<form method="dialog">` carrying a `<textarea aria-required="true">` (NOT the native `required` — that lets the browser block the form submit before our handler runs, swallowing the inline-error UX), a Cancel button, and a Confirm submit button. The page-tail JS opens the dialog via `showModal()` on `[data-action]` clicks, validates the trimmed reason on submit (load-bearing gate is server-side), forwards `ureason` to the JSON action, and on success flips the row in place via the same `flipRowToUnbanned`/`flipRowToUnmuted` helper the legacy single-click flow used. The legacy GET fallback (`?p=banlist&a=unban&id=…&key=…&ureason=…` / `?p=commslist&a=ungag…&ureason=…`) is the no-JS / hand-edited-URL path; both halves now reject empty `ureason` server-side so the audit log carries the *why*. **Do not** put `onclick="event.stopPropagation()"` on the trigger button — `document.addEventListener('click')` is how the dialog opener picks the click up, and stopPropagation would silently swallow it (the action button isn't inside any `[data-drawer-href]` ancestor anyway, so the defensiveness was a copy-paste from the row-name anchor that doesn't apply here). |
+| Surface unban-reason / removed-by inline on a public-list row (admin-lifted bans / comms — banlist-ureason or commslist-ureason inline) | `web/themes/default/page_bans.tpl` + `web/themes/default/page_comms.tpl` (#1315). Reason cell on the desktop table emits a `<div class="text-xs text-faint mt-1" data-testid="ban-unban-meta">` (or `comm-unban-meta` for comms) with "Unbanned by `<admin>`: `<reason>`" when `$ban.state == 'unbanned'` (or `$comm.state == 'unmuted'`); mobile cards mirror with the `-mobile` testid suffix. Always gated on `!$hideadminname` so anonymous viewers under a hidden-admins config don't get the admin name leaked. The `ureason` / `removedby` row fields come from the page handler's existing data path (`page.banlist.php` lines 635-643, `page.commslist.php` lines 626-635) — read-only render, no write-side overlap with #1301 / #1323's unban-reason flow. The commslist surface is higher-priority than the banlist (no drawer fallback on `<tr data-testid="comm-row">`); banlist users have the drawer as the canonical detail view. |
+| Re-apply (Reban) affordance on the public banlist for expired / unbanned rows | `web/themes/default/page_bans.tpl` (#1315 desktop only). The desktop row-actions cell emits `<a class="btn btn--ghost btn--sm btn--icon" data-testid="row-action-reapply" href="index.php?p=admin&c=bans&section=add-ban&rebanid={$ban.bid}&key={$admin_postkey}">` when `$can_add_ban && ($ban.state == 'expired' \|\| $ban.state == 'unbanned')`. The smart-default block on `admin.bans.php`'s `add-ban` section detects `?rebanid=…` and pre-populates the form via `BansPrepareReban`. Mobile parity (matching the commslist's `row-action-reapply-mobile` button on `.ban-card__actions`) is intentionally deferred — the mobile card wraps content in a single `<a data-testid="drawer-trigger">`, so adding a sibling button requires restructuring to a `<div>` wrapper + inner anchor (the `page_comms.tpl` mobile-card shape). Drawer is the canonical mobile detail view in the meantime. |
+| Wrap a public-list legacy advanced-search form (banlist / commslist) in a default-collapsed `<details class="filters-details">` disclosure | `web/themes/default/page_bans.tpl` + `web/themes/default/page_comms.tpl` (#1315). The same `.filters-details` rules in `web/themes/default/css/theme.css` cover the chrome (summary chevron, `[open]` state, hover bg, focus ring, count badge); the public-list usage adds a `.filters-details__body > form.card` rule that suppresses the inner card framing because the disclosure body wraps a `{load_template file="admin.bans.search"}` (or `…comms.search`) — i.e. a sibling page-render that emits its own `<form class="card">`. The View DTO (`Sbpp\View\BanListView` / `Sbpp\View\CommsListView`) carries a `bool $is_advanced_search_open` set by the page handler from `isset($_GET['advSearch']) && (string) $_GET['advSearch'] !== ''` so the legacy `?advSearch=…&advType=…` URL shim auto-opens the disclosure. Bare `?p=banlist` / `?p=commslist` and simple-bar filters (`?searchText=` / `?server=` / `?time=`) leave it closed so the unfiltered list reaches above the fold. Selectors anchor on `[data-testid="banlist-advsearch-disclosure"]` / `[data-testid="commslist-advsearch-disclosure"]` (the `<details>`) + the `…-toggle` (the `<summary>`) + the `…-active` count badge. The same `.filters-details` chrome was introduced for admin-admins by #1303 — both surfaces share the CSS so a future tweak is single-source. |
 | Edit the player-detail drawer (open trigger, tabs, panes, lazy loaders) | `web/themes/default/js/theme.js` (`renderDrawerBody` / `loadPaneIfNeeded`) |
 | Render the per-server map thumbnail in the expanded public server card | `web/themes/default/page_servers.tpl` (`<img data-testid="server-map-img" hidden>` slot inside `[data-testid="server-players-panel"]`) + `web/scripts/server-tile-hydrate.js`'s `applyData()` (patches `src` from `r.data.mapimg`, toggles `hidden` on `load` / `error`). The lookup is feature-detected via the testid so the admin Server Management list (which does NOT ship the slot) silently no-ops. The URL itself comes from global helper `\GetMapImage()` in `web/includes/system-functions.php` (falls back to `images/maps/nomap.jpg` when the file is missing); the bundled `nomap.jpg` placeholder ships under `web/images/maps/`. The slot must default to `hidden` and stay hidden on the `error` branch — fork installs without `nomap.jpg` would otherwise paint a broken-image icon. Regression guards: `web/tests/integration/ServerMapImageRenderTest.php` (template ships the slot + helper carries the wiring + handler still emits `mapimg`) + `web/tests/e2e/specs/flows/server-map-thumbnail.spec.ts` (runtime visibility under success / 404 / connect-error). #1312 restored this surface after the #1123 D1 redesign dropped the legacy `<img id="mapimg_{$server.sid}">`; #1313 moved the wiring out of the inline `<script>` block into the shared helper. |
 | Hydrate server-tile cards with live A2S data (status pill / map / players / hostname / refresh) on the public servers list AND the admin Server Management list | `web/scripts/server-tile-hydrate.js` (`window.SBPP.hydrateServerTiles`) — auto-runs on first paint for every container marked `data-server-hydrate="auto"`. Consumed by `web/themes/default/page_servers.tpl` (public) and `web/themes/default/page_admin_servers_list.tpl` (admin, #1313). Selector contract per tile: `[data-testid="server-tile"]` outer card + `data-id="<sid>"` + `[data-testid="server-{status,map,players,host}"]` cells + optional `[data-testid="server-{refresh,toggle}"]` / `[data-players-bar]` / `[data-testid="server-players-panel"]` / `[data-testid="server-map-img"]` (the player-list panel + the map thumbnail are public-only; the helper feature-detects every optional element). Disabled tiles carry `data-server-skip="1"` so the helper leaves them at the server-rendered placeholder. Never copy-paste the hydration code into a new template — wire the testids and `data-server-hydrate="auto"` instead and the helper picks the surface up automatically. |

web/includes/View/BanListView.php

@@ -80,6 +80,19 @@ public function __construct(
         // advanced-search box doesn't have to rename the property.
         public readonly array $server_list,
         public readonly array $filters,
+        // #1315: drives the `<details class="filters-details">`
+        // disclosure that wraps the advanced-search box at the top
+        // of `page_bans.tpl`. True iff the request URL carries the
+        // `?advSearch=&advType=` legacy-shim pair (the v1.x power-
+        // user surface re-exposed as a default-collapsed disclosure
+        // — see "Sub-paged advanced search" notes in the issue body).
+        // Bare `?p=banlist` / simple-bar filters (`?searchText=` /
+        // `?server=` / `?time=`) intentionally leave the disclosure
+        // closed so the unfiltered list reaches above the fold —
+        // those filters are visible on the inline sticky bar and
+        // don't need the larger card open. Mirrors the post-submit
+        // auto-open contract #1303 introduced for admin-admins.
+        public readonly bool $is_advanced_search_open,
     ) {
     }
 }

web/includes/View/CommsListView.php

@@ -115,6 +115,18 @@ public function __construct(
         public readonly bool $hideadminname,
         public readonly bool $view_comments,
         public readonly bool $view_bans,
+        // #1315: drives the `<details class="filters-details">`
+        // disclosure that wraps the advanced-search box at the top
+        // of `page_comms.tpl`. True iff the request URL carries the
+        // `?advSearch=&advType=` legacy-shim pair (the v1.x power-
+        // user surface re-exposed as a default-collapsed disclosure).
+        // Bare `?p=commslist` / simple-bar filters (`?searchText=` /
+        // `?server=` / `?time=` / `?type=` / `?state=`) intentionally
+        // leave the disclosure closed — those filters are visible on
+        // the inline sticky bar and don't need the larger card open.
+        // Mirrors the post-submit auto-open contract #1303 introduced
+        // for admin-admins.
+        public readonly bool $is_advanced_search_open,
     ) {
     }
 }

web/pages/page.banlist.php

@@ -1072,6 +1072,17 @@ function setPostKey()
     'time'   => (isset($publicFilterTimeMap[$timeFilter]) ? $timeFilter : ''),
 ];
 
+// #1315: auto-open the advanced-search disclosure on a post-submit
+// paint. Bare `?p=banlist` and simple-bar filters
+// (`?searchText=` / `?server=` / `?time=`) leave it closed so the
+// unfiltered list reaches above the fold. The legacy ?advSearch shim
+// is the only surface that re-opens it, mirroring v1.x behaviour
+// where the form was always-open below the row table — the v2.0
+// disclosure is the post-#1303 collapsed shape with the same
+// post-submit affordance the admin-admins page uses.
+$banlistAdvancedOpen =
+    isset($_GET['advSearch']) && (string) $_GET['advSearch'] !== '';
+
 Renderer::render($theme, new BanListView(
     ban_list:        $bans,
     ban_nav:         $ban_nav,
@@ -1097,7 +1108,8 @@ function setPostKey()
     can_export:      (bool) $userbank->HasAccess(WebPermission::Owner) || Config::getBool('config.exportpublic'),
     admin_postkey:   $_SESSION['banlist_postkey'],
     can_add_ban:     (bool) $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::AddBan)),
-    is_filtered:     $banlistIsFiltered,
-    server_list:     $banlistServerList,
-    filters:         $banlistFilters,
+    is_filtered:             $banlistIsFiltered,
+    server_list:             $banlistServerList,
+    filters:                 $banlistFilters,
+    is_advanced_search_open: $banlistAdvancedOpen,
 ));

web/pages/page.commslist.php

@@ -1086,6 +1086,17 @@ function setPostKey()
     || $filters['type']   !== ''
     || $hideInactive;
 
+// #1315: auto-open the advanced-search disclosure on a post-submit
+// paint. Bare `?p=commslist` and simple-bar filters
+// (`?searchText=` / `?server=` / `?time=` / `?type=` / `?state=`)
+// leave it closed so the unfiltered list reaches above the fold. The
+// legacy ?advSearch shim is the only surface that re-opens it,
+// mirroring v1.x behaviour where the form was always-open below the
+// row table — the v2.0 disclosure is the post-#1303 collapsed shape
+// with the same post-submit affordance the admin-admins page uses.
+$commsAdvancedOpen =
+    isset($_GET['advSearch']) && (string) $_GET['advSearch'] !== '';
+
 // Aggregate permission flags. Each precomputed via Perms::for($userbank)
 // so the template's {if $can_*} reads stay opinion-free about the bit
 // math — see the AGENTS.md "Permissions" section + Perms::for() docblock.
@@ -1129,4 +1140,5 @@ function setPostKey()
     hideadminname:            $hideAdminName,
     view_comments:            (bool) $view_comments,
     view_bans:                $viewBans,
+    is_advanced_search_open:  $commsAdvancedOpen,
 ));

web/tests/e2e/specs/flows/public-banlist-regressions.spec.ts

@@ -0,0 +1,116 @@
+/**
+ * #1315 — public banlist / commslist v1.x → v2.0 regressions.
+ *
+ * Three regression slices land in the same PR:
+ *   1. Advanced-search disclosure on the banlist + commslist
+ *      (default-collapsed `<details class="filters-details">`,
+ *      auto-opens on a post-submit `?advType=&advSearch=` URL).
+ *   2. Banlist Re-apply icon affordance for expired / unbanned rows
+ *      (gated on `ADMIN_OWNER | ADMIN_ADD_BAN`).
+ *   3. Inline unban-meta line ("Unbanned by <admin>: <reason>")
+ *      below the truncated reason cell on admin-lifted rows
+ *      (priority on commslist — no drawer fallback there).
+ *
+ * This spec covers slice 1 end-to-end in a real browser. Slices 2
+ * and 3 require seeded DB state (`RemovedBy` / `ureason` columns
+ * populated; ban rows in `expired`/`unbanned` states) that the e2e
+ * fixture doesn't provide and the JSON API doesn't expose cleanly
+ * enough to reach without coupling to PR #1323's in-flight unban
+ * flow. They're locked server-side by `PublicBanListRegressionTest`
+ * (PHPUnit, isolated DB state).
+ */
+
+import { expect, test } from '../../fixtures/auth.ts';
+import { expectNoCriticalA11y } from '../../fixtures/axe.ts';
+
+/**
+ * Closed-disclosure axe coverage runs at the start of each test (the
+ * bare page paint, with the form's `<select>` elements still inside
+ * a collapsed `<details>` and therefore hidden from the a11y tree).
+ *
+ * We deliberately do NOT run axe AFTER opening the disclosure: the
+ * legacy `box_admin_bans_search.tpl` / `box_admin_comms_search.tpl`
+ * partials predate #1123's testability sweep and ship `<select>`s
+ * without an associated label (axe rule `select-name`, critical).
+ * That's a real a11y bug, but it lived in the legacy form for
+ * years; #1315 just makes it reachable from the bare page (it was
+ * always reachable via the `?advSearch=…&advType=…` URL shim). Per
+ * AGENTS.md "Playwright E2E specifics" the threshold must NOT be
+ * downgraded to make tests green; the right move is a follow-up
+ * a11y issue against the underlying legacy form, not a `disabled`
+ * filter here. The smoke specs (`smoke/banlist.spec.ts`,
+ * `smoke/commslist.spec.ts`) already audit the closed-disclosure
+ * paint; this spec adds focused coverage on the disclosure
+ * vocabulary itself.
+ */
+test.describe('#1315: public banlist / commslist disclosure regressions', () => {
+    test('banlist advanced-search disclosure defaults closed; opens on submit URL', async ({ page }, testInfo) => {
+        await page.goto('/index.php?p=banlist');
+
+        const disclosure = page.locator('[data-testid="banlist-advsearch-disclosure"]');
+        const toggle     = page.locator('[data-testid="banlist-advsearch-toggle"]');
+        await expect(disclosure).toBeVisible();
+        await expect(toggle).toBeVisible();
+
+        // Closed-state axe — the legacy form is not yet exposed.
+        await expectNoCriticalA11y(page, testInfo);
+
+        // Native <details> reflects [open] as a JS property —
+        // synchronous attribute, no animation in the way.
+        await expect
+            .poll(async () => await disclosure.evaluate((el) => (el as HTMLDetailsElement).open))
+            .toBe(false);
+        await expect(page.locator('[data-testid="banlist-advsearch-active"]')).toHaveCount(0);
+
+        await toggle.click();
+        await expect
+            .poll(async () => await disclosure.evaluate((el) => (el as HTMLDetailsElement).open))
+            .toBe(true);
+
+        // The legacy advanced-search form is now reachable inside
+        // the disclosure body. The form carries the
+        // `[data-testid="search-bans-form"]` hook from the legacy
+        // box_admin_bans_search.tpl partial.
+        await expect(page.locator('[data-testid="search-bans-form"]')).toBeVisible();
+
+        // Post-submit auto-open: navigating to a URL with the
+        // legacy `?advType=&advSearch=` shim must paint the
+        // disclosure with `[open]` already set so the form chrome
+        // stays visible while the user iterates on filters.
+        await page.goto('/index.php?p=banlist&advType=name&advSearch=somenick');
+        await expect
+            .poll(async () => await disclosure.evaluate((el) => (el as HTMLDetailsElement).open))
+            .toBe(true);
+        await expect(page.locator('[data-testid="banlist-advsearch-active"]')).toBeVisible();
+    });
+
+    test('commslist advanced-search disclosure defaults closed; opens on submit URL', async ({ page }, testInfo) => {
+        await page.goto('/index.php?p=commslist');
+
+        const disclosure = page.locator('[data-testid="commslist-advsearch-disclosure"]');
+        const toggle     = page.locator('[data-testid="commslist-advsearch-toggle"]');
+        await expect(disclosure).toBeVisible();
+        await expect(toggle).toBeVisible();
+
+        // Closed-state axe — the legacy form is not yet exposed.
+        await expectNoCriticalA11y(page, testInfo);
+
+        await expect
+            .poll(async () => await disclosure.evaluate((el) => (el as HTMLDetailsElement).open))
+            .toBe(false);
+        await expect(page.locator('[data-testid="commslist-advsearch-active"]')).toHaveCount(0);
+
+        await toggle.click();
+        await expect
+            .poll(async () => await disclosure.evaluate((el) => (el as HTMLDetailsElement).open))
+            .toBe(true);
+
+        await expect(page.locator('[data-testid="search-comms-form"]')).toBeVisible();
+
+        await page.goto('/index.php?p=commslist&advType=name&advSearch=somenick');
+        await expect
+            .poll(async () => await disclosure.evaluate((el) => (el as HTMLDetailsElement).open))
+            .toBe(true);
+        await expect(page.locator('[data-testid="commslist-advsearch-active"]')).toBeVisible();
+    });
+});