diff --git a/web/includes/View/AdminModsListView.php b/web/includes/View/AdminModsListView.php index ced82f9fb..29a113903 100644 --- a/web/includes/View/AdminModsListView.php +++ b/web/includes/View/AdminModsListView.php @@ -16,7 +16,8 @@ * Property names keep the historical `permission_*` shape (rather than * the newer `can_*` convention `Sbpp\View\Perms::for()` produces) because * the template references `{$permission_listmods}` / - * `{$permission_editmods}` / `{$permission_deletemods}`. See + * `{$permission_addmods}` / `{$permission_editmods}` / + * `{$permission_deletemods}`. See * `AdminServersListView` / `AdminBansAddView` for the canonical * `can_*` convention on Views that don't have to honour a historical * variable name. @@ -30,6 +31,7 @@ final class AdminModsListView extends View */ public function __construct( public readonly bool $permission_listmods, + public readonly bool $permission_addmods, public readonly bool $permission_editmods, public readonly bool $permission_deletemods, public readonly int $mod_count, diff --git a/web/pages/admin.mods.php b/web/pages/admin.mods.php index d0378f4b9..7f12c7b8e 100644 --- a/web/pages/admin.mods.php +++ b/web/pages/admin.mods.php @@ -37,11 +37,13 @@ return; } +// mid=0 is the reserved Web pseudo-mod, not a configurable game mod. $mod_list = $GLOBALS['PDO']->query("SELECT * FROM `:prefix_mods` WHERE mid > 0 ORDER BY name ASC")->resultset(); -$mod_count = (int) $GLOBALS['PDO']->query("SELECT COUNT(mid) AS cnt FROM `:prefix_mods`")->single()['cnt']; +$mod_count = (int) $GLOBALS['PDO']->query("SELECT COUNT(mid) AS cnt FROM `:prefix_mods` WHERE mid > 0")->single()['cnt']; \Sbpp\View\Renderer::render($theme, new \Sbpp\View\AdminModsListView( permission_listmods: $canList, + permission_addmods: $canAdd, permission_editmods: $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::EditMods)), permission_deletemods: $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::DeleteMods)), mod_count: $mod_count, diff --git a/web/tests/e2e/specs/flows/admin-groups-select-all-flags.spec.ts b/web/tests/e2e/specs/flows/admin-groups-select-all-flags.spec.ts index 1996c4b48..1283d59b4 100644 --- a/web/tests/e2e/specs/flows/admin-groups-select-all-flags.spec.ts +++ b/web/tests/e2e/specs/flows/admin-groups-select-all-flags.spec.ts @@ -128,6 +128,13 @@ test.describe('flow: admin groups select-all permission flags (upstream #1436)', await expect(selectAll).toHaveAttribute('type', 'button'); await expect(selectNone).toHaveAttribute('type', 'button'); await expect(bitmaskBadge).toHaveText(/^0 bitmask$/); + // Nothing checked yet: "Select none" starts out reflecting that + // there's nothing left to clear (upstream #1573 parity — see the + // SbppGroupsRefreshSelectAllButtons docblock for why this fork + // uses aria-disabled + a dimmed style instead of a real indeterminate + // checkbox or the native `disabled` attribute). + await expect(selectNone).toHaveAttribute('aria-disabled', 'true'); + await expect(selectAll).toHaveAttribute('aria-disabled', 'false'); const total = await checkboxes.count(); expect(total).toBeGreaterThan(0); @@ -146,10 +153,17 @@ test.describe('flow: admin groups select-all permission flags (upstream #1436)', } await expect(bitmaskBadge).toHaveText(`${expected} bitmask`); await expect(bitmaskBadge).not.toContainText('-'); + // Everything is now checked: "Select all" reflects there's nothing + // left to add, "Select none" flips back to actionable. + await expect(selectAll).toHaveAttribute('aria-disabled', 'true'); + await expect(selectNone).toHaveAttribute('aria-disabled', 'false'); // Idempotent: a second press changes nothing and must not corrupt - // the preview. - await selectAll.click(); + // the preview. The button now carries aria-disabled="true", which + // Playwright's actionability check treats as disabled, so force + // the click: a mouse user can still press it (no native + // `disabled`), and that press must stay a no-op. + await selectAll.click({ force: true }); await expect(bitmaskBadge).toHaveText(`${expected} bitmask`); // ---- Save round-trips the folded OR-sum -------------------------- @@ -181,13 +195,21 @@ test.describe('flow: admin groups select-all permission flags (upstream #1436)', for (let i = 0; i < reloadedTotal; i++) { await expect(reloadedChecks.nth(i)).toBeChecked(); } + // Fresh page load re-derives the same state from the SSR-checked + // grid, not just from in-memory JS state carried across the click. + const reloadedSelectAll = reloadedDetail.locator('[data-testid="flag-select-all"]'); + const reloadedSelectNone = reloadedDetail.locator('[data-testid="flag-select-none"]'); + await expect(reloadedSelectAll).toHaveAttribute('aria-disabled', 'true'); + await expect(reloadedSelectNone).toHaveAttribute('aria-disabled', 'false'); // ---- Select none → every checkbox cleared, badge back to 0 ------- - await reloadedDetail.locator('[data-testid="flag-select-none"]').click(); + await reloadedSelectNone.click(); for (let i = 0; i < reloadedTotal; i++) { await expect(reloadedChecks.nth(i)).not.toBeChecked(); } await expect(reloadedBadge).toHaveText(/^0 bitmask$/); + await expect(reloadedSelectNone).toHaveAttribute('aria-disabled', 'true'); + await expect(reloadedSelectAll).toHaveAttribute('aria-disabled', 'false'); }); test('a bulk toggle arms the unsaved-changes guard', async ({ page }) => { diff --git a/web/themes/default/page_admin_groups_list.tpl b/web/themes/default/page_admin_groups_list.tpl index 61fce5012..a8222b1bc 100644 --- a/web/themes/default/page_admin_groups_list.tpl +++ b/web/themes/default/page_admin_groups_list.tpl @@ -613,6 +613,63 @@ function SbppGroupsToggleAllFlags(checked) { var preview = document.querySelector('[data-testid="flag-bitmask"]'); if (preview) preview.textContent = SbppFoldFlags(grid) + ' bitmask'; + // The `change` above targets the grid itself, which the grid listener + // filters out (it only reacts to `input[name="flags[]"]` targets), so + // refresh the Select all / Select none state explicitly here. + SbppGroupsRefreshSelectAllButtons(); +} + +/** + * Port of upstream sbpp/sourcebans-pp#1573's tri-state "select all" + * checkbox, adapted to this fork's two-button shape instead of a + * single ``: a checkbox here would have lost + * the explicit Select-all/Select-none affordance the #1436 spec + * (admin-groups-select-all-flags.spec.ts) already locks in — including + * the unsigned bit-31 OR-fold guard (#1272) and the dirty-tracker-arming + * contract, both of which are exercised through `SbppGroupsToggleAllFlags` + * and would need re-proving from scratch against a checkbox's native + * `indeterminate` handling. Reflecting the grid's current state via each + * button's `aria-disabled` + dimmed style gives the same "you can see + * there's nothing left to do" feedback without touching that logic. + * + * Deliberately uses `aria-disabled` + a visual dim, NOT the native + * `disabled` attribute: `SbppGroupsToggleAllFlags` is already a safe + * no-op when everything is already on/off (it bails before dispatching + * `change`, per the comment above it), and the #1436 spec exercises a + * redundant "Select all" press on a fully-selected grid as a deliberate + * idempotency check. A native `disabled` button can't receive a click at + * all; `aria-disabled` communicates the same "nothing left to do" state + * to assistive tech and sighted users while the button stays pressable + * (the spec forces that click, since Playwright's actionability check + * treats aria-disabled="true" as disabled). + * + * Call after anything that can change the grid's checked state without + * going through a user click on an individual checkbox: the bulk toggle + * (`SbppGroupsToggleAllFlags` dispatches `change` on the grid itself, + * which the grid listener ignores), the master-detail `paintGroup()` + * repaint, and the bootstrap call below. Manual clicks on a checkbox are + * covered by the `change` listener wired further down this file. + */ +function SbppGroupsRefreshSelectAllButtons() { + var grid = document.querySelector('[data-testid="flag-grid"]'); + var selectAll = document.querySelector('[data-testid="flag-select-all"]'); + var selectNone = document.querySelector('[data-testid="flag-select-none"]'); + if (!grid || !(selectAll || selectNone)) return; + + var checks = grid.querySelectorAll('input[name="flags[]"]:not([disabled])'); + var total = checks.length; + var checked = grid.querySelectorAll('input[name="flags[]"]:not([disabled]):checked').length; + + /** @param {Element|null} btn @param {boolean} exhausted */ + function reflect(btn, exhausted) { + if (!btn) return; + btn.setAttribute('aria-disabled', exhausted ? 'true' : 'false'); + /** @type {HTMLElement} */ (btn).style.opacity = exhausted ? '0.5' : ''; + /** @type {HTMLElement} */ (btn).style.cursor = exhausted ? 'default' : ''; + } + + reflect(selectAll, total === 0 || checked === total); + reflect(selectNone, checked === 0); } /** @@ -769,7 +826,12 @@ function SbppServerGroupsDelete(gid, name, type, btn) { if (!target || !target.matches || !target.matches('input[name="flags[]"]')) return; preview.textContent = SbppFoldFlags(grid) + ' bitmask'; + SbppGroupsRefreshSelectAllButtons(); }); + + // Reflect the SSR-rendered initial state (e.g. a group whose flags + // already cover every checkbox loads with "Select all" disabled). + SbppGroupsRefreshSelectAllButtons(); })(); // --- Client-side master-detail selection --- @@ -852,6 +914,10 @@ function SbppServerGroupsDelete(gid, name, type, btn) { if (bitmaskEl) { bitmaskEl.textContent = flags + ' bitmask'; } + // paintGroup() sets .checked directly (no change event), so the + // Select all/Select none disabled state needs an explicit refresh + // here too — the grid's `change` listener alone won't catch it. + SbppGroupsRefreshSelectAllButtons(); var rows = list.querySelectorAll('[data-testid="group-row"]'); for (var r = 0; r < rows.length; r++) { diff --git a/web/themes/default/page_admin_mods_list.tpl b/web/themes/default/page_admin_mods_list.tpl index 273dc1bbe..41c7bcb37 100644 --- a/web/themes/default/page_admin_mods_list.tpl +++ b/web/themes/default/page_admin_mods_list.tpl @@ -7,6 +7,7 @@ Variable contract (kept in sync by SmartyTemplateRule): - $permission_listmods — gate the whole tab body. + - $permission_addmods — gate the empty-state "Add mod" CTA. - $permission_editmods — gate the per-row "Edit" link. - $permission_deletemods — gate the per-row "Delete" button. - $mod_count — total mods configured. @@ -55,7 +56,7 @@
{if $mod_count > 0} -
+
@@ -69,20 +70,23 @@ + {* Mod metadata is entity-encoded on store. Decode before + Smarty's automatic final escape at every text and + attribute sink so values stay readable without raw HTML. *} {foreach from=$mod_list item=mod} - +
- - {$mod.name} + {$mod.name|unescape:'html'}
{$mod.modfolder}{$mod.modfolder|unescape:'html'} {$mod.steam_universe} {if $mod.enabled} @@ -103,7 +107,7 @@ href="index.php?p=admin&c=mods&o=edit&id={$mod.mid|escape:'url'}" data-testid="editmod-link" data-tooltip="Edit" - aria-label="Edit mod {$mod.name|escape}"> + aria-label="Edit mod {$mod.name|unescape:'html'}"> {/if} @@ -127,11 +131,11 @@ type="button" data-action="mod-delete" data-mid="{$mod.mid}" - data-name="{$mod.name|escape}" + data-name="{$mod.name|unescape:'html'}" data-fallback-href="index.php?p=admin&c=mods" data-testid="deletemod-btn" data-tooltip="Delete" - aria-label="Delete mod {$mod.name|escape}"> + aria-label="Delete mod {$mod.name|unescape:'html'}"> {/if} @@ -151,17 +155,19 @@ {foreach from=$mod_list item=mod}
-
-
{$mod.name}
-
- {$mod.modfolder} - · SU {$mod.steam_universe} +
{$mod.name|unescape:'html'}
+
+ {$mod.modfolder|unescape:'html'} + SU {$mod.steam_universe}
{if $mod.enabled} @@ -179,7 +185,7 @@ href="index.php?p=admin&c=mods&o=edit&id={$mod.mid|escape:'url'}" data-testid="editmod-link-mobile" data-tooltip="Edit" - aria-label="Edit mod {$mod.name|escape}"> + aria-label="Edit mod {$mod.name|unescape:'html'}"> {/if} @@ -188,11 +194,11 @@ type="button" data-action="mod-delete" data-mid="{$mod.mid}" - data-name="{$mod.name|escape}" + data-name="{$mod.name|unescape:'html'}" data-fallback-href="index.php?p=admin&c=mods" data-testid="deletemod-btn-mobile" data-tooltip="Delete" - aria-label="Delete mod {$mod.name|escape}"> + aria-label="Delete mod {$mod.name|unescape:'html'}"> {/if} @@ -202,8 +208,22 @@ {/foreach}
{else} -
-

No mods configured yet.

+
+ +

No mods configured yet

+

Add a game mod before assigning it to bans or servers.

+ {if $permission_addmods} +
+ + + Add mod + +
+ {/if}
{/if}