diff --git a/test/js/README.md b/test/js/README.md index 01c1e7a4..c5f4013a 100644 --- a/test/js/README.md +++ b/test/js/README.md @@ -37,6 +37,7 @@ nothing is listening, so it stays useful in a bare checkout. | `unit/test_list_filter.js` | no | `ListFilter` search/filter/sort/count/sticky, and the installed-plugins config **extracted verbatim** from `plugins_manager.js` so the test can't drift from it | | `unit/test_render_cards.js` | no | `renderInstalledCards` markup, both empty states, and HTML-escaping of hostile plugin metadata | | `unit/test_style_editor_element_keys.js` | no | `elementKeys()` from `widgets/style-editor.js`: a hand-written `customization.layout` entry with no style block of its own still gets a row, since the widget claims the whole `layout` block from the generic fallback renderer | +| `unit/test_style_editor_layout_leaf_columns.js` | no | `columnsFor()` from `widgets/style-editor.js`: a layout-only key whose own value is a leaf (no x/y sub-object, e.g. a `show_logo` toggle) gets a self-keyed column instead of a blank, uneditable row | | `dom/test_installed_dom.js` | yes | The toolbar in a real DOM: pill/search/sort interaction, the HTMX partial re-swap, and a `getComputedStyle` check that `.filter-pill[data-active]` really matches the emitted markup | | `dom/test_store_dom.js` | yes | Store pagination, per-page, category, tri-state Installed button, and persistence across a re-boot, against the live registry | | `dom/test_no_double_fetch.js` | yes | Loads the **whole** `plugins_manager.js` and counts requests: typing in the store search must filter the cached list, not refetch `/api/v3/plugins/store/list` | diff --git a/test/js/run_all.js b/test/js/run_all.js index 562d2548..a5b0c31a 100755 --- a/test/js/run_all.js +++ b/test/js/run_all.js @@ -15,7 +15,8 @@ const fs = require('fs'); const BASE = process.env.BASE || 'http://localhost:5000'; const UNIT = ['unit/test_list_filter.js', 'unit/test_render_cards.js', - 'unit/test_html_escaping.js', 'unit/test_style_editor_element_keys.js']; + 'unit/test_html_escaping.js', 'unit/test_style_editor_element_keys.js', + 'unit/test_style_editor_layout_leaf_columns.js']; const DOM = ['dom/test_installed_dom.js', 'dom/test_store_dom.js', 'dom/test_no_double_fetch.js', 'dom/test_tools_sections.js']; diff --git a/test/js/unit/test_style_editor_layout_leaf_columns.js b/test/js/unit/test_style_editor_layout_leaf_columns.js new file mode 100644 index 00000000..4d495ace --- /dev/null +++ b/test/js/unit/test_style_editor_layout_leaf_columns.js @@ -0,0 +1,89 @@ +// Regression test for style-editor.js's columnsFor(), extracted verbatim +// from the shipped widget (own/ownObj/elementKeys/columnsFor have no DOM +// dependency) so the test can't drift from the real implementation. +// +// elementKeys() already appends a layout-only key that never got its own +// top-level style block (see test_style_editor_element_keys.js), so table() +// draws a row for it. But render() still claims the whole `layout` child as +// the widget's own regardless of what that row actually shows -- and +// columnsFor() only produced a column for a layout-only key whose own value +// is an *object* with sub-fields (x_offset, y_offset, ...). A hand-written +// schema can instead put a plain leaf value directly under layout -- a +// "show_logo" toggle, a timeout indicator's on/off flag -- with no x/y +// object underneath it. That key still got a row (data-element="show_logo"), +// but zero columns ever matched it, so the row rendered with every cell +// blank: no control, and no way back to the fallback since layout was +// removed wholesale. This pins the fix: such a key gets its own +// self-keyed 'layout-leaf' column. +const fs = require('fs'); +const path = require('path'); +const V3 = path.resolve(__dirname, '../../../web_interface/static/v3'); + +const src = fs.readFileSync(path.join(V3, 'js/widgets/style-editor.js'), 'utf8'); +const start = src.indexOf('function own(obj, key)'); +const endMark = 'function control(opts)'; +const end = src.indexOf(endMark); +if (start === -1 || end === -1) { + console.error('FAIL: could not locate own()/columnsFor() in style-editor.js -- slice markers need updating'); + process.exit(1); +} +eval(src.slice(start, end)); // defines own/ownObj/elementKeys/columnsFor + +let failures = 0; +function ok(desc, cond) { + if (cond) { console.log(`ok - ${desc}`); } + else { console.log(`FAIL - ${desc}`); failures++; } +} + +// score_text is a real style element; possession is layout-only but shaped +// like an object (x/y); show_logo is layout-only and a bare leaf value. +const schema = { + properties: { + score_text: { + type: 'object', + 'x-style-managed': true, + properties: { font: { type: 'string' }, text_color: { type: 'array' } }, + }, + layout: { + type: 'object', + properties: { + score_text: { type: 'object', properties: { x_offset: {}, y_offset: {} } }, + possession: { type: 'object', properties: { x_offset: {}, y_offset: {} } }, + show_logo: { type: 'boolean', default: true }, + }, + }, + }, +}; + +const columns = columnsFor(schema); +const byKey = {}; +columns.forEach(function (c) { byKey[c.key] = c; }); + +ok('an element style field still gets its own column', byKey.font && byKey.font.where === 'element'); +ok('a layout-only object key still contributes its x/y columns', + byKey.x_offset && byKey.x_offset.where === 'layout'); +ok('a layout-only leaf key gets a column keyed to itself', + !!byKey.show_logo); +ok('...marked as a layout-leaf column, not a shared sub-field', + byKey.show_logo && byKey.show_logo.where === 'layout-leaf'); +ok('nothing is duplicated', columns.length === new Set(columns.map(function (c) { return c.key; })).size); + +// A schema with no leaf-valued layout key must not gain a phantom column -- +// this is the common case (every existing scoreboard) and must be unaffected. +const noLeaf = { + properties: { + score_text: { + type: 'object', 'x-style-managed': true, + properties: { font: { type: 'string' } }, + }, + layout: { + type: 'object', + properties: { score_text: { type: 'object', properties: { x_offset: {} } } }, + }, + }, +}; +ok('a schema with only object-shaped layout keys gets no layout-leaf column', + columnsFor(noLeaf).every(function (c) { return c.where !== 'layout-leaf'; })); + +if (failures) { console.log(`\n${failures} failure(s)`); process.exit(1); } +console.log('\nall checks passed'); diff --git a/web_interface/static/v3/js/widgets/style-editor.js b/web_interface/static/v3/js/widgets/style-editor.js index 24d77feb..ca28a7de 100644 --- a/web_interface/static/v3/js/widgets/style-editor.js +++ b/web_interface/static/v3/js/widgets/style-editor.js @@ -382,9 +382,12 @@ /** * The columns a table needs, derived from the schema rather than fixed. * - * Two sources: the sub-fields elements declare (font, font_size, - * text_color, visible, align) and the sub-fields their layout blocks - * declare (x_offset, y_offset, scale). + * Three sources: the sub-fields elements declare (font, font_size, + * text_color, visible, align), the sub-fields their layout blocks + * declare (x_offset, y_offset, scale), and a layout-only key whose own + * value *is* the field to set -- a plain "show_logo" toggle has no x/y + * object underneath it, so it gets a column keyed to itself rather than + * to a shared sub-field name. */ function columnsFor(schema) { var props = schema.properties || {}; @@ -396,8 +399,17 @@ elementKeys(schema).forEach(function (key) { Object.keys(ownObj(props, key).properties || {}).forEach( function (f) { seen.set(f, 'element'); }); - Object.keys(ownObj(layoutProps, key).properties || {}).forEach( - function (f) { seen.set(f, 'layout'); }); + var layoutEntry = own(layoutProps, key); + if (layoutEntry && typeof layoutEntry === 'object' && layoutEntry.properties) { + Object.keys(layoutEntry.properties).forEach( + function (f) { seen.set(f, 'layout'); }); + } else if (layoutEntry && !seen.has(key)) { + // layout-only and a leaf: nothing else will share this + // column, but leaving it out drops the field's only control + // the moment the wholesale `layout` claim removes its + // fallback (#569 review). + seen.set(key, 'layout-leaf'); + } }); var known = COLUMN_ORDER.filter(function (f) { return seen.get(f); }); // Anything the schema declares that this file has never heard of @@ -483,8 +495,13 @@ } opts.columns.forEach(function (col) { + var isLeaf = col.where === 'layout-leaf'; var inLayout = col.where === 'layout'; - var prop = inLayout ? own(axes, col.key) : own(props, col.key); + // A leaf column only applies to the one row named after it -- + // every other row leaves it blank, same as an element that + // doesn't declare a shared sub-field. + var prop = isLeaf ? (col.key === key ? own(layoutProps, key) : null) + : inLayout ? own(axes, col.key) : own(props, col.key); var cell; if (!prop) { // This element does not declare that field; keep the grid @@ -492,13 +509,14 @@ row.appendChild(el('span')); return; } - var path = inLayout ? ['layout', key, col.key] : [key, col.key]; - var base = inLayout ? opts.layoutPrefix + '.' + key + var path = isLeaf ? ['layout', key] + : inLayout ? ['layout', key, col.key] : [key, col.key]; + var base = (isLeaf || inLayout) ? opts.layoutPrefix + '.' + key : opts.prefix + '.' + key; var node = control({ key: col.key, prop: prop, - name: base + '.' + col.key, + name: isLeaf ? base : base + '.' + col.key, current: effective(value, path, prop, optional), optional: optional, fonts: opts.fonts,