From 90bdef6f62283f8c68a1a339b86008a2dc72308a Mon Sep 17 00:00:00 2001 From: William Jacoby Date: Thu, 12 Nov 2026 13:48:24 -0600 Subject: [PATCH 1/3] Fix tab contrast, small touch targets, doubled labels and no-JS forms - Inactive Knowledge Base tabs use the board's dark text colour, which passes WCAG AA on prosilver's tab colour (it was 3.7:1). - Article title links and the queue's buttons and menus are at least 24px high (WCAG 2.2, 2.5.8). - The post-as, also-list-in and Markdown fields on the posting form and the ACP field type menu had two labels each; the wrapping one is gone. - The ACP introduction settings are in the page from the start and the script hides them while the introduction is off, so they work without JavaScript. The custom-field filter and the drafts-only box get a Go button when JavaScript is off. - .gitignore is left out of release archives. - tests/browser/kb_form_access.js checks all of this on a seeded board. Co-Authored-By: Claude Opus 5.5 --- .gitattributes | 1 + adm/style/acp_knowledgebase_config.html | 6 +- adm/style/acp_knowledgebase_fields.html | 2 +- adm/style/js/acp_knowledgebase.js | 11 ++ controller/acp_controller.php | 1 - styles/all/template/kb_cat_body.html | 1 + styles/all/template/kb_my_articles_body.html | 1 + styles/all/template/kb_post_body.html | 8 +- .../all/theme/phpbbmodders_knowledgebase.css | 30 ++++ tests/browser/kb_form_access.js | 146 ++++++++++++++++++ 10 files changed, 197 insertions(+), 10 deletions(-) create mode 100644 tests/browser/kb_form_access.js diff --git a/.gitattributes b/.gitattributes index c5a925a..57a3fbb 100644 --- a/.gitattributes +++ b/.gitattributes @@ -13,3 +13,4 @@ phpunit.xml.* export-ignore tests export-ignore docs export-ignore .editorconfig export-ignore +.gitignore export-ignore diff --git a/adm/style/acp_knowledgebase_config.html b/adm/style/acp_knowledgebase_config.html index 366d3eb..aae2709 100644 --- a/adm/style/acp_knowledgebase_config.html +++ b/adm/style/acp_knowledgebase_config.html @@ -81,12 +81,10 @@

{{ lang('ACP_KNOWLEDGE_BASE_CONFIGURE') }}

{{ lang('KB_INTRO_ENABLE_EXPLAIN') }}
- +
-
+

diff --git a/adm/style/acp_knowledgebase_fields.html b/adm/style/acp_knowledgebase_fields.html index 646fe03..4dee08e 100644 --- a/adm/style/acp_knowledgebase_fields.html +++ b/adm/style/acp_knowledgebase_fields.html @@ -68,7 +68,7 @@

{{ lang('ACP_LIBRARY_FIELDS') }}

-
+

diff --git a/adm/style/js/acp_knowledgebase.js b/adm/style/js/acp_knowledgebase.js index f9beb5d..1e35827 100644 --- a/adm/style/js/acp_knowledgebase.js +++ b/adm/style/js/acp_knowledgebase.js @@ -10,4 +10,15 @@ $icon.attr('class', ''); } }); + // The introduction's text and Markdown options show only while it is + // switched on. They are in the page from the start, so the setting still + // works without JavaScript. + let $introEnabled = $('#intro_enabled'); + let $introOptions = $('#intro_options'); + if ($introEnabled.length && $introOptions.length) { + $introOptions.toggle($introEnabled.prop('checked')); + $introEnabled.on('change', function() { + $introOptions.toggle(this.checked); + }); + } })(jQuery); diff --git a/controller/acp_controller.php b/controller/acp_controller.php index 05b63bb..6ff852e 100644 --- a/controller/acp_controller.php +++ b/controller/acp_controller.php @@ -288,7 +288,6 @@ public function settings(): void 'ADVANCED_FORM' => $announce_forum_id ? '' : 'none', 'HIERARCHICAL_INDEX_ON' => $this->config['kb_hierarchical_index'] ? 'checked="checked"' : '', 'INTRO_ON' => $this->intro && $this->intro->is_enabled() ? 'checked="checked"' : '', - 'INTRO_DISPLAY' => $this->intro && $this->intro->is_enabled() ? '' : 'none', 'INTRO_TEXT' => $this->intro ? $this->intro->for_edit() : '', 'INTRO_MARKDOWN_ON' => $this->intro && $this->intro->is_markdown() ? 'checked="checked"' : '', 'ALT_STYLE_ON' => $this->config['kb_alt_style_enabled'] ? 'checked="checked"' : '', diff --git a/styles/all/template/kb_cat_body.html b/styles/all/template/kb_cat_body.html index d84c6dc..c10c5e6 100644 --- a/styles/all/template/kb_cat_body.html +++ b/styles/all/template/kb_cat_body.html @@ -71,6 +71,7 @@

{{ CATEGORY }}

{{ FIELD_FILTER_OPTIONS }} + {% if S_FIELD_FILTER_ACTIVE %} {{ lang('RESET_FILTER') }} {% endif %} diff --git a/styles/all/template/kb_my_articles_body.html b/styles/all/template/kb_my_articles_body.html index 15a54ba..d09930c 100644 --- a/styles/all/template/kb_my_articles_body.html +++ b/styles/all/template/kb_my_articles_body.html @@ -24,6 +24,7 @@

{{ lang('LIBRARY') }} » {{ l
+
diff --git a/styles/all/template/kb_post_body.html b/styles/all/template/kb_post_body.html index 7289d60..b4076a5 100644 --- a/styles/all/template/kb_post_body.html +++ b/styles/all/template/kb_post_body.html @@ -29,7 +29,7 @@

{{ lang('POST_A') }}

{% if not S_EDIT_POST and S_CAN_POST_AS_TEAM %}
-
+
{% endif %} {% if S_CAN_CHOOSE_SUBMIT_AS %} @@ -66,8 +66,8 @@

{{ lang('POST_A') }}


{{ lang('KB_CROSS_CATEGORIES_EXPLAIN') }}
-
+
{{ FIELD_WIDGETS }}
@@ -92,7 +92,7 @@

{{ lang('POST_A') }}


{{ lang('KB_USE_MARKDOWN_EXPLAIN') }}
-
+
{{ lang('YES') }}
{% INCLUDEJS '@phpbbmodders_knowledgebase/js/markdown_toggle.js' %} diff --git a/styles/all/theme/phpbbmodders_knowledgebase.css b/styles/all/theme/phpbbmodders_knowledgebase.css index 4fdb9c1..e85339c 100644 --- a/styles/all/theme/phpbbmodders_knowledgebase.css +++ b/styles/all/theme/phpbbmodders_knowledgebase.css @@ -291,6 +291,25 @@ table.kb-index-table .kb-index-author { overflow-wrap: break-word; } +/* + * Touch targets at least 24px high (WCAG 2.2, 2.5.8): the article title links + * and the queue's buttons and menus sat closer together than that on a phone. + */ +table.kb-index-table td:first-child > a { + display: inline-block; + min-height: 24px; + line-height: 24px; +} + +table.kb-index-table .button2, +table.kb-index-table select { + display: inline-block; + box-sizing: border-box; + min-height: 24px; + min-width: 24px; + vertical-align: middle; +} + /* * Headings in article text. Markdown articles can use all six levels, which * a forum post never does: prosilver paints an h1 white and large because it @@ -366,6 +385,17 @@ table.kb-index-table .kb-index-author { font-size: 0.95em; } +/* + * prosilver's grey-blue tab text is too faint for these small labels on its + * tab colour (3.7:1). The board's dark text colour keeps the style's tab + * background and passes WCAG AA (4.5:1). An alternative stylesheet can change + * it for a style with dark tabs. + */ +.tabs.kb-tabs .kb-tab:not(.kb-tab-active) > a, +.tabs.kb-tabs .kb-tab:not(.kb-tab-active) > a:hover { + color: #28313f; +} + .tabs.kb-tabs .kb-tab-active > a, .tabs.kb-tabs .kb-tab-active > a:hover { background: transparent; diff --git a/tests/browser/kb_form_access.js b/tests/browser/kb_form_access.js new file mode 100644 index 0000000..6c42fef --- /dev/null +++ b/tests/browser/kb_form_access.js @@ -0,0 +1,146 @@ +// Check the Knowledge Base forms work for people without JavaScript and +// carry one label per field, as the seeded administrator: +// - the ACP introduction settings: with JavaScript, the text box shows only +// while the introduction is ticked; without it, the text box is always there; +// - the custom-field filter and the "drafts only" box have a Go button when +// JavaScript is off (both normally submit on change); +// - no field on the add-article form, nor the ACP field type menu, has more than one label; +// - the inactive tabs' text passes WCAG AA contrast (4.5:1) on its background; +// - article title links and the queue's buttons and menus are at least 24px +// high on a phone. +// Read-only: it submits nothing but the login forms. +// +// KB_TEST_URL is the board's base URL, without a trailing slash; routes go +// through app.php, so the board needs no clean-URL rewriting. +const {chromium} = require('playwright'); + +const base = process.env.KB_TEST_URL; +if (!/^http:\/\/(localhost|127\.0\.0\.1):\d+$/.test(base || '')) throw new Error('Set KB_TEST_URL to a disposable local board, for example http://localhost:8171'); +const password = 'KbTest1234!'; +const ready = {waitUntil: 'domcontentloaded'}; +const results = []; + +function record(label, ok, detail = '') { + results.push({label, ok, detail}); +} + +// phpBB refuses a form submitted within two seconds of it being made +async function waitForFormAge(page) { + await page.waitForFunction(() => { + const field = document.querySelector('[name="creation_time"]'); + return field && Date.now() / 1000 > Number(field.value) + 2; + }); +} + +// Log in to the board, then unlock the ACP, in a fresh browser context +async function adminPage(browser, javaScriptEnabled) { + const context = await browser.newContext({javaScriptEnabled, viewport: {width: 1280, height: 900}}); + const page = await context.newPage(); + page.setDefaultTimeout(120000); + await page.goto(`${base}/ucp.php?mode=login`, ready); + await page.locator('#username').fill('admin'); + await page.locator('#password').fill(password); + if (javaScriptEnabled) await waitForFormAge(page); else await page.waitForTimeout(2500); + await Promise.all([page.waitForNavigation(ready), page.locator('[name="login"][type="submit"]').click()]); + return page; +} + +async function openAcp(page, path) { + const sid = (await page.context().cookies()).find(item => item.name.endsWith('_sid')); + await page.goto(`${base}/adm/index.php?${path}&sid=${sid ? sid.value : ''}`, ready); + if (await page.locator('input[type="password"]').count()) { + await page.locator('input[type="password"]').fill(password); + await page.waitForTimeout(2500); + await Promise.all([page.waitForNavigation(ready), page.locator('[name="login"][type="submit"]').click()]); + const now = (await page.context().cookies()).find(item => item.name.endsWith('_sid')); + await page.goto(`${base}/adm/index.php?${path}&sid=${now ? now.value : ''}`, ready); + } +} + +// Fields that more than one