Skip to content

Fix #1313: hydrate Map / Players cells on admin Server Management list - #1327

Merged
rumblefrog merged 3 commits into
mainfrom
fix/issue-1313-admin-server-list-hydration
May 10, 2026
Merged

rumblefrog merged 3 commits into
mainfrom
fix/issue-1313-admin-server-list-hydration

Conversation

@rumblefrog

Copy link
Copy Markdown
Member

Summary

Fixes #1313.

The admin Server Management list (?p=admin&c=servers&section=list)
shipped placeholder Map / Players cells with data-hydrate="..."
attributes that nothing read, so the values stayed at the em-dash
forever. The public Server List (?p=servers) had the same card
shape plus an inline <script> that drove Actions.ServersHostPlayers
and patched the cells, but the admin surface never picked up the call
site (#1313 audit).

This PR extracts the public list's hydration loop into a shared
helper at web/scripts/server-tile-hydrate.js and pulls it from
both templates. The helper auto-runs on first paint for any
container marked data-server-hydrate=\"auto\", walks
[data-testid=\"server-tile\"] children, fires
Actions.ServersHostPlayers per tile, and patches the live cells
(status pill, map, players, hostname, players bar, plus the
public-only player-list panel — feature-detected per tile).

The admin tile picks up the canonical testids the public tile uses
(server-status / server-map / server-players / server-host)
plus a status pill with loading / online / offline states and a
refresh button per enabled row. Disabled tiles carry
data-server-skip=\"1\" so the helper leaves them at the
server-rendered placeholder — no point poking a UDP socket for a
server the panel just told you is disabled by config.

Files

  • web/scripts/server-tile-hydrate.js — new shared helper, vanilla
    JS + // @ts-check. Exposed as window.SBPP.hydrateServerTiles.
  • web/scripts/globals.d.ts — typed declaration for the new
    window.SBPP.hydrateServerTiles entry point.
  • web/themes/default/page_servers.tpl — replaced the inline
    hydration loop with the shared <script src=\"...server-tile-hydrate.js\">.
  • web/themes/default/page_admin_servers_list.tpl — added the
    status pill, hostname target, refresh button, data-status=\"loading\"
    / data-server-skip=\"1\" markers, and the script include.
    Renamed the legacy data-hydrate=\"map|players\" placeholders to
    the canonical data-testid=\"server-{map,players}\" testids.
  • web/tests/integration/AdminServersListHydrationTest.php — locks
    the per-tile markup contract (testids, script include,
    data-server-skip shape on disabled tiles, absence of the legacy
    data-hydrate= placeholders).
  • web/tests/e2e/specs/smoke/admin/servers.spec.ts +
    web/tests/e2e/pages/admin/AdminServers.ts — pins the page mount,
    the script wiring, and the axe-critical=0 a11y gate on the admin
    route.
  • AGENTS.md — added an anti-pattern entry (inert
    data-hydrate=\"...\" placeholders without the helper include)
    and a Where-to-find-what row pointing at the new helper.
  • ARCHITECTURE.md — added a server-tile-hydrate.js row to the
    Frontend JavaScript table.

Test plan

  • ./sbpp.sh phpstan — 4 errors, all baseline (matches
    origin/main after --generate-baseline=phpstan-baseline.neon).
    My changes add 0 new errors.
  • ./sbpp.sh test — 409 tests pass, including the 6 new
    AdminServersListHydrationTest cases.
  • ./sbpp.sh ts-check — clean.
  • ./sbpp.sh composer api-contract — no diff (the API handler
    api_servers_host_players was untouched).
  • ./sbpp.sh e2e --grep \"servers\" — all 10 servers-related e2e
    tests pass (chromium + mobile-chromium), including the new
    smoke /admin/servers spec.
  • ./sbpp.sh e2e --grep \"smoke\" — all 60 smoke specs pass; no
    regressions on the public servers list, dashboard, comms,
    protests, login, etc.

@rumblefrog

Copy link
Copy Markdown
Member Author

Heads-up from #1326 review: that PR (now ready to merge) restores the <img data-testid="server-map-img"> slot inside [data-testid="server-players-panel"] in page_servers.tpl, plus the inline applyData patch that wires r.data.mapimg into src and toggles hidden on load / error (#1312).

When you rebase this branch, the extracted web/scripts/server-tile-hydrate.js will need to absorb that wiring or the public servers page silently loses the thumbnail again. The PHPUnit guard at web/tests/integration/ServerMapImageRenderTest.php will fail your build if the inline initializer drops the tile.querySelector('[data-testid="server-map-img"]') lookup, and the e2e spec at web/tests/e2e/specs/flows/server-map-thumbnail.spec.ts pins the runtime behaviour. Worth adding equivalent hydration coverage to the helper for both the public and admin surfaces while you're touching the file.

@rumblefrog
rumblefrog force-pushed the fix/issue-1313-admin-server-list-hydration branch from 3e0000d to 4b9599b Compare May 10, 2026 21:19
The admin Server Management list (?p=admin&c=servers&section=list)
shipped placeholder Map / Players cells with `data-hydrate="..."`
attributes that nothing read, so the values stayed at the em-dash
forever. The public Server List (?p=servers) had the same card shape
plus an inline <script> that drove `Actions.ServersHostPlayers` and
patched the cells, but the admin surface never picked up the call site.

Extract the public list's hydration loop into
web/scripts/server-tile-hydrate.js and pull it from both templates.
The helper auto-runs on first paint for any container marked
`data-server-hydrate="auto"`, walks `[data-testid="server-tile"]`
children, fires `Actions.ServersHostPlayers` per tile, and patches
the live cells (status pill, map, players, hostname, players bar,
plus the public-only player-list panel — feature-detected per tile).

The admin tile picks up the canonical testids the public tile uses
(server-status / server-map / server-players / server-host) plus a
status pill with loading / online / offline states and a refresh
button per enabled row. Disabled tiles carry `data-server-skip="1"`
so the helper leaves them at the server-rendered placeholder — no
point poking a UDP socket for a server the panel just told you is
disabled by config.

Regression tests:

  - web/tests/integration/AdminServersListHydrationTest.php pins the
    per-tile markup contract (testids, the script include, the
    `data-server-skip` shape on disabled tiles, the absence of the
    legacy `data-hydrate=` placeholders).
  - web/tests/e2e/specs/smoke/admin/servers.spec.ts pins the page
    mount + script wiring + axe-critical=0 on the admin route, with
    a matching page object under web/tests/e2e/pages/admin/.

Docs synced (AGENTS.md anti-pattern + Where-to-find-what row,
ARCHITECTURE.md Frontend-JavaScript table) per the AGENTS.md
keep-the-docs-in-sync table.

Fixes #1313
@rumblefrog
rumblefrog force-pushed the fix/issue-1313-admin-server-list-hydration branch from 4b9599b to 131cd0d Compare May 10, 2026 22:35
Two CI regressions surfaced after rebasing onto origin/main:

  1. ts-check fails on the new feature-detected `mapimg` wiring inside
     `applyData()`. The `onload` / `onerror` closures reference the
     narrowed `mapImg` binding, and tsc loses the `instanceof` narrowing
     across closure boundaries (the parent scope's reference could in
     principle be reassigned before the closure fires). Pin a local
     non-null `imgEl = mapImg` inside the narrowed branch and use that
     in the closures.

  2. `ServerMapImageRenderTest::testHandlerStillEmitsMapimgField` was
     asserting the literal substring `'mapimg'   => GetMapImage(`
     (three spaces around the arrow). #1311's `SourceQueryCache` split
     reflowed the response builder and the column alignment shifted to
     five spaces. The contract we care about is "the handler still
     sources `mapimg` from `GetMapImage()`", not the exact whitespace —
     swap the substring assertion for a whitespace-tolerant regex
     (`/'mapimg'\\s*=>\\s*GetMapImage\\s*\\(/`).
`testHydrationHelperWiresMapImg` asserted the literal substring
`mapImg.src = String(d.mapimg)` (and the matching `mapImg.onload` /
`mapImg.onerror` regexes). The previous commit renamed the local
binding from `mapImg` to `imgEl` inside the narrowed branch to satisfy
tsc's "narrowing lost across closure boundary" rule — the contract the
test cares about is "the helper assigns d.mapimg to the slot's src and
toggles hidden on load/error", not the variable name.

Relax the assertions to match any `\w+`-shaped local binding. The
testid `tile.querySelector('[data-testid="server-map-img"]')` lookup
stays as a string-literal assertion since that hook is the load-bearing
contract — the regex just covers what we do with the binding once we
have it.
@rumblefrog
rumblefrog added this pull request to the merge queue May 10, 2026
Merged via the queue into main with commit 1830291 May 10, 2026
4 checks passed
@rumblefrog
rumblefrog deleted the fix/issue-1313-admin-server-list-hydration branch May 10, 2026 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Admin → Server Management list omits Map / Players (live A2S data shown on the public Server List)

1 participant