From f9b1f87e8df11c06aaa2c6d383379aecd43268d0 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 14 Sep 2026 12:48:36 -0400 Subject: [PATCH] fix: clamp colour components, and let the style editor actually take over (#569) * fix(element-style): clamp out-of-range colour components instead of rejecting A regression this framework shipped. The eight scoreboards used to read their colours through sports_card.coerce_rgb, which clamps; routing them through the shared element_color sent them through _normalize_color, which rejected any component outside 0..255 and fell back to the default. So a configured [999, -5, 20] -- a typo'd bright red -- rendered white instead of (255, 0, 20). Their own test_element_text_colors.py caught it: one case of nineteen, in all eight plugins, failing only once the core change reached main. Co-Authored-By: Claude Opus 5 * fix(web): the style editor takes over its own blocks -- and gets to at all Two defects, both found by rendering the real partial in a browser rather than by reading the code. It was losing a race to its own fields. The hand-off guard asked "do any fallback controls differ from their server-rendered defaults?" as a proxy for "is someone editing this?". But the fallback holds this block's own font fields, and the font-selector widget populates them on the same 50ms timer -- so a plain page load, with nobody touching anything, raced into "dirty" and the editor removed itself, leaving the 701-line accordion form it exists to replace. Measured: seven customization.*.font selects dirty ~60ms after injection, clean again by 400ms. The question is whether a *person* typed, and event.isTrusted answers exactly that; the listeners now go on synchronously, because the edit worth protecting can happen before initWidget runs. It took over too much. Taking over removed the whole fallback section, but a customization block can hold more than styling -- football keeps favorite_result_colors there -- so that removed the only UI those fields had, and the editor also rendered them as an element, giving every row an "enabled" and three colour columns. Core now marks the blocks it recognises as styling (the compact declaration already did; hand-written adoption did not), the widget renders only those, and the template drops only the children the widget reports owning. Verified on football's real schema: 28 rows across four mode tabs, columns Element/Font/Size/Colour/X/Y, favorite_result_colors still editable with its ten inputs, no duplicated field names, no console errors. Co-Authored-By: Claude Opus 5 * fix(web): style editor no longer drops layout-only fields it never rendered CodeRabbit flagged elementKeys() in style-editor.js: render() claims the whole customization.layout child as the widget's own (removing it from the generic fallback renderer, since posting the same offset twice is worse), but elementKeys() only listed keys that also have their own top-level style block. A hand-written schema can put a key under layout that never got one -- a logo, a timeout indicator, a possession arrow with a position but no font or colour -- and that key's only control silently disappeared: no row in the style editor's table (elementKeys never listed it) and no fallback section either (layout was removed wholesale). elementKeys() now appends any layout-declared key not already covered by a style element, so table() renders a row for it (layout columns only, no style columns) and the wholesale layout ownership claim stays truthful. Verified against current code before fixing. New regression test (test/js/unit/test_style_editor_element_keys.js, following this repo's existing eval-extraction pattern for testing widget JS without a browser) fails against the reverted function and passes with the fix; added to run_all.js and the suite table in test/js/README.md. Full pytest suite: 4887 passed, 62 skipped, 2 failed -- both the pre-existing Europe/Kiev/Asia/Calcutta tzdata-alias gap on this sandbox, identical on origin/main, unrelated to this change. Co-Authored-By: Claude Opus 5 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Dpg3HLWohdCUdzz2QNHanm * 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 . 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., 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 * fix(web): keep layout-leaf style-editor columns distinct from name collisions columnsFor() keyed a layout-only leaf field's column by its bare field name. If an unrelated element's style block or another element's layout axis block happened to declare a sub-field with that same name, the `!seen.has(key)` guard skipped creating the leaf's column, silently dropping its only control again -- the same failure the leaf-column fix was meant to close, just reached through a name collision (CodeRabbit review on 324a7ea). Key layout-leaf columns under a namespaced id so they can never be shadowed by an unrelated column sharing their name. Co-Authored-By: Claude Opus 5 * fix(web): CSS.escape() the owned key before it becomes a selector container.dataset.ownedKeys round-trips schema property keys through a DOM dataset attribute, and the takeover handoff spliced each one straight into '[data-child-key="' + k + '"]' with no escaping -- inconsistent with this codebase's own convention elsewhere (plugin-file-manager.js, app-shell.js's escapeCssSelector) for building a selector from a dynamic value. A key containing a quote or backslash would break the selector or be steerable; Codacy's static analysis flagged this pattern (1 high ErrorProne finding on PR #569, current head at the time) as a new issue, though its dashboard is unreachable from this sandbox (egress to app.codacy.com is blocked) and the check-run API returned no detail text -- verified and fixed by reading the diff directly rather than the tool's own description. Added a source-assertion regression test alongside this file's existing ones (this behavior lives in an inline script no Python test executes). Full suite: 4888 passed, 62 skipped, 2 failed -- both the pre-existing Europe/Kiev/Asia/Calcutta tzdata-alias gap in this sandbox, identical on origin/main, unrelated to this change. Co-Authored-By: Claude Opus 5 * feat(web): every advertised layout offset gets a control in the style editor The editor took the whole layout section over but matched offsets to style rows by exact key. A hand-written schema's two blocks were never named alike -- football styles score_text but positions score -- so of football's eleven positionable things only status_text had a control. Score, odds, both logos, timeouts, possession, down-and-distance, date, time and records were options the schema advertised and the renderer reads, reachable nowhere in the UI. Core now resolves each style element's layout key through alias_keys, the map the resolver already reads offsets with, and records it as x-layout-key. The widget reads that rather than carrying a second copy of the rules, and posts under the key the schema declares: football's own offset reader looks up layout.score, so a value saved as layout.score_text would be kept and never drawn. Layout entries no style element claims get an "Other positions" table with its own columns, in every mode panel as well as the base one, in the order the plugin declared them. Verified in a browser against football's real schema: 92 of 92 layout fields (23 base, 23 per mode) rendered exactly once under their declared names, none posted under a style key, no duplicated field names, favorite_result_colors still editable. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- src/element_style.py | 49 +++- test/js/README.md | 3 + test/js/run_all.js | 4 +- .../js/unit/test_style_editor_element_keys.js | 126 +++++++++ ...test_style_editor_layout_leaf_collision.js | 76 ++++++ .../test_style_editor_layout_leaf_columns.js | 89 +++++++ test/test_sports_card.py | 8 + .../test_style_editor_takeover.py | 251 ++++++++++++++++++ .../static/v3/js/widgets/style-editor.js | 197 +++++++++++--- .../templates/v3/partials/plugin_config.html | 79 ++++-- 10 files changed, 826 insertions(+), 56 deletions(-) create mode 100644 test/js/unit/test_style_editor_element_keys.js create mode 100644 test/js/unit/test_style_editor_layout_leaf_collision.js create mode 100644 test/js/unit/test_style_editor_layout_leaf_columns.js create mode 100644 test/web_interface/test_style_editor_takeover.py diff --git a/src/element_style.py b/src/element_style.py index 5319ee07..0285406f 100644 --- a/src/element_style.py +++ b/src/element_style.py @@ -761,14 +761,54 @@ def _adopt_handwritten_block(schema: Dict[str, Any], customization.setdefault('x-widget', 'style-editor') props = customization['properties'] + layout_block = props.get('layout') + layout_fields = (layout_block.get('properties') + if isinstance(layout_block, dict) else None) + layout_fields = layout_fields if isinstance(layout_fields, dict) else {} + for key in element_keys: _upgrade_font_property(props[key]) + # Marked like a declared element, so consumers can tell the style + # blocks from whatever else the plugin keeps under customization. + # Football's block also holds favorite_result_colors, which is a + # feature with its own fields -- without this the editor treats it + # as an element and every row grows an "enabled"/"win color" column. + props[key]['x-style-managed'] = True + # Where this element's offsets live, resolved through the same alias + # map the renderer reads them with. The two blocks were never named + # alike -- football styles score_text but positions score -- and the + # editor matched them by exact name, so it drew one offset in eleven + # and the rest had no control anywhere. Recording the answer here + # keeps the alias rules in one place instead of a JavaScript copy. + # Only an object-shaped entry (x_offset/y_offset/...) can hold an + # element's offsets. A leaf directly under layout -- a show_logo + # toggle -- is its own control, so it is never claimed by a row and + # always gets a position row of its own. + layout_key = next((name for name in alias_keys(key) + if isinstance(layout_fields.get(name), dict) + and isinstance(layout_fields[name].get('properties'), + dict)), None) + if layout_key is not None: + props[key]['x-layout-key'] = layout_key + + if layout_fields: + # Positions are listed in the order the plugin declared them. Flask's + # JSON provider sorts keys, which would put the logos after the date. + layout_block.setdefault('x-propertyOrder', list(layout_fields)) modes = customization.get('x-style-modes') if isinstance(modes, list) and modes: props.setdefault('modes', _modes_block_from_properties(props, element_keys, modes)) + mode_blocks = props['modes'].get('properties') if isinstance( + props.get('modes'), dict) else None + for mode_block in (mode_blocks or {}).values(): + mode_layout = ((mode_block or {}).get('properties') or {}).get('layout') + if (isinstance(mode_layout, dict) + and isinstance(mode_layout.get('properties'), dict)): + mode_layout.setdefault('x-propertyOrder', + list(mode_layout['properties'])) # Stated explicitly because the config form serialises the schema with # Flask's JSON provider, which sorts keys -- without this the elements @@ -904,11 +944,14 @@ def _normalize_color(value: Any) -> Optional[Tuple[int, int, int]]: return None if isinstance(value, (list, tuple)) and len(value) == 3: try: - rgb = tuple(int(c) for c in value) + # Clamped, not rejected. The readers this replaced clamped + # (sports_card.coerce_rgb), and the eight scoreboards' own tests + # pin it: a configured [999, -5, 20] is a typo'd bright red, and + # answering "unusable, take the default" turned it white instead. + rgb = tuple(max(0, min(255, int(c))) for c in value) except (TypeError, ValueError): return None - if all(0 <= c <= 255 for c in rgb): - return rgb # type: ignore[return-value] + return rgb # type: ignore[return-value] return None diff --git a/test/js/README.md b/test/js/README.md index 023e0d9a..c635ce80 100644 --- a/test/js/README.md +++ b/test/js/README.md @@ -36,6 +36,9 @@ 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()`/`styleRows()`/`positionRows()` from `widgets/style-editor.js`: every `customization.layout` entry gets exactly one row -- paired with its style element through core's `x-layout-key` (so `score` belongs to `score_text`, not a second row), or a position row of its own, leaves included -- 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 | +| `unit/test_style_editor_layout_leaf_collision.js` | no | `columnsFor()` from `widgets/style-editor.js`: a layout-only leaf key still gets its own column even when its name collides with an unrelated element's style sub-field or another layout axis's sub-field | | `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 b10a9827..99ddaa01 100755 --- a/test/js/run_all.js +++ b/test/js/run_all.js @@ -15,7 +15,9 @@ 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_html_escaping.js', 'unit/test_style_editor_element_keys.js', + 'unit/test_style_editor_layout_leaf_columns.js', + 'unit/test_style_editor_layout_leaf_collision.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_element_keys.js b/test/js/unit/test_style_editor_element_keys.js new file mode 100644 index 00000000..b3d22462 --- /dev/null +++ b/test/js/unit/test_style_editor_element_keys.js @@ -0,0 +1,126 @@ +// Regression test for how style-editor.js decides which rows to draw, +// extracted verbatim from the shipped widget (own/ownObj/elementKeys/ +// styleRows/positionRows have no DOM dependency) so the test can't drift from +// the real implementation. +// +// render() claims the whole `layout` child of a customization schema as the +// widget's own -- removing it from the generic fallback renderer entirely, +// because posting the same offset twice from two controls is worse than +// owning slightly too much. That is only safe when every entry under `layout` +// gets exactly one row: +// +// * styleRows(): one per style element, paired with the layout entry that +// holds its offsets. The pairing is not by exact name -- a hand-written +// schema styles 'score_text' but positions 'score' -- so core records it +// as x-layout-key, resolved through the same alias map the renderer reads +// offsets with. +// * positionRows(): one per layout entry no style element claimed -- a logo, +// a timeout indicator, a possession arrow, or a bare leaf like a +// show_logo toggle. +// +// This contract replaced an earlier one in which elementKeys() itself +// appended layout-only keys. That compared names exactly, so an aliased entry +// ('score' for 'score_text') was listed a second time and its offsets got two +// controls posting the same field. +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()..positionRows() in style-editor.js -- slice markers need updating'); + process.exit(1); +} +eval(src.slice(start, end)); // defines own/ownObj/elementKeys/styleRows/positionRows + +let failures = 0; +function ok(desc, cond) { + if (cond) { console.log(`ok - ${desc}`); } + else { console.log(`FAIL - ${desc}`); failures++; } +} + +function layoutKeysOf(rows) { return rows.map(function (r) { return r.layoutKey; }); } + +// score_text is styled and positioned under an alias; odds_text has no +// offsets at all; possession is positioned but never styled; show_logo is a +// bare leaf under layout. +const schema = { + properties: { + score_text: { + type: 'object', 'x-style-managed': true, 'x-layout-key': 'score', + properties: { font: { type: 'string' }, text_color: { type: 'array' } }, + }, + odds_text: { + type: 'object', 'x-style-managed': true, + properties: { font: { type: 'string' } }, + }, + layout: { + type: 'object', + 'x-propertyOrder': ['score', 'possession', 'show_logo'], + properties: { + score: { type: 'object', properties: { x_offset: {}, y_offset: {} } }, + possession: { type: 'object', properties: { x_offset: {}, y_offset: {} } }, + show_logo: { type: 'boolean', default: true }, + }, + }, + }, +}; + +const keys = elementKeys(schema); +ok('elementKeys lists the style elements', keys.join(',') === 'score_text,odds_text'); +ok('layout and modes containers are never treated as elements', + keys.indexOf('layout') === -1 && keys.indexOf('modes') === -1); + +const styled = styleRows(schema); +ok('a style element is paired with the layout entry core resolved for it', + styled[0].key === 'score_text' && styled[0].layoutKey === 'score'); +ok('a style element with no offsets claims nothing', styled[1].layoutKey === null); + +const positions = positionRows(schema); +ok('a layout-only object entry gets a position row', + layoutKeysOf(positions).indexOf('possession') !== -1); +ok('a layout-only leaf gets a position row too', + layoutKeysOf(positions).indexOf('show_logo') !== -1); +ok('an entry claimed through an alias is not listed again as a position', + layoutKeysOf(positions).indexOf('score') === -1); +ok('positions follow the declared order', + layoutKeysOf(positions).join(',') === 'possession,show_logo'); + +const every = layoutKeysOf(styled).filter(Boolean).concat(layoutKeysOf(positions)); +ok('every layout entry gets exactly one row', + every.length === new Set(every).size + && every.slice().sort().join(',') === ['possession', 'score', 'show_logo'].join(',')); + +// A schema with no layout block at all must behave exactly as before -- +// the overwhelmingly common case must not gain a phantom row. +const noLayout = { + properties: { + score_text: { + type: 'object', 'x-style-managed': true, + properties: { font: { type: 'string' } }, + }, + }, +}; +ok('a schema without a layout block has no positions', + elementKeys(noLayout).join(',') === 'score_text' && positionRows(noLayout).length === 0); + +// Without an annotation (the compact declaration, or a schema that never went +// through expansion) the two blocks share one key. +const unannotated = { + properties: { + home_logo: { type: 'object', 'x-style-managed': true, properties: {} }, + layout: { + type: 'object', + properties: { home_logo: { type: 'object', properties: { x_offset: {}, scale: {} } } }, + }, + }, +}; +ok('an exact-name layout entry is claimed without an annotation', + styleRows(unannotated)[0].layoutKey === 'home_logo' + && positionRows(unannotated).length === 0); + +if (failures) { console.log(`\n${failures} failure(s)`); process.exit(1); } +console.log('\nall checks passed'); diff --git a/test/js/unit/test_style_editor_layout_leaf_collision.js b/test/js/unit/test_style_editor_layout_leaf_collision.js new file mode 100644 index 00000000..c128f242 --- /dev/null +++ b/test/js/unit/test_style_editor_layout_leaf_collision.js @@ -0,0 +1,76 @@ +// Regression test for style-editor.js's columnsFor(), extracted verbatim +// from the shipped widget (see test_style_editor_layout_leaf_columns.js for +// the base case this builds on). +// +// A layout-only leaf key (e.g. a bare "scale" toggle directly under +// `layout`, no x/y object underneath) used to be keyed in the internal +// `seen` map by its own bare field name. If some *other* element's style +// block, or another element's layout axis block, happened to declare a +// sub-field with that exact same name (e.g. "scale" is also a real +// COLUMN_ORDER axis name most elements use), the `!seen.has(key)` guard +// skipped creating the leaf's column -- it silently reused the existing +// 'element'/'layout' column instead. That column's row binding in +// elementRow() then looked the field up under the wrong schema location for +// the leaf's own row, found nothing, and rendered a blank cell: the same +// "silently disappears" bug the leaf-column fix was meant to close, just +// reached through a name collision instead of a missing column altogether. +// This pins the fix: layout-leaf columns are keyed under a namespaced id so +// they can never be shadowed by an unrelated column sharing their name. +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++; } +} + +// `possession` is a real style element whose layout axis block declares a +// "scale" sub-field, which seeds seen.set('scale', 'layout'). `timeout_flag` +// is layout-only and a bare leaf valued directly under layout, but its own +// key is *also* "scale" -- an unrelated collision with that same name. +const schema = { + properties: { + possession: { + type: 'object', + 'x-style-managed': true, + properties: { font: { type: 'string' } }, + }, + scale: { + // The collision: a second element literally named "scale", entirely + // unrelated to the layout-only leaf below sharing that string. + type: 'boolean', + }, + layout: { + type: 'object', + properties: { + possession: { type: 'object', properties: { scale: { type: 'number' } } }, + scale: { type: 'boolean', default: true }, + }, + }, + }, +}; + +const columns = columnsFor(schema); +const leafColumns = columns.filter(function (c) { return c.where === 'layout-leaf'; }); + +ok('the colliding layout axis sub-field still gets its own column', + columns.some(function (c) { return c.key === 'scale' && c.where === 'layout'; })); +ok('the layout-only leaf key still gets its own column despite the name collision', + leafColumns.length === 1 && leafColumns[0].key === 'scale'); +ok('the leaf column is distinct from the colliding element/layout column', + columns.filter(function (c) { return c.key === 'scale'; }).length === 2); + +if (failures) { console.log(`\n${failures} failure(s)`); process.exit(1); } +console.log('\nall checks passed'); 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/test/test_sports_card.py b/test/test_sports_card.py index 0bf88f37..f8de86f5 100644 --- a/test/test_sports_card.py +++ b/test/test_sports_card.py @@ -53,6 +53,14 @@ class TestColour: cfg = {"customization": {"score_text": {"text_color": value}}} assert C.element_color(cfg, "score_text", (9, 9, 9)) == (9, 9, 9) + def test_out_of_range_components_are_clamped_not_rejected(self): + """The readers this replaced clamped, and the eight scoreboards' own + tests pin it. Rejecting instead turned a typo'd [999, -5, 20] -- a + bright red -- into the default white, which is a colour the user never + asked for rather than the one they nearly asked for.""" + cfg = {"customization": {"score_text": {"text_color": [999, -5, 20]}}} + assert C.element_color(cfg, "score_text", (9, 9, 9)) == (255, 0, 20) + def test_coerce_rgb_clamps_rather_than_rejecting(self): assert C.coerce_rgb([300, -5, 20], (0, 0, 0)) == (255, 0, 20) diff --git a/test/web_interface/test_style_editor_takeover.py b/test/web_interface/test_style_editor_takeover.py new file mode 100644 index 00000000..5d6618d7 --- /dev/null +++ b/test/web_interface/test_style_editor_takeover.py @@ -0,0 +1,251 @@ +"""When the style editor takes over a customization block, it takes over +exactly its own part of it -- and it actually gets to take over at all. + +Two defects this pins, both found by rendering the real partial in a browser +rather than by reading the code: + +1. The hand-off guard asked "do any fallback controls differ from their + server-rendered defaults?" as a proxy for "is someone editing this?". The + fallback contains this block's own font fields, and the font-selector + widget populates them on the same 50ms timer -- so a plain page load with + nobody touching anything raced into "dirty" (seven customization.*.font + selects, measured ~60ms after injection) and the style editor removed + itself, leaving the accordion form it exists to replace. The question is + whether a *person* typed, and isTrusted answers exactly that. + +2. Taking over used to remove the whole fallback section. A customization + block can hold more than styling -- football keeps favorite_result_colors + there -- so that removed the only UI those fields had. + +These assert on the template source because the behaviour lives in an inline +script that no Python test executes; the browser check that found both is not +something CI runs. +""" + +import re +import sys +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).parent.parent.parent +sys.path.insert(0, str(PROJECT_ROOT)) + +TEMPLATE = (PROJECT_ROOT / "web_interface" / "templates" / "v3" / "partials" + / "plugin_config.html") + + +@pytest.fixture(scope="module") +def source(): + return TEMPLATE.read_text(encoding="utf-8") + + +class TestTheHandOffGuard: + def test_it_asks_whether_a_person_typed(self, source): + assert "isTrusted" in source, ( + "a programmatic change by a sibling widget must not read as a " + "user edit -- that is what removed the editor on a plain load") + assert "userEdited" in source + + def test_it_no_longer_diffs_values_against_defaults(self, source): + assert "fallbackIsDirty" not in source, ( + "value-diffing cannot distinguish a user's edit from another " + "widget populating this block's own fields") + assert "defaultSelected" not in source + + def test_the_listeners_are_attached_synchronously(self, source): + """The edit worth protecting can happen before initWidget runs, so + the listeners cannot wait for it. + + Scoped to this branch: the template defines an initWidget for every + widget it can render, and the first one in the file belongs to a + different branch entirely. + """ + start = source.index("obj_widget == 'style-editor'") + branch = source[start:source.index("{% elif prop.properties %}", start)] + assert "watchForRealEdits" in branch + assert branch.index("watchForRealEdits") < branch.index("function initWidget"), ( + "the watcher must be installed above this branch's initWidget, " + "and invoked immediately rather than from inside it") + + def test_the_guard_still_backs_off_for_a_real_edit(self, source): + assert re.search(r"if \(userEdited\) \{ container\.remove\(\); return; \}", + source), "a genuine in-progress edit must still win" + + +class TestItTakesOverOnlyItsOwnBlocks: + def test_children_are_individually_addressable(self, source): + assert 'data-child-key="{{ nested_key }}"' in source, ( + "without a handle per child the only options are removing the " + "whole section or none of it") + + def test_removal_is_driven_by_what_the_widget_reported(self, source): + assert "ownedKeys" in source + assert "fallback.querySelector(" in source + + def test_an_unowned_child_survives(self, source): + """The empty-section case still removes the lot, so a block that is + entirely styling looks exactly as it did before.""" + assert "!fallback.querySelector('[data-child-key]')" in source + + def test_the_owned_key_is_escaped_before_it_becomes_a_selector(self, source): + """ownedKeys comes back off a dataset attribute, not a literal -- + splicing it into '[data-child-key="' + k + '"]' unescaped breaks (or + is steerable) on a key containing a quote or backslash. The rest of + this codebase (plugin-file-manager.js, app-shell.js) always routes a + dynamic value through CSS.escape() before it lands in a selector.""" + assert "CSS.escape(k)" in source, ( + "an owned key must be CSS.escape()'d before being interpolated " + "into the data-child-key attribute selector") + + +class TestTheSchemaSaysWhichBlocksAreStyling: + def test_adopted_blocks_are_marked_and_others_are_not(self): + from src.element_style import expand_style_elements + + schema = { + "type": "object", + "properties": { + "customization": { + "type": "object", + "properties": { + "score_text": { + "type": "object", + "properties": { + "font": {"type": "string", + "default": "PressStart2P-Regular.ttf"}, + "font_size": {"type": "integer", "default": 10}, + "text_color": {"type": "array", + "default": [255, 255, 255]}, + }, + }, + # Not styling: a feature that happens to live here. + "favorite_result_colors": { + "type": "object", + "properties": { + "enabled": {"type": "boolean", "default": False}, + "win_color": {"type": "array", + "default": [0, 255, 0]}, + }, + }, + }, + }, + }, + } + props = expand_style_elements(schema)["properties"]["customization"]["properties"] + assert props["score_text"]["x-style-managed"] is True + assert "x-style-managed" not in props["favorite_result_colors"] + + +# Football's real layout keys, in its schema's order. The style block and the +# layout block were written years apart and never agreed on names. +FOOTBALL_LAYOUT = ["home_logo", "away_logo", "score", "status_text", "date", + "time", "down_distance", "timeouts", "possession", + "records", "odds"] + + +def _offsets(*axes): + return {"type": "object", + "properties": {a: {"type": "integer", "default": 0} for a in axes}} + + +def _style_block(): + return {"type": "object", "properties": { + "font": {"type": "string", "default": "PressStart2P-Regular.ttf"}, + "font_size": {"type": "integer", "default": 10}, + "text_color": {"type": "array", "default": [255, 255, 255]}, + }} + + +def _football_shaped(): + layout = {k: _offsets("x_offset", "y_offset") for k in FOOTBALL_LAYOUT} + layout["records"] = _offsets("away_x_offset", "home_x_offset", "y_offset") + return {"type": "object", "properties": {"customization": { + "type": "object", + "x-style-modes": ["live", "recent"], + "properties": { + **{k: _style_block() for k in ( + "score_text", "period_text", "team_name", "status_text", + "detail_text", "odds_text", "rank_text")}, + "layout": {"type": "object", "properties": layout}, + }}}} + + +class TestEveryAdvertisedOffsetGetsAControl: + """The style editor took the layout section over but only drew offsets + whose layout key matched a style key exactly. In football that was + status_text alone: score, odds, both logos, timeouts, possession, + down-and-distance, date, time and records -- options the schema + advertises and the renderer reads -- had no control anywhere.""" + + def _customization(self): + from src.element_style import expand_style_elements + return expand_style_elements(_football_shaped())[ + "properties"]["customization"] + + def test_a_style_element_is_told_where_its_offsets_live(self): + """Resolved in core through the same alias map the resolver reads + offsets with, so the editor cannot drift from the renderer.""" + props = self._customization()["properties"] + assert props["score_text"]["x-layout-key"] == "score" + assert props["odds_text"]["x-layout-key"] == "odds" + assert props["status_text"]["x-layout-key"] == "status_text" + + def test_an_element_with_no_offsets_claims_nothing(self): + props = self._customization()["properties"] + for key in ("period_text", "detail_text", "team_name", "rank_text"): + assert "x-layout-key" not in props[key], key + + def test_the_mode_copies_carry_it_too(self): + live = self._customization()["properties"]["modes"]["properties"][ + "live"]["properties"] + assert live["score_text"]["x-layout-key"] == "score" + + def test_positions_keep_the_order_the_plugin_declared(self): + """Flask sorts keys when it serialises the schema, which would + otherwise list the logos after the date.""" + c = self._customization() + assert c["properties"]["layout"]["x-propertyOrder"] == FOOTBALL_LAYOUT + live_layout = c["properties"]["modes"]["properties"]["live"][ + "properties"]["layout"] + assert live_layout["x-propertyOrder"] == FOOTBALL_LAYOUT + + def test_every_layout_entry_is_claimed_or_listed_as_a_position(self): + """Mirrors the widget's split: a layout entry either belongs to a + style row or gets a row of its own. Nothing is left over.""" + props = self._customization()["properties"] + layout = props["layout"]["properties"] + claimed = {props[k]["x-layout-key"] for k in props + if isinstance(props[k], dict) and "x-layout-key" in props[k]} + positions = [k for k in props["layout"]["x-propertyOrder"] + if k not in claimed] + assert claimed | set(positions) == set(layout) + assert positions == ["home_logo", "away_logo", "date", "time", + "down_distance", "timeouts", "possession", + "records"] + + def test_the_widget_reads_the_annotation_and_lists_positions(self): + js = (PROJECT_ROOT / "web_interface" / "static" / "v3" / "js" + / "widgets" / "style-editor.js").read_text(encoding="utf-8") + assert "x-layout-key" in js, ( + "the alias rules live in element_style.py; the widget must read " + "their result rather than carry a second copy") + assert "function positionRows" in js + + def test_a_leaf_under_layout_is_never_claimed_as_offsets(self): + """A show_logo toggle straight under layout has no x/y object, so it + cannot hold an element's offsets. Claiming it would give that row no + layout columns and remove the leaf from the positions list -- the + field would lose its only control.""" + from src.element_style import expand_style_elements + + schema = _football_shaped() + cust = schema["properties"]["customization"] + # A style element whose alias ("show_logo") names a leaf. + cust["properties"]["show_logo_text"] = _style_block() + cust["properties"]["layout"]["properties"]["show_logo"] = { + "type": "boolean", "default": True} + props = expand_style_elements(schema)["properties"]["customization"][ + "properties"] + assert "x-layout-key" not in props["show_logo_text"] + assert "show_logo" in props["layout"]["x-propertyOrder"] diff --git a/web_interface/static/v3/js/widgets/style-editor.js b/web_interface/static/v3/js/widgets/style-editor.js index 1017ae39..c495cd6d 100644 --- a/web_interface/static/v3/js/widgets/style-editor.js +++ b/web_interface/static/v3/js/widgets/style-editor.js @@ -146,10 +146,20 @@ function elementKeys(schema) { var props = (schema && schema.properties) || {}; var order = schema['x-propertyOrder'] || Object.keys(props); - return order.filter(function (k) { + var shaped = order.filter(function (k) { return k !== 'layout' && k !== 'modes' && own(props, k) && ownObj(props, k).properties; }); + // Core marks the blocks it recognises as styling. Prefer that: a + // customization block can also hold a feature of its own -- football + // keeps favorite_result_colors there -- and treating one as an element + // gives every row that feature's fields as extra columns. + var managed = shaped.filter(function (k) { + return ownObj(props, k)['x-style-managed'] === true; + }); + // Nothing marked means the schema never went through expansion, so + // fall back to the shape test rather than rendering an empty table. + return managed.length ? managed : shaped; } function titleOf(schema, key) { @@ -357,25 +367,100 @@ visible: '3.5rem' }; + /** + * Where an element's offsets live in the layout block, or null. + * + * A hand-written schema does not line up: football styles 'score_text' + * but positions it under 'score'. Core resolves that through the same + * alias map the renderer reads offsets with and records the answer as + * x-layout-key, so the rules exist once, in element_style.py. Without + * the annotation both blocks share one key, as the compact declaration + * does. + */ + function layoutKeyFor(schema, key) { + var props = (schema && schema.properties) || {}; + var layoutProps = ownObj(props, 'layout').properties || {}; + var declared = ownObj(props, key)['x-layout-key']; + // Only an object-shaped entry holds offsets. A leaf straight under + // layout (a show_logo toggle) is a control of its own, so it is never + // claimed here and always gets a position row. + if (typeof declared === 'string' && ownObj(layoutProps, declared).properties) { + return declared; + } + return ownObj(layoutProps, key).properties ? key : null; + } + + /** One row per styled element, paired with its offsets if it has any. */ + function styleRows(schema) { + return elementKeys(schema).map(function (key) { + return { key: key, layoutKey: layoutKeyFor(schema, key) }; + }); + } + + /** + * One row per positioned thing that has no style block of its own. + * + * Football positions both logos, timeouts, possession, down-and-distance, + * the date, the time and the records, none of which has a font or a + * colour. The editor takes the whole layout section over, so anything + * not drawn here has no control at all. + */ + function positionRows(schema) { + var layout = ownObj((schema && schema.properties) || {}, 'layout'); + var layoutProps = layout.properties || {}; + var claimed = styleRows(schema).map(function (r) { return r.layoutKey; }); + var order = layout['x-propertyOrder'] || Object.keys(layoutProps); + return order.filter(function (lk) { + var entry = own(layoutProps, lk); + // Object-shaped entries carry offsets; a leaf is itself the + // control. Both need a row here, because the editor removes the + // fallback's whole layout section. + return entry && typeof entry === 'object' && claimed.indexOf(lk) === -1; + }).map(function (lk) { + return { key: null, layoutKey: lk }; + }); + } + + function layoutTitleOf(schema, layoutKey) { + var layoutProps = ownObj((schema && schema.properties) || {}, 'layout').properties || {}; + return ownObj(layoutProps, layoutKey).title || layoutKey.replace(/_/g, ' '); + } + /** * 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 a row's style block declares (font, + * font_size, text_color, visible, align), the sub-fields of its layout + * entry (x_offset, y_offset, scale), and a layout entry whose own value + * *is* the field -- a show_logo toggle straight under layout has no x/y + * object underneath, so it gets a column keyed to itself. + * + * `rows` defaults to every row the schema produces, style and position. */ - function columnsFor(schema) { + function columnsFor(schema, rows) { var props = schema.properties || {}; var layoutProps = ownObj(props, 'layout').properties || {}; + rows = rows || styleRows(schema).concat(positionRows(schema)); // A Map, not an object: the keys are field names out of a schema, so // a field literally named "constructor" is a column like any other // and never touches a prototype. var seen = new Map(); - 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'); }); + rows.forEach(function (row) { + if (row.key) { + Object.keys(ownObj(props, row.key).properties || {}).forEach( + function (f) { seen.set(f, 'element'); }); + } + if (!row.layoutKey) { return; } + var entry = own(layoutProps, row.layoutKey); + if (entry && typeof entry === 'object' && entry.properties) { + Object.keys(entry.properties).forEach( + function (f) { seen.set(f, 'layout'); }); + } else if (entry && typeof entry === 'object') { + // A leaf. Namespaced so an unrelated column sharing its name + // -- another entry's "scale" axis, say -- can never shadow it + // and leave this row's only control blank. + seen.set('layout-leaf:' + row.layoutKey, 'layout-leaf'); + } }); var known = COLUMN_ORDER.filter(function (f) { return seen.get(f); }); // Anything the schema declares that this file has never heard of @@ -384,10 +469,12 @@ return COLUMN_ORDER.indexOf(f) === -1; }).sort(); return known.concat(extra).map(function (f) { + var isLeaf = f.indexOf('layout-leaf:') === 0; + var fieldKey = isLeaf ? f.slice('layout-leaf:'.length) : f; return { - key: f, + key: fieldKey, where: seen.get(f), - label: own(COLUMN_LABELS, f) || f.replace(/_/g, ' ') + label: own(COLUMN_LABELS, fieldKey) || fieldKey.replace(/_/g, ' ') }; }); } @@ -421,21 +508,22 @@ function elementRow(opts) { var schema = opts.schema; - var key = opts.key; + var key = opts.row.key; // null for a position-only row + var layoutKey = opts.row.layoutKey; // null for a style-only row var value = opts.value; var optional = opts.optional; - var props = ownObj(schema.properties || {}, key).properties || {}; + var props = key ? (ownObj(schema.properties || {}, key).properties || {}) : {}; var layoutProps = ownObj(schema.properties || {}, 'layout').properties || {}; - var axes = ownObj(layoutProps, key).properties || {}; + var axes = layoutKey ? (ownObj(layoutProps, layoutKey).properties || {}) : {}; var row = el('div', { class: 'style-editor-row grid items-center gap-2 py-1', - 'data-element': key + 'data-element': key || ('layout.' + layoutKey) }); row.appendChild(el('div', { class: 'text-sm text-gray-700 style-editor-label', - text: titleOf(schema, key) + text: key ? titleOf(schema, key) : layoutTitleOf(schema, layoutKey) })); var sizeInput = null; @@ -461,8 +549,12 @@ } 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 belongs to the one position row named after it; + // every other row leaves that cell blank. + var prop = isLeaf ? (col.key === layoutKey ? own(layoutProps, layoutKey) : null) + : inLayout ? own(axes, col.key) : own(props, col.key); var cell; if (!prop) { // This element does not declare that field; keep the grid @@ -470,13 +562,19 @@ row.appendChild(el('span')); return; } - var path = inLayout ? ['layout', key, col.key] : [key, col.key]; - var base = inLayout ? opts.layoutPrefix + '.' + key - : opts.prefix + '.' + key; + var path = isLeaf ? ['layout', layoutKey] + : inLayout ? ['layout', layoutKey, col.key] : [key, col.key]; + // Posted under the key the schema declares, never the style key: + // a plugin's own offset reader looks up layout.score, so a value + // saved as layout.score_text would be kept and never drawn. A + // leaf is the field itself, so its name has no sub-field suffix -- + // the same name the generic renderer would have posted. + var base = (isLeaf || inLayout) ? opts.layoutPrefix + '.' + layoutKey + : 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, @@ -500,11 +598,11 @@ return row; } - function header(columns) { + function header(columns, firstLabel) { var row = el('div', { class: 'style-editor-row style-editor-head grid gap-2 pb-1 mb-1 border-b border-gray-300' }); - ['Element'].concat(columns.map(function (c) { return c.label; })) + [firstLabel || 'Element'].concat(columns.map(function (c) { return c.label; })) .forEach(function (label) { row.appendChild(el('div', { class: 'text-xs font-semibold text-gray-500 uppercase', @@ -516,7 +614,9 @@ function table(opts) { var wrap = el('div', { class: 'style-editor-table' }); - var columns = columnsFor(opts.schema); + // Columns per table, so the positions table does not inherit a Font + // column and the style table does not grow a "home x offset" one. + var columns = columnsFor(opts.schema, opts.rows); // Sized here rather than in CSS: the column count depends on what // the plugin declared. wrap.style.gridTemplateColumns = ''; @@ -524,11 +624,11 @@ 'minmax(7rem, 1.4fr) ' + columns.map(function (c) { return COLUMN_WIDTHS[c.key] || '5rem'; }).join(' ')); - wrap.appendChild(header(columns)); - elementKeys(opts.schema).forEach(function (key) { + wrap.appendChild(header(columns, opts.firstLabel)); + opts.rows.forEach(function (row) { wrap.appendChild(elementRow({ schema: opts.schema, - key: key, + row: row, columns: columns, prefix: opts.prefix, layoutPrefix: opts.prefix + '.layout', @@ -540,6 +640,29 @@ return wrap; } + /** One panel's tables: the styled elements, then anything only positioned. */ + function panelBody(opts) { + var body = el('div'); + var shared = { + schema: opts.schema, prefix: opts.prefix, value: opts.value, + fonts: opts.fonts, optional: opts.optional + }; + var styled = styleRows(opts.schema); + if (styled.length) { + body.appendChild(table(Object.assign({ rows: styled }, shared))); + } + var positioned = positionRows(opts.schema); + if (positioned.length) { + body.appendChild(el('div', { + class: 'text-xs font-semibold text-gray-500 uppercase mt-4 mb-1 style-editor-subhead', + text: 'Other positions' + })); + body.appendChild(table(Object.assign( + { rows: positioned, firstLabel: 'Item' }, shared))); + } + return body; + } + // ---- widget ---------------------------------------------------------- window.LEDMatrixWidgets.register('style-editor', { @@ -555,6 +678,20 @@ var root = el('div', { class: 'style-editor' }); container.appendChild(root); + // Published synchronously, because the host reads it the moment + // this returns while the panels below wait on the font catalog. + // layout and modes count as ours: their fields appear as columns + // in these rows, so leaving them to the generic renderer would + // post every offset twice from two different controls. + var owned = elementKeys(schema); + if (ownObj(schema.properties || {}, 'layout').properties) { + owned = owned.concat(['layout']); + } + if (ownObj(schema.properties || {}, 'modes').properties) { + owned = owned.concat(['modes']); + } + container.dataset.ownedKeys = owned.join(','); + loadFonts().then(function (fonts) { var modeProps = ownObj(schema.properties || {}, 'modes').properties || {}; var modes = Object.keys(modeProps); @@ -596,7 +733,7 @@ tabs.appendChild(b); } - panel('__base__', table({ + panel('__base__', panelBody({ schema: schema, prefix: base, value: current, fonts: fonts, optional: false })); @@ -610,7 +747,7 @@ class: 'text-xs text-gray-500 mb-2', text: 'Anything left blank follows the "All modes" tab.' })); - node.appendChild(table({ + node.appendChild(panelBody({ schema: modeSchema, prefix: base + '.modes.' + mode, value: modeValue, diff --git a/web_interface/templates/v3/partials/plugin_config.html b/web_interface/templates/v3/partials/plugin_config.html index 8249581b..fc7909fb 100644 --- a/web_interface/templates/v3/partials/plugin_config.html +++ b/web_interface/templates/v3/partials/plugin_config.html @@ -97,23 +97,30 @@ var container = document.getElementById('{{ field_id }}_container'); if (container) { container.remove(); } } - function fallbackIsDirty(root) { - if (!root) { return false; } - var controls = root.querySelectorAll('input, select, textarea'); - for (var i = 0; i < controls.length; i++) { - var c = controls[i]; - if (c.type === 'checkbox' || c.type === 'radio') { - if (c.checked !== c.defaultChecked) { return true; } - } else if (c.tagName === 'SELECT') { - for (var j = 0; j < c.options.length; j++) { - if (c.options[j].selected !== c.options[j].defaultSelected) { return true; } - } - } else if (c.value !== c.defaultValue) { - return true; - } - } - return false; - } + // Did a *person* type into the fallback before this widget + // took over? Comparing values against their defaults cannot + // answer that. The fallback holds this block's own font + // fields, and the font-selector widget populates them on the + // same 50ms timer -- so a plain page load with nobody + // touching anything raced into "dirty" (measured: seven + // customization.*.font selects, ~60ms in) and silently + // removed the style editor, leaving the accordion form it + // exists to replace. + // + // isTrusted is false for programmatic changes and true only + // for real input, which is exactly the question. The + // listeners go on now, synchronously, because the edit to + // protect can happen before initWidget ever runs. + var userEdited = false; + (function watchForRealEdits() { + var fb = document.getElementById('{{ field_id }}_fallback'); + if (!fb) { return; } + ['input', 'change'].forEach(function (evt) { + fb.addEventListener(evt, function (e) { + if (e && e.isTrusted) { userEdited = true; } + }, true); + }); + })(); function initWidget() { var widget = window.LEDMatrixWidgets && window.LEDMatrixWidgets.get('style-editor'); if (!widget) { @@ -128,7 +135,7 @@ // while this widget's script is still loading; swapping // in the widget now would replace them with the stale // server-rendered value and silently drop that edit. - if (fallbackIsDirty(fallback)) { container.remove(); return; } + if (userEdited) { container.remove(); return; } var value = {{ obj_value|tojson|safe }}; var config = { schema: {{ prop|tojson|safe }} }; try { @@ -142,9 +149,32 @@ container.remove(); return; } - // The widget owns these fields now; drop the generic - // rendering so the form does not post both. - if (fallback) { fallback.remove(); } + // The widget owns the style blocks now, so drop those from + // the generic rendering or the form would post both. Only + // those: a customization block can also hold a feature of + // its own (football keeps favorite_result_colors there), + // and removing the section wholesale took the only UI that + // feature had with it. + if (fallback) { + var owned = (container.dataset.ownedKeys || '') + .split(',').filter(function (k) { return k; }); + owned.forEach(function (k) { + // CSS.escape: k is a schema property key read + // back off dataset.ownedKeys, not a literal -- + // building the attribute selector by raw + // concatenation would break (or be steerable) + // on a key containing a quote or backslash. + var node = fallback.querySelector( + '[data-child-key="' + CSS.escape(k) + '"]'); + if (node) { node.remove(); } + }); + // Nothing left but the section heading, or a widget + // that reported nothing: drop the lot, as before. + if (!owned.length + || !fallback.querySelector('[data-child-key]')) { + fallback.remove(); + } + } } if (document.readyState === 'loading') { document.addEventListener('DOMContentLoaded', initWidget); @@ -1091,7 +1121,12 @@ {% if nested_key in prop.properties %} {% set nested_prop = prop.properties[nested_key] %} {% set nested_val = nested_value[nested_key] if nested_key in nested_value else none %} - {{ render_field(nested_key, nested_prop, nested_val, full_key, plugin_id) }} + {# Tagged so a widget that takes over part of a block can + drop exactly the children it owns and leave the rest + rendered, rather than removing the whole section. #} +
+ {{ render_field(nested_key, nested_prop, nested_val, full_key, plugin_id) }} +
{% endif %} {% endfor %}