mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 14:25:08 +00:00
fix(web): style editor no longer strands leaf-valued layout fields
A prior fix on this PR made elementKeys() append any layout-only key with
no style block of its own (a logo, a timeout indicator, a possession
arrow), so table() draws a row for it instead of losing it when the
wholesale `layout` claim removes the generic fallback. That covers a
layout-only key shaped like an object (x_offset/y_offset, ...), because
columnsFor() only ever produced columns from a key's *sub-fields*.
It missed the case where the layout-only key's own value is itself a
leaf -- a plain "show_logo" boolean directly under layout, no x/y object
underneath. elementKeys() still lists it (any row: no matching column),
so it renders as an uneditable blank row and its only control -- the
generic fallback checkbox -- is still gone. Confirmed by executing the
real widget's render() against a synthetic schema in Node (a DOM-stub
harness, not committed): the field's name never appeared as an <input>.
columnsFor() now gives such a leaf key a column keyed to itself
('layout-leaf'), and elementRow() binds it to the leaf's own path
(customization.layout.<key>, matching the name the fallback would have
used) instead of leaving every cell blank.
New regression test (test/js/unit/test_style_editor_layout_leaf_columns.js,
following this PR's existing eval-extraction pattern) checks the leaf
column is produced, is self-keyed, doesn't duplicate, and that a schema
with no leaf-valued layout key is unaffected; wired into run_all.js and
the suite table in test/js/README.md.
test/js/run_all.js: 84 + 6 + 6 = all suites passed (jsdom unavailable
here, DOM suites skip as before). Python suite untouched by this change;
test_style_editor_extra_fields.py, test_style_editor_save_roundtrip.py
and the one PIL-dependent style_editor_takeover.py case fail identically
before this commit -- missing flask/PIL in this sandbox, not this PR.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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_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_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_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_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_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` |
|
| `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` |
|
||||||
|
|||||||
+2
-1
@@ -15,7 +15,8 @@ const fs = require('fs');
|
|||||||
|
|
||||||
const BASE = process.env.BASE || 'http://localhost:5000';
|
const BASE = process.env.BASE || 'http://localhost:5000';
|
||||||
const UNIT = ['unit/test_list_filter.js', 'unit/test_render_cards.js',
|
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',
|
const DOM = ['dom/test_installed_dom.js', 'dom/test_store_dom.js', 'dom/test_no_double_fetch.js',
|
||||||
'dom/test_tools_sections.js'];
|
'dom/test_tools_sections.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');
|
||||||
@@ -382,9 +382,12 @@
|
|||||||
/**
|
/**
|
||||||
* The columns a table needs, derived from the schema rather than fixed.
|
* The columns a table needs, derived from the schema rather than fixed.
|
||||||
*
|
*
|
||||||
* Two sources: the sub-fields elements declare (font, font_size,
|
* Three sources: the sub-fields elements declare (font, font_size,
|
||||||
* text_color, visible, align) and the sub-fields their layout blocks
|
* text_color, visible, align), the sub-fields their layout blocks
|
||||||
* declare (x_offset, y_offset, scale).
|
* 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) {
|
function columnsFor(schema) {
|
||||||
var props = schema.properties || {};
|
var props = schema.properties || {};
|
||||||
@@ -396,8 +399,17 @@
|
|||||||
elementKeys(schema).forEach(function (key) {
|
elementKeys(schema).forEach(function (key) {
|
||||||
Object.keys(ownObj(props, key).properties || {}).forEach(
|
Object.keys(ownObj(props, key).properties || {}).forEach(
|
||||||
function (f) { seen.set(f, 'element'); });
|
function (f) { seen.set(f, 'element'); });
|
||||||
Object.keys(ownObj(layoutProps, key).properties || {}).forEach(
|
var layoutEntry = own(layoutProps, key);
|
||||||
function (f) { seen.set(f, 'layout'); });
|
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); });
|
var known = COLUMN_ORDER.filter(function (f) { return seen.get(f); });
|
||||||
// Anything the schema declares that this file has never heard of
|
// Anything the schema declares that this file has never heard of
|
||||||
@@ -483,8 +495,13 @@
|
|||||||
}
|
}
|
||||||
|
|
||||||
opts.columns.forEach(function (col) {
|
opts.columns.forEach(function (col) {
|
||||||
|
var isLeaf = col.where === 'layout-leaf';
|
||||||
var inLayout = col.where === 'layout';
|
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;
|
var cell;
|
||||||
if (!prop) {
|
if (!prop) {
|
||||||
// This element does not declare that field; keep the grid
|
// This element does not declare that field; keep the grid
|
||||||
@@ -492,13 +509,14 @@
|
|||||||
row.appendChild(el('span'));
|
row.appendChild(el('span'));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
var path = inLayout ? ['layout', key, col.key] : [key, col.key];
|
var path = isLeaf ? ['layout', key]
|
||||||
var base = inLayout ? opts.layoutPrefix + '.' + key
|
: inLayout ? ['layout', key, col.key] : [key, col.key];
|
||||||
|
var base = (isLeaf || inLayout) ? opts.layoutPrefix + '.' + key
|
||||||
: opts.prefix + '.' + key;
|
: opts.prefix + '.' + key;
|
||||||
var node = control({
|
var node = control({
|
||||||
key: col.key,
|
key: col.key,
|
||||||
prop: prop,
|
prop: prop,
|
||||||
name: base + '.' + col.key,
|
name: isLeaf ? base : base + '.' + col.key,
|
||||||
current: effective(value, path, prop, optional),
|
current: effective(value, path, prop, optional),
|
||||||
optional: optional,
|
optional: optional,
|
||||||
fonts: opts.fonts,
|
fonts: opts.fonts,
|
||||||
|
|||||||
Reference in New Issue
Block a user