diff --git a/src/element_style.py b/src/element_style.py index 6e4a30ae..5a0864d8 100644 --- a/src/element_style.py +++ b/src/element_style.py @@ -761,6 +761,11 @@ 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 @@ -769,12 +774,35 @@ def _adopt_handwritten_block(schema: Dict[str, Any], # 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. + layout_key = next((name for name in alias_keys(key) + if isinstance(layout_fields.get(name), 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 diff --git a/test/web_interface/test_style_editor_takeover.py b/test/web_interface/test_style_editor_takeover.py index 4831fb97..645f644f 100644 --- a/test/web_interface/test_style_editor_takeover.py +++ b/test/web_interface/test_style_editor_takeover.py @@ -125,3 +125,99 @@ class TestTheSchemaSaysWhichBlocksAreStyling: 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 diff --git a/web_interface/static/v3/js/widgets/style-editor.js b/web_interface/static/v3/js/widgets/style-editor.js index 29d0b359..71b2616d 100644 --- a/web_interface/static/v3/js/widgets/style-editor.js +++ b/web_interface/static/v3/js/widgets/style-editor.js @@ -374,18 +374,74 @@ * text_color, visible, align) and the sub-fields their layout blocks * declare (x_offset, y_offset, scale). */ - function columnsFor(schema) { + /** + * 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']; + if (typeof declared === 'string' && own(layoutProps, declared)) { + return declared; + } + return own(layoutProps, key) ? 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) { + return ownObj(layoutProps, lk).properties && 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, ' '); + } + + function columnsFor(schema, rows) { var props = schema.properties || {}; var layoutProps = ownObj(props, 'layout').properties || {}; // 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) { + Object.keys(ownObj(layoutProps, row.layoutKey).properties || {}).forEach( + function (f) { seen.set(f, 'layout'); }); + } }); var known = COLUMN_ORDER.filter(function (f) { return seen.get(f); }); // Anything the schema declares that this file has never heard of @@ -431,21 +487,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; @@ -480,8 +537,11 @@ row.appendChild(el('span')); return; } - var path = inLayout ? ['layout', key, col.key] : [key, col.key]; - var base = inLayout ? opts.layoutPrefix + '.' + key + var path = 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. + var base = inLayout ? opts.layoutPrefix + '.' + layoutKey : opts.prefix + '.' + key; var node = control({ key: col.key, @@ -510,11 +570,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', @@ -526,7 +586,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 = ''; @@ -534,11 +596,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', @@ -550,6 +612,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', { @@ -620,7 +705,7 @@ tabs.appendChild(b); } - panel('__base__', table({ + panel('__base__', panelBody({ schema: schema, prefix: base, value: current, fonts: fonts, optional: false })); @@ -634,7 +719,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,