From 98e52d5430f7106ab61d348dc5fcea5c60d62151 Mon Sep 17 00:00:00 2001 From: codad5 Date: Mon, 12 Oct 2026 14:48:31 +0100 Subject: [PATCH 1/2] feat(data): render multiple choices as checkboxes with a chip picker A select with multiple() was the browser's `. A plain click selects one +item and clears the rest (Ctrl/Cmd+click adds), which most people don't know, it looks different in every +browser and is hard to use on touch screens. 0.x plugins fixed this with jQuery "Chosen". + +It also had a correctness problem: with nothing selected the browser submits nothing, the meta box +treats a missing key as "not on this form", and a saved selection could never be cleared. + +## Decision + +1. A multiple-choice field renders as **real checkboxes**: up to six options a plain labelled list, more + than that a **chip picker** (chips for the selection, a search box, a panel of options). `display()` + overrides: `checkboxes`, `tokens`, or `native` for the old select. +2. The picker is **progressive enhancement over the checkboxes**. The checkboxes stay in the page and + remain what the form submits, so keyboard and screen-reader use are the platform's own, a script + failure leaves a usable list, and the stored value is exactly what it was. +3. **No library.** About 120 lines of plain JavaScript and 50 of CSS, no jQuery (rule 5 is about + Composer dependencies, but shipping and scoping a third-party JS widget, and its jQuery + requirement, is the same cost). Chosen is unmaintained; Select2 needs jQuery. +4. The script and style are **printed once in the page footer** by the first picker on the page, from the + library's `resources/` files; two bundled copies of the library may both print them, and an element + is enhanced by whichever runs first. +5. **Every rendering sends an empty marker** (`name[]` with an empty value) first, so "nothing chosen" + arrives and clears the field. Validation ignores empty entries of a multiple field; the stored + format is unchanged (ADR-0016). + +## Options considered + +- **Keep `` (which selects one item per click and cannot be cleared once something is saved): + +- up to 6 options: a plain list of labelled checkboxes; +- more: a searchable **chip picker**. Type to filter, Enter picks the first match, Backspace removes the + last chip, Escape closes, every chip has a remove button. It is a small piece of plain JavaScript + (no jQuery, no library) that enhances the checkboxes, so keyboard and screen-reader use work the + same, and without JavaScript the list is still usable. + +Choose a look with `->display('checkboxes' | 'tokens' | 'native')`; `native` keeps the old select. +The form always sends an empty marker, so unticking everything is saved as "nothing chosen". What is +stored is unchanged: one serialized array row, as 0.x stored it +([ADR-0027](../adr/0027-multiple-choice-fields-render-as-checkboxes-with-an-optional-picker.md)). + ## Meta boxes ```php diff --git a/examples/todo/src/Todo.php b/examples/todo/src/Todo.php index 2d46369..fbcf48b 100644 --- a/examples/todo/src/Todo.php +++ b/examples/todo/src/Todo.php @@ -34,6 +34,17 @@ public static function fields(FieldFactory $f): array 'high' => __('High', 'wptk-todo'), 'urgent' => __('Urgent', 'wptk-todo'), ])->label(__('Priority', 'wptk-todo'))->default('medium')->required(), + // More than six options, so it renders as the searchable chip picker. + $f->select('labels', [ + 'home' => __('Home', 'wptk-todo'), + 'work' => __('Work', 'wptk-todo'), + 'errand' => __('Errand', 'wptk-todo'), + 'finance' => __('Finance', 'wptk-todo'), + 'health' => __('Health', 'wptk-todo'), + 'learning' => __('Learning', 'wptk-todo'), + 'travel' => __('Travel', 'wptk-todo'), + 'urgent-call' => __('Urgent call', 'wptk-todo'), + ])->label(__('Labels', 'wptk-todo'))->multiple(), $f->select('status', [ 'pending' => __('Pending', 'wptk-todo'), 'in_progress' => __('In progress', 'wptk-todo'), diff --git a/languages/wptoolkit.pot b/languages/wptoolkit.pot index 2c0e039..5b670b2 100644 --- a/languages/wptoolkit.pot +++ b/languages/wptoolkit.pot @@ -7,7 +7,7 @@ msgstr "" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" -"POT-Creation-Date: 2026-10-12T08:02:38+00:00\n" +"POT-Creation-Date: 2026-10-12T13:48:18+00:00\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "X-Generator: WP-CLI 2.12.0\n" "X-Domain: wptoolkit\n" @@ -113,6 +113,20 @@ msgstr "" msgid "Details" msgstr "" +#: src/Data/Field/Types/ChoiceType.php:152 +msgid "Search…" +msgstr "" + +#. translators: %s: name of a selected item +#: src/Data/Field/Types/ChoiceType.php:154 +#, php-format +msgid "Remove %s" +msgstr "" + +#: src/Data/Field/Types/ChoiceType.php:155 +msgid "No matches" +msgstr "" + #: src/Data/Field/Types/InputType.php:66 msgid "Saved — leave blank to keep" msgstr "" diff --git a/resources/css/picker.css b/resources/css/picker.css new file mode 100644 index 0000000..73b6185 --- /dev/null +++ b/resources/css/picker.css @@ -0,0 +1,40 @@ +/* WPToolkit multiple-choice picker. A plain checkbox list needs only the first rules. */ +.wptoolkit-picker { border: 0; margin: 0; padding: 0; min-width: 0; position: relative; } +.wptoolkit-picker > label, +.wptoolkit-picker__panel > label { display: block; margin: 0 0 4px; } + +.wptoolkit-picker[data-wptoolkit-ready] .wptoolkit-picker__control { + display: flex; flex-wrap: wrap; align-items: center; gap: 4px; box-sizing: border-box; + min-height: 30px; max-width: 25em; padding: 2px 6px; background: #fff; cursor: text; + border: 1px solid #8c8f94; border-radius: 4px; +} +.wptoolkit-picker.is-open .wptoolkit-picker__control { + border-color: var(--wp-admin-theme-color, #2271b1); + box-shadow: 0 0 0 1px var(--wp-admin-theme-color, #2271b1); +} +.wptoolkit-picker__chips { display: contents; list-style: none; margin: 0; padding: 0; } +.wptoolkit-picker__chip { + display: inline-flex; align-items: center; gap: 2px; margin: 0; padding: 1px 2px 1px 8px; + background: #f0f0f1; border: 1px solid #c3c4c7; border-radius: 3px; line-height: 1.6; +} +.wptoolkit-picker__remove { + all: unset; box-sizing: border-box; display: inline-flex; align-items: center; justify-content: center; + width: 20px; height: 20px; border-radius: 3px; cursor: pointer; font-size: 16px; line-height: 1; color: #50575e; +} +.wptoolkit-picker__remove:hover { background: #dcdcde; color: #1d2327; } +.wptoolkit-picker__remove:focus-visible { outline: 2px solid var(--wp-admin-theme-color, #2271b1); outline-offset: 0; } +.wptoolkit-picker .wptoolkit-picker__search { + flex: 1 1 8em; min-width: 6em; margin: 0; padding: 0 2px; border: 0; background: transparent; box-shadow: none; min-height: 26px; +} +.wptoolkit-picker .wptoolkit-picker__search:focus { outline: 0; box-shadow: none; } + +.wptoolkit-picker[data-wptoolkit-ready] .wptoolkit-picker__panel { + display: none; position: absolute; inset-inline-start: 0; z-index: 1000; box-sizing: border-box; + width: 100%; max-width: 25em; max-height: 14em; overflow: auto; margin-top: 2px; padding: 4px 0; + background: #fff; border: 1px solid #8c8f94; border-radius: 4px; box-shadow: 0 3px 8px rgba(0, 0, 0, .15); +} +.wptoolkit-picker.is-open .wptoolkit-picker__panel { display: block; } +.wptoolkit-picker[data-wptoolkit-ready] .wptoolkit-picker__panel > label { margin: 0; padding: 4px 10px; } +.wptoolkit-picker[data-wptoolkit-ready] .wptoolkit-picker__panel > label:hover { background: #f0f6fc; } +.wptoolkit-picker__panel > label[hidden], .wptoolkit-picker__empty[hidden] { display: none; } +.wptoolkit-picker__empty { margin: 0; padding: 6px 10px; color: #646970; } diff --git a/resources/js/picker.js b/resources/js/picker.js new file mode 100644 index 0000000..2243681 --- /dev/null +++ b/resources/js/picker.js @@ -0,0 +1,141 @@ +/*! + * WPToolkit multiple-choice picker: turns a checkbox list into a searchable chip picker. + * + * The checkboxes stay in the page and stay the source of truth, so the form submits exactly what it + * would without this script, keyboard use works the same, and a failure here leaves a usable list. + * No library and no jQuery. Plugins that bundle different copies of WPToolkit each print this file; + * an element is enhanced by whichever runs first. + */ +(function () { + 'use strict'; + + var READY = 'data-wptoolkit-ready'; + + function make(tag, className, attributes) { + var node = document.createElement(tag); + if (className) { node.className = className; } + Object.keys(attributes || {}).forEach(function (name) { node.setAttribute(name, attributes[name]); }); + return node; + } + + function toggle(box, checked) { + box.checked = checked; + box.dispatchEvent(new Event('change', { bubbles: true })); + } + + function enhance(root) { + if (root.getAttribute('data-wptoolkit-picker') !== 'tokens' || root.hasAttribute(READY)) { return; } + var boxes = [].slice.call(root.querySelectorAll('input[type="checkbox"]')); + if (!boxes.length) { return; } + root.setAttribute(READY, '1'); + + var labels = { + search: root.getAttribute('data-label-search') || 'Search…', + remove: root.getAttribute('data-label-remove') || 'Remove %s', + none: root.getAttribute('data-label-none') || 'No matches' + }; + var options = boxes.map(function (box) { + var row = box.closest('label'); + return { box: box, row: row, text: (row.textContent || '').trim() }; + }); + + var control = make('div', 'wptoolkit-picker__control'); + var chips = make('ul', 'wptoolkit-picker__chips', { 'aria-live': 'polite' }); + var search = make('input', 'wptoolkit-picker__search', { + type: 'text', autocomplete: 'off', placeholder: labels.search, 'aria-label': labels.search + }); + var panel = make('div', 'wptoolkit-picker__panel'); + var empty = make('p', 'wptoolkit-picker__empty', { hidden: 'hidden' }); + empty.textContent = labels.none; + + options.forEach(function (option) { panel.appendChild(option.row); }); + panel.appendChild(empty); + control.appendChild(chips); + control.appendChild(search); + root.appendChild(control); + root.appendChild(panel); + + function open(show) { root.classList.toggle('is-open', show); } + + function visibleOptions() { + return options.filter(function (option) { return !option.row.hidden; }); + } + + function filter() { + var query = search.value.trim().toLowerCase(); + options.forEach(function (option) { + option.row.hidden = query !== '' && option.text.toLowerCase().indexOf(query) === -1; + }); + empty.hidden = visibleOptions().length > 0; + } + + function renderChips() { + chips.textContent = ''; + options.forEach(function (option) { + if (!option.box.checked) { return; } + var chip = make('li', 'wptoolkit-picker__chip'); + var text = make('span'); + text.textContent = option.text; + var remove = make('button', 'wptoolkit-picker__remove', { + type: 'button', 'aria-label': labels.remove.replace('%s', option.text) + }); + remove.textContent = '×'; + remove.addEventListener('click', function () { + toggle(option.box, false); + search.focus(); + }); + chip.appendChild(text); + chip.appendChild(remove); + chips.appendChild(chip); + }); + } + + boxes.forEach(function (box) { box.addEventListener('change', renderChips); }); + search.addEventListener('input', function () { filter(); open(true); }); + search.addEventListener('focus', function () { open(true); }); + control.addEventListener('click', function () { search.focus(); open(true); }); + + search.addEventListener('keydown', function (event) { + if (event.key === 'Enter') { + // Enter picks the first match instead of submitting the form. + event.preventDefault(); + var first = visibleOptions()[0]; + if (first) { + toggle(first.box, !first.box.checked); + search.value = ''; + filter(); + } + } else if (event.key === 'Backspace' && search.value === '') { + var selected = options.filter(function (option) { return option.box.checked; }); + var last = selected[selected.length - 1]; + if (last) { toggle(last.box, false); } + } else if (event.key === 'Escape') { + open(false); + } + }); + root.addEventListener('keydown', function (event) { + if (event.key === 'Escape') { open(false); search.focus(); } + }); + root.addEventListener('focusout', function (event) { + if (!event.relatedTarget || !root.contains(event.relatedTarget)) { open(false); } + }); + document.addEventListener('mousedown', function (event) { + if (!root.contains(event.target)) { open(false); } + }); + if (root.form) { + root.form.addEventListener('reset', function () { window.setTimeout(renderChips, 0); }); + } + + renderChips(); + } + + function init() { + [].slice.call(document.querySelectorAll('[data-wptoolkit-picker="tokens"]')).forEach(enhance); + } + + if (document.readyState === 'loading') { + document.addEventListener('DOMContentLoaded', init); + } else { + init(); + } +}()); diff --git a/src/Data/Field/Field.php b/src/Data/Field/Field.php index 7238a48..47e4c03 100644 --- a/src/Data/Field/Field.php +++ b/src/Data/Field/Field.php @@ -78,6 +78,20 @@ public function multiple(bool $multiple = true): self return $this->with('multiple', $multiple); } + /** + * How a multiple-choice field looks: 'checkboxes' (a plain list), 'tokens' (chips with search; the + * checkboxes are enhanced with JavaScript) or 'native' (the browser's `', Html::attributes([ 'id' => $id, @@ -88,4 +95,73 @@ public function render(Field $field, string $name, string $id, mixed $value): st $options ); } + + /** + * Sent first so that ticking nothing still submits the field. Without it the key is missing, the + * save treats the field as "not on this form", and a selection could never be cleared. + */ + private static function emptyMarker(string $name): string + { + return sprintf('', esc_attr($name)); + } + + private function displayOf(Field $field): string + { + $display = $field->setting('display'); + if (is_string($display)) { + return $display; + } + + return count($field->choices()) <= self::MAX_PLAIN_CHECKBOXES ? 'checkboxes' : 'tokens'; + } + + /** + * A multiple choice as real checkboxes. With 'tokens' a script (ChoicePickerAssets) turns the list + * into chips with search; the checkboxes remain the source of truth either way. + * + * @param list $selected + */ + private function renderCheckboxes(Field $field, string $name, string $id, array $selected): string + { + $mode = $this->displayOf($field); + $attributes = $field->htmlAttributes(); + $class = trim('wptoolkit-picker ' . (isset($attributes['class']) ? (string) $attributes['class'] : '')); + unset($attributes['class']); + + $boxes = ''; + foreach ($field->choices() as $option => $label) { + $boxId = $id . '-' . sanitize_key((string) $option); + $boxes .= sprintf( + '', + esc_attr($boxId), + Html::attributes([ + 'type' => 'checkbox', + 'id' => $boxId, + 'name' => $name . '[]', + 'value' => (string) $option, + 'checked' => in_array((string) $option, $selected, true), + ]), + esc_html($label) + ); + } + + $labels = []; + if ($mode === 'tokens') { + $this->assets?->queue(); + $labels = [ + 'data-label-search' => __('Search…', 'wptoolkit'), + /* translators: %s: name of a selected item */ + 'data-label-remove' => __('Remove %s', 'wptoolkit'), + 'data-label-none' => __('No matches', 'wptoolkit'), + ]; + } + + return sprintf( + '%s%s%s', + Html::attributes(['id' => $id, 'class' => $class, 'data-wptoolkit-picker' => $mode] + $labels + $attributes), + esc_html($field->labelText()), + self::emptyMarker($name), + $boxes + ); + } } diff --git a/tests/E2E/todo-example.spec.ts b/tests/E2E/todo-example.spec.ts index 54c09e0..d0ddd67 100644 --- a/tests/E2E/todo-example.spec.ts +++ b/tests/E2E/todo-example.spec.ts @@ -111,6 +111,59 @@ test.describe.serial('the todo example (Phase 5 DoD)', () => { expect(problems(takeDebugLog())).toEqual([]); }); + test('a long multiple-choice field is a chip picker: search, pick, remove, save, clear', async ({ page }) => { + const pageErrors: string[] = []; + page.on('pageerror', (error) => pageErrors.push(error.message)); + const key = 'todo_details_wptk_todo_labels'; + + await page.goto(NEW_TODO); + await page.fill('#title', 'Picker todo'); + const picker = page.locator('[data-wptoolkit-picker="tokens"]'); + await expect(picker).toHaveAttribute('data-wptoolkit-ready', '1'); + const search = picker.getByRole('textbox', { name: 'Search…' }); + const panel = picker.locator('.wptoolkit-picker__panel'); + const chips = picker.locator('.wptoolkit-picker__chip span'); + + await expect(panel, 'closed until the search box is used').toBeHidden(); + await search.click(); + await expect(panel).toBeVisible(); + + await search.fill('fin'); + await expect(panel.locator('label:visible')).toHaveCount(1); + await page.keyboard.press('Enter'); // picks the first match instead of submitting the form + await search.fill('tra'); + await page.keyboard.press('Enter'); + await search.fill('zzz'); + await expect(picker.locator('.wptoolkit-picker__empty')).toBeVisible(); + await search.fill(''); + await expect(chips).toHaveText(['Finance', 'Travel']); + + await picker.getByRole('button', { name: 'Remove Travel' }).click(); + await expect(chips).toHaveText(['Finance']); + await page.keyboard.press('Escape'); + await expect(panel).toBeHidden(); + + await page.click('#publish'); + await expect(page.locator('#message')).toBeVisible(); + const postId = new URL(page.url()).searchParams.get('post') ?? ''; + expect(postId).not.toBe(''); + + // Saved exactly as before: one serialized row holding the chosen values. + expect(wp('post', 'meta', 'get', postId, key, '--format=json').output.trim()).toBe('["finance"]'); + expect(wp('post', 'meta', 'list', postId, `--keys=${key}`, '--format=count').output.trim()).toBe('1'); + await expect(chips).toHaveText(['Finance']); + + // Removing the last choice and saving must clear it (an empty selection is still sent). + await picker.getByRole('button', { name: 'Remove Finance' }).click(); + await page.click('#publish'); + await expect(page.locator('#message')).toBeVisible(); + expect(wp('post', 'meta', 'list', postId, `--keys=${key}`, '--format=count').output.trim()).toBe('0'); + await expect(picker.locator('.wptoolkit-picker__chip')).toHaveCount(0); + + expect(pageErrors).toEqual([]); + expect(problems(takeDebugLog())).toEqual([]); + }); + test('Arabic: screens mirror right to left and use the example’s translations', async ({ page }) => { expect(wp('language', 'core', 'install', 'ar').code).toBe(0); expect(wp('site', 'switch-language', 'ar').code).toBe(0); diff --git a/tests/Unit/Data/FieldTest.php b/tests/Unit/Data/FieldTest.php index 888382b..98db9f1 100644 --- a/tests/Unit/Data/FieldTest.php +++ b/tests/Unit/Data/FieldTest.php @@ -111,10 +111,15 @@ public function test_rendered_values_and_options_are_escaped(): void public function test_multiple_select_submits_an_array(): void { - $html = $this->types->get('select')->render($this->f->select('tags', ['a' => 'A'])->multiple(), 'tags', 'tags', ['a']); + $field = $this->f->select('tags', ['a' => 'A'])->multiple(); - self::assertStringContainsString('name="tags[]"', $html); - self::assertStringContainsString(' multiple', $html); + $default = $this->types->get('select')->render($field, 'tags', 'tags', ['a']); + self::assertStringContainsString('name="tags[]"', $default); + self::assertStringContainsString('type="checkbox"', $default, 'multiple choices are checkboxes by default'); + + $native = $this->types->get('select')->render($field->display('native'), 'tags', 'tags', ['a']); + self::assertStringContainsString('name="tags[]"', $native); + self::assertStringContainsString(' multiple', $native); } public function test_custom_types_plug_in_without_changing_the_library(): void diff --git a/tests/Unit/Data/MetaBoxTest.php b/tests/Unit/Data/MetaBoxTest.php index 39533ef..ef0dbbd 100644 --- a/tests/Unit/Data/MetaBoxTest.php +++ b/tests/Unit/Data/MetaBoxTest.php @@ -168,6 +168,29 @@ public function test_blank_sensitive_field_keeps_the_stored_secret(): void self::assertSame(['sk_live'], $this->wp->meta[5]['d_book_api_key']); } + public function test_deselecting_every_choice_clears_a_multiple_field(): void + { + // The form sends the empty marker even with nothing ticked; without it, the key is absent, + // the field is "not on this form", and the old selection could never be removed. + $this->wp->meta[5] = ['d_book_tags' => [['a', 'b']]]; + $box = $this->box('d', 'book', [$this->f->select('tags', ['a' => 'A', 'b' => 'B'])->multiple()]); + $_POST = [$box->nonceName() => 'n', 'd_book_tags' => ['']]; + + $box->handleSave(5, $this->post(5, 'book')); + + self::assertArrayNotHasKey('d_book_tags', $this->wp->meta[5]); + } + + public function test_the_empty_marker_does_not_disturb_a_real_selection(): void + { + $box = $this->box('d', 'book', [$this->f->select('tags', ['a' => 'A', 'b' => 'B'])->multiple()]); + $_POST = [$box->nonceName() => 'n', 'd_book_tags' => ['', 'b']]; + + $box->handleSave(5, $this->post(5, 'book')); + + self::assertSame([['b']], $this->wp->meta[5]['d_book_tags']); + } + public function test_fields_missing_from_the_form_are_left_alone(): void { $this->wp->meta[5] = ['d_book_title' => ['Kept']]; diff --git a/tests/Unit/Data/MultipleChoiceTest.php b/tests/Unit/Data/MultipleChoiceTest.php new file mode 100644 index 0000000..6819073 --- /dev/null +++ b/tests/Unit/Data/MultipleChoiceTest.php @@ -0,0 +1,171 @@ + selects one item per click and can't be cleared + * (nothing selected submits nothing), so they render as checkbox lists, enhanced into a searchable + * chip picker. The checkboxes stay the source of truth. + */ +final class MultipleChoiceTest extends TestCase +{ + private FieldFactory $f; + + private FieldTypes $types; + + protected function setUp(): void + { + parent::setUp(); + $this->f = new FieldFactory(); + $this->types = new FieldTypes(); + } + + /** + * @param array $options + */ + private function render(array $options, mixed $value = [], ?string $display = null, string $name = 'tags'): string + { + $field = $this->f->select($name, $options)->multiple(); + if ($display !== null) { + $field = $field->display($display); + } + + return $this->types->get('select')->render($field->label('Tags'), $name, $name, $value); + } + + public function test_a_short_list_is_a_checkbox_group_that_submits_an_array(): void + { + $html = $this->render(['a' => 'Alpha', 'b' => 'Beta'], ['b']); + + self::assertStringContainsString(']* checked/', $html); + self::assertDoesNotMatchRegularExpression('/value="a"[^>]* checked/', $html); + self::assertStringNotContainsString('render($options)); + } + + public function test_the_mode_can_be_chosen_explicitly(): void + { + $options = ['a' => 'A', 'b' => 'B']; + + self::assertStringContainsString('data-wptoolkit-picker="tokens"', $this->render($options, [], 'tokens')); + self::assertStringContainsString('render($options, [], 'native')); + self::assertStringContainsString(' multiple', $this->render($options, [], 'native')); + } + + public function test_an_unknown_mode_is_refused_when_the_field_is_defined(): void + { + $this->expectException(InvalidConfigException::class); + $this->f->select('tags', ['a' => 'A'])->multiple()->display('fancy'); + } + + public function test_every_display_sends_an_empty_marker_so_deselecting_everything_is_saved(): void + { + foreach ([null, 'checkboxes', 'tokens', 'native'] as $display) { + $html = $this->render(['a' => 'A', 'b' => 'B'], ['a'], $display); + + self::assertStringContainsString('', $html, (string) $display); + self::assertLessThan( + strpos($html, 'name="tags[]"', strpos($html, 'type="hidden"') + 10) ?: PHP_INT_MAX, + strpos($html, 'type="hidden"'), + 'the marker comes first (' . $display . ')' + ); + } + } + + public function test_options_values_and_labels_are_escaped_and_ids_are_unique(): void + { + $html = $this->render(['x">' => '', 'plain' => 'Plain'], [], 'checkboxes'); + + self::assertStringNotContainsString('render(['a' => 'Alpha'], [], 'checkboxes'); + + self::assertMatchesRegularExpression('/]*>.*]*type="checkbox".*Alpha/s', $html); + self::assertStringContainsString('Tags', $html); + } + + public function test_a_single_select_and_radio_are_unchanged(): void + { + $single = $this->types->get('select')->render($this->f->select('size', ['s' => 'S', 'm' => 'M']), 'size', 'size', 'm'); + + self::assertStringStartsWith('types->get('radio')->render($this->f->radio('r', ['a' => 'A']), 'r', 'r', 'a')); + } + + public function test_the_picker_script_and_style_are_printed_once_in_the_footer_however_many_fields(): void + { + $printers = []; + Actions\expectAdded('admin_print_footer_scripts')->zeroOrMoreTimes()->whenHappen(static function ($callback) use (&$printers): void { + $printers[] = $callback; + }); + Actions\expectAdded('wp_print_footer_scripts')->zeroOrMoreTimes()->whenHappen(static function ($callback) use (&$printers): void { + $printers[] = $callback; + }); + $options = array_combine(range('a', 'h'), range('A', 'H')); + + $this->render($options, [], null, 'one'); + $this->render($options, [], null, 'two'); + $this->render(['a' => 'A'], [], 'checkboxes', 'three'); + + self::assertNotEmpty($printers, 'assets are queued for the footer, not printed inline'); + ob_start(); + foreach ($printers as $printer) { + $printer(); + } + $output = (string) ob_get_clean(); + + self::assertSame(1, substr_count($output, ''), 'the script holds no closing tag that would end it early'); + } + + public function test_checkbox_only_lists_need_no_script(): void + { + $printers = []; + Actions\expectAdded('admin_print_footer_scripts')->zeroOrMoreTimes()->whenHappen(static function ($callback) use (&$printers): void { + $printers[] = $callback; + }); + + $this->render(['a' => 'A', 'b' => 'B'], [], 'checkboxes'); + + self::assertSame([], $printers); + } + + public function test_empty_entries_are_ignored_when_validating_a_multiple_field(): void + { + $validator = new FieldValidator($this->types); + $field = $this->f->select('tags', ['a' => 'A', 'b' => 'B'])->multiple(); + + self::assertSame([], $validator->validate($field, ['', 'a']), 'the empty marker is not a choice'); + self::assertSame([], $validator->validate($field, ['']), 'nothing chosen is fine when the field is optional'); + self::assertNotSame([], $validator->validate($field->required(), ['']), 'and counts as nothing when it is required'); + self::assertNotSame([], $validator->validate($field, ['', 'zzz']), 'a real bad value is still caught'); + } +} From 441cc9b3bee5089fdc406fbcb09e645622456b20 Mon Sep 17 00:00:00 2001 From: codad5 Date: Mon, 12 Oct 2026 14:48:32 +0100 Subject: [PATCH 2/2] feat(cli): lock, write atomically and clean up builds; add a spinner One build per project runs at a time (OS file lock; waitForLock() or --wait to queue). Zips and directories are written beside their target and moved into place, so a failed or stopped build leaves the previous output whole. Ctrl+C stops the running command and what it started and removes staging; abandoned staging is swept. Steps end with their duration, and an interactive terminal gets a spinner that never reaches logs. The runner polls a file the shell redirects into, because a descriptor handed to proc_open lost output in long test runs. See ADR-0026. Co-Authored-By: Claude Sonnet 5.5 --- ...tomically-and-clean-up-after-themselves.md | 64 +++ docs/adr/README.md | 2 + docs/guides/getting-started.md | 8 + docs/phases/track-p-packager.md | 2 + docs/reference/work-log.md | 7 + llms.txt | 3 +- packages/build/bin/wptoolkit-build | 12 +- .../build/src/BackgroundCommandRunner.php | 29 ++ packages/build/src/Build.php | 115 ++++- packages/build/src/BuildContext.php | 88 +++- packages/build/src/BuildLock.php | 109 +++++ packages/build/src/DirectoryPackager.php | 60 ++- packages/build/src/Interruption.php | 107 +++++ packages/build/src/JsonConfig.php | 6 + packages/build/src/ProcessCommandRunner.php | 102 +++- packages/build/src/Progress.php | 174 +++++++ packages/build/src/ZipPackager.php | 81 +++- tests/Unit/Build/BuildRobustnessTest.php | 436 ++++++++++++++++++ tests/Unit/Build/ProgressTest.php | 202 ++++++++ 19 files changed, 1537 insertions(+), 70 deletions(-) create mode 100644 docs/adr/0026-builds-lock-write-atomically-and-clean-up-after-themselves.md create mode 100644 packages/build/src/BackgroundCommandRunner.php create mode 100644 packages/build/src/BuildLock.php create mode 100644 packages/build/src/Interruption.php create mode 100644 packages/build/src/Progress.php create mode 100644 tests/Unit/Build/BuildRobustnessTest.php create mode 100644 tests/Unit/Build/ProgressTest.php diff --git a/docs/adr/0026-builds-lock-write-atomically-and-clean-up-after-themselves.md b/docs/adr/0026-builds-lock-write-atomically-and-clean-up-after-themselves.md new file mode 100644 index 0000000..edce3bb --- /dev/null +++ b/docs/adr/0026-builds-lock-write-atomically-and-clean-up-after-themselves.md @@ -0,0 +1,64 @@ +# ADR-0026 — Builds take a lock, write atomically and clean up after themselves + +- **Status:** Accepted +- **Date:** 2026-10-12 +- **Deciders:** codad5 +- **Affects:** `packages/build` (`BuildLock`, `Interruption`, `Progress`, `ZipPackager`, `DirectoryPackager`, `ProcessCommandRunner`) + +## Context + +The packager copied the project, ran commands, and wrote the zip directly to its final name. Three +things went wrong, or could: + +- Two builds of one project (two CI jobs sharing a workspace, a watcher firing during a build) ran + `npm run build` in the same folder and wrote the same output; a reader could pick up a half-written zip. +- A build killed part-way (Ctrl+C, a CI timeout) left its temporary copy behind, and possibly a partial + zip or a half-emptied output folder that looked like a real one. A command it had started could keep + running. +- A slow step printed nothing, so a two-minute asset build looked hung. + +## Decision + +1. **One build per project at a time.** An operating-system file lock (`flock`) on a file in the system + temp directory, named from the project's resolved path. The system releases it when the process + ends however it ends, so there are no stale locks to clear. A second build fails at once saying + which process holds it, or waits (`waitForLock()`, `--wait`). The lock is outside the project so it + is never shipped or committed. +2. **Output is atomic.** A zip is written next to its target under a temporary name and renamed into + place when complete, then its checksum; a directory is built beside its target and swapped in, with + the previous one put back if the swap fails. A failed or stopped build leaves the previous output + whole. +3. **Interruption is cooperative.** SIGINT/SIGTERM (pcntl) or Ctrl+C (Windows) set a flag; the build + checks it between steps, several times a second while a command runs, and before each file it + writes. It stops by throwing, so the same cleanup runs as for any failure: the command and its + children are terminated (`taskkill /T` on Windows, `pkill -P` elsewhere), staging and partial output + are removed, the lock is released. Staging copies older than six hours left by builds that were + killed outright are swept at the start of the next build; recent ones may belong to a running build. +4. **Commands are watched, not blocked on.** The runner polls a temporary file the shell redirects the + command's output into, so it can call back several times a second without a non-blocking pipe, which + PHP cannot provide on Windows. The shell does the redirect, not a descriptor passed to `proc_open`: in + a long test run the descriptor form lost the child's output after certain earlier tests, for a cause + that was not identified. +5. **Progress is plain lines everywhere, plus a spinner on a real terminal.** Every step ends with its + duration. The spinner is drawn in place and erased when its step ends, so the screen shows what a log + would. It exists only when stdout is an interactive terminal and none of `CI`, `NO_COLOR`, + `TERM=dumb`, `quiet()`, `plain()` or a caller-supplied `withOutput()` handler applies, and on Windows + only if virtual-terminal sequences can be enabled. A command whose output is streamed shows that + output instead. + +## Options considered + +- **No lock; document "don't run two builds".** Cheap, but the failure is silent corruption. +- **A lock file with a PID inside.** Needs stale-lock detection, which is platform-specific and racy; + `flock` gets this for free. +- **Handle signals only with `pcntl`.** Leaves Windows, where most of this project's authors work, with + nothing; Windows has its own control handler. +- **A spinner library.** A dependency for forty lines of drawing, and it would not know when to stay + plain. + +## Consequences + +- A build that is killed with SIGKILL or a power cut still can't clean up; the sweep and the atomic + writes bound the damage to a temporary directory and a temporary file next to the output. +- `toDirectory()` needs room for two copies of the output while it swaps. +- Tests exercise real processes, real locks and, on Linux, a real SIGINT. diff --git a/docs/adr/README.md b/docs/adr/README.md index 5898a92..ff98e28 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -58,4 +58,6 @@ Not for a naming choice, a library with no real alternative, or anything you wou | [0022](0022-shortcodes-are-declared-validated-and-lazy.md) | Shortcodes are declared, validated, access-checked and load assets lazily | Accepted | | [0023](0023-settings-may-keep-a-legacy-option-prefix.md) | Settings may keep an option prefix that already exists in users' data | Accepted | | [0024](0024-the-packager-scopes-the-consumers-code-too.md) | The packager scopes the consumer's code and autoloader, not only the library | Accepted | +| [0027](0027-multiple-choice-fields-render-as-checkboxes-with-an-optional-picker.md) | Multiple-choice fields are checkboxes, with an optional chip picker | Accepted | +| [0026](0026-builds-lock-write-atomically-and-clean-up-after-themselves.md) | Builds take a lock, write atomically and clean up after themselves | Accepted | | [0025](0025-the-guard-can-autoload-the-plugins-own-namespaces.md) | The guard can autoload the plugin's own namespaces (completes ADR-0014) | Accepted | diff --git a/docs/guides/getting-started.md b/docs/guides/getting-started.md index e421934..e8e7210 100644 --- a/docs/guides/getting-started.md +++ b/docs/guides/getting-started.md @@ -136,6 +136,14 @@ $files = Build::plugin(__DIR__)->dryRun(); // what the commit to SVN, to mount the built plugin in a test site, or to inspect what ships. The folder is **replaced**, so it must be for output only: the project itself, a folder above it, or an existing folder that is neither empty nor an earlier build of this project is refused. +- **Safe to run twice, and to stop.** One build per project runs at a time: a second one fails with who holds + the lock, or waits with `->waitForLock(60)` / `--wait=60`. Output is written under a temporary name + and moved into place when complete, so a failed or stopped build leaves the previous zip (or folder) + whole. Ctrl+C stops the running command and what it started and removes the temporary copy. + ([ADR-0026](../adr/0026-builds-lock-write-atomically-and-clean-up-after-themselves.md)) +- **Progress.** Every step ends with its duration. On an interactive terminal a step running a quiet + command also shows a spinner; in CI, a pipe, a file, with `NO_COLOR`, `->quiet()`, `->withOutput()`, + `->plain()` or `--plain` you get plain lines only. - `dryRun()` (`--dry-run`) runs no commands and no verification, so files that steps create (built assets, `vendor/`) are not in its list. For "what exactly ships", use `toDirectory()`. diff --git a/docs/phases/track-p-packager.md b/docs/phases/track-p-packager.md index 6e43c8b..34da48d 100644 --- a/docs/phases/track-p-packager.md +++ b/docs/phases/track-p-packager.md @@ -68,6 +68,8 @@ output as it runs; `"verbose": true` and `Build::verbose()` / `runVerbose()` do and from a build script. `--quiet` / `Build::quiet()` print only warnings. `--dir=PATH` / `Build::toDirectory()` (added 2026-10-12) write a folder instead of a zip, with the same steps and verification; the target is replaced only if it is empty or an earlier build of the same project. +`--plain`, `--wait=N` and the build lock, atomic output, interruption cleanup and the spinner (added +2026-10-12, ADR-0026) complete the robustness items below. **Configuration** is one optional `wptoolkit.json` at the project root. It's JSON so PHP reads it without dependencies, and it has a JSON Schema so editors autocomplete it: diff --git a/docs/reference/work-log.md b/docs/reference/work-log.md index 8cf05b7..8f927c3 100644 --- a/docs/reference/work-log.md +++ b/docs/reference/work-log.md @@ -61,3 +61,10 @@ One dated entry per working session: what landed, what was learned, what's next. - **Landed:** `Build::toDirectory()` (`--dir`) and `Build::quiet()` (`--quiet`); packaging moved out of `Build` into `ZipPackager` / `DirectoryPackager`. **Learned:** a folder target compared as a string let `.` and a different letter case through as "not the project" — paths are now resolved and compared without case. +- **Landed (rc.8):** build lock, atomic zip/directory output, Ctrl+C cleanup with a sweep of abandoned staging, a spinner + with step timings (ADR-0026); multiple-choice fields as checkboxes with a chip picker and an empty marker so a + selection can be cleared (ADR-0027). **Learned:** `proc_open` with a file descriptor lost the child's output + after certain earlier tests in the same PHP process (cause not pinned down); the shell now does the redirect. + The packager tests also run clean on Linux (WSL), including a real SIGINT. +- **Noticed, not fixed:** the `media` field renders `data-wptoolkit-media` for an admin script that does not exist + yet, so its "Choose media" button does nothing. diff --git a/llms.txt b/llms.txt index d6017b2..04fea40 100644 --- a/llms.txt +++ b/llms.txt @@ -21,7 +21,8 @@ Namespace: `Codad5\WPToolkit` (or your own, if you scope the library). Package: 10. Every user-facing string uses `__()`/`_e()`/`esc_html__()` with the plugin's own text domain, inside `boot()` or later. 11. Declare assets with `AssetManager` (`->script()`, `->style()`); never call `wp_enqueue_*` yourself. 12. Shortcodes: `$shortcodes->add('name', $handler)` with `->public()`/`->loggedIn()`/`->can()`, `->attributes([...])` (same shape as route args), `->view()`, `->assets()`. Handlers return; never echo, and never build `[tag …]` strings from request data — call `$shortcodes->render()`. -13. Change stored data with a `Migration` registered via `->migrations([...])`, never with branches on a "data version". +13. A multiple-choice field (`select` + `->multiple()`) renders as checkboxes, or a chip picker above 6 options; `->display('checkboxes'|'tokens'|'native')` overrides. Do not hand-write a `