diff --git a/docs/styling.md b/docs/styling.md index c69cf196..4769dc47 100644 --- a/docs/styling.md +++ b/docs/styling.md @@ -124,7 +124,8 @@ Set them on `.xy` or any ancestor: | `--chart-legend-bg` | Legend background | `rgba(128,128,128,.08)` | | `--chart-badge-bg` / `--chart-badge-text` | Reduction badges | `rgba(255,255,255,.82)` / `#0f172a` | | `--chart-modebar-bg` / `--chart-modebar-active` | Modebar / active button | `rgba(255,255,255,.78)` / `rgba(128,128,128,.22)` | -| `--chart-selection` / `--chart-selection-fill` | Selection rectangle | `rgba(90,140,240,.9)` / `…,.15)` | +| `--chart-selection` / `--chart-selection-fill` | Box-select rectangle | `rgba(90,140,240,.9)` / `…,.15)` | +| `--chart-zoom-selection` / `--chart-zoom-selection-fill` | Box-zoom drag rectangle | `rgba(120,120,120,.9)` / `…,.12)` | | `--chart-crosshair` | Crosshair lines | `rgba(15,23,42,.42)` | | `--chart-annotation-text` | Annotation label color | falls back to `--chart-text` | | `--chart-cursor` / `--chart-cursor-pan` | Plot cursor (box-zoom / pan) | `crosshair` / `grab` | diff --git a/js/src/20_theme.js b/js/src/20_theme.js index 251892fd..aeaa4b59 100644 --- a/js/src/20_theme.js +++ b/js/src/20_theme.js @@ -86,6 +86,7 @@ const FC_CHROME_CSS = ` :where(.xy [data-fc-slot="modebar_button"]){width:26px;height:24px;padding:0;border:none;background:transparent;border-radius:3px;color:var(--chart-axis,currentColor);cursor:pointer} :where(.xy [data-fc-slot="modebar_button"].fc-active){background:var(--chart-modebar-active,rgba(128,128,128,.22))} :where(.xy [data-fc-slot="selection"]){border:1px solid var(--chart-selection,rgba(90,140,240,.9));background:var(--chart-selection-fill,rgba(90,140,240,.15))} +:where(.xy [data-fc-slot="selection"][data-fc-band="zoom"]){border-color:var(--chart-zoom-selection,rgba(120,120,120,.9));background:var(--chart-zoom-selection-fill,rgba(120,120,120,.12))} :where(.xy [data-fc-slot="crosshair_x"],.xy [data-fc-slot="crosshair_y"]){background:var(--chart-crosshair,rgba(15,23,42,.42))} :where(.xy [data-fc-slot="tick_label"]){color:var(--chart-text,inherit)} :where(.xy [data-fc-slot="axis_title"]){color:var(--chart-text,inherit);font-size:12px} diff --git a/js/src/50_chartview.js b/js/src/50_chartview.js index 5be659f8..2a37b10f 100644 --- a/js/src/50_chartview.js +++ b/js/src/50_chartview.js @@ -392,7 +392,14 @@ class ChartView { _stylePropertyName(key) { if (key.startsWith("--")) return key; - return key.replace(/[A-Z]/g, (match) => `-${match.toLowerCase()}`); + // Accept snake_case (the Python API form, e.g. `font_size`), camelCase + // (React-style `fontSize`), and kebab-case interchangeably, normalizing all + // to the CSS property name. The underscore pass is load-bearing: Python + // kebab-normalizes keys only for its grammar check and ships the raw key in + // the spec, so without it a validated `font_size` reached + // setProperty("font_size", …) and the browser silently dropped it. It also + // lets the unitless-property check below see the real (kebab) name. + return key.replace(/_/g, "-").replace(/[A-Z]/g, (match) => `-${match.toLowerCase()}`); } _stylePropertyValue(property, value) { @@ -431,7 +438,16 @@ class ChartView { const styles = this.spec.dom?.styles; const style = styles && typeof styles === "object" ? styles[slot] : null; if (!style || typeof style !== "object" || Array.isArray(style)) return null; - if (Object.prototype.hasOwnProperty.call(style, property)) return style[property]; + // Match on the canonical CSS property name so a snake_case key (`max_height`, + // the Python API form), camelCase, and kebab all resolve. _applyStyle already + // normalizes the author's key onto the element, so this guard must too — + // otherwise the responsive max-height cap in _resize re-applies over an + // explicit styles[legend] value on resize (browser-verified: 50px → plot + // height). hasOwnProperty on the raw key alone missed the snake_case form. + const want = this._stylePropertyName(property); + for (const key of Object.keys(style)) { + if (this._stylePropertyName(key) === want) return style[key]; + } return null; } @@ -694,11 +710,9 @@ class ChartView { this.chrome.style.height = this.size.h + "px"; this.chrome.width = this.size.w * this.dpr; this.chrome.height = this.size.h * this.dpr; - if ( - this._legend && - this._slotStyleValue("legend", "max-height") == null && - this._slotStyleValue("legend", "maxHeight") == null - ) { + if (this._legend && this._slotStyleValue("legend", "max-height") == null) { + // _slotStyleValue canonicalizes keys, so this one check honors snake_case + // / camelCase / kebab author styles alike (no separate maxHeight probe). this._legend.style.maxHeight = p.h - 12 + "px"; } this._positionReductionBadges(); diff --git a/js/src/53_interaction.js b/js/src/53_interaction.js index 16b89ded..438d8e09 100644 --- a/js/src/53_interaction.js +++ b/js/src/53_interaction.js @@ -191,13 +191,12 @@ Object.assign(ChartView.prototype, { const px = this.plot.x, py = this.plot.y; const x2 = Math.min(x + w, px + this.plot.w), y2 = Math.min(y + h, py + this.plot.h); const cx = Math.max(x, px), cy = Math.max(y, py); - if (band.mode === "zoom") { - this.selRect.style.border = "1px solid var(--chart-zoom-selection, rgba(120,120,120,.9))"; - this.selRect.style.background = "var(--chart-zoom-selection-fill, rgba(120,120,120,.12))"; - } else { - this.selRect.style.border = "1px solid var(--chart-selection, rgba(90,140,240,.9))"; - this.selRect.style.background = "var(--chart-selection-fill, rgba(90,140,240,.15))"; - } + // Band paint (border/background) is a defeatable :where() default keyed on + // data-fc-band, NOT pinned inline — otherwise a `class_names={"selection":…}` + // utility (or `styles[selection]`) would lose to the inline style, breaking + // the "your styles always win" contract for this one slot. Only the mode + // discriminator and structural position/size stay inline (matches §36). + this.selRect.dataset.fcBand = band.mode === "zoom" ? "zoom" : "select"; this.selRect.style.display = "block"; this.selRect.style.left = cx + "px"; this.selRect.style.top = cy + "px"; diff --git a/python/xy/static/index.js b/python/xy/static/index.js index 8089ef2e..ada47eac 100644 --- a/python/xy/static/index.js +++ b/python/xy/static/index.js @@ -262,6 +262,7 @@ const FC_CHROME_CSS = ` :where(.xy [data-fc-slot="modebar_button"]){width:26px;height:24px;padding:0;border:none;background:transparent;border-radius:3px;color:var(--chart-axis,currentColor);cursor:pointer} :where(.xy [data-fc-slot="modebar_button"].fc-active){background:var(--chart-modebar-active,rgba(128,128,128,.22))} :where(.xy [data-fc-slot="selection"]){border:1px solid var(--chart-selection,rgba(90,140,240,.9));background:var(--chart-selection-fill,rgba(90,140,240,.15))} +:where(.xy [data-fc-slot="selection"][data-fc-band="zoom"]){border-color:var(--chart-zoom-selection,rgba(120,120,120,.9));background:var(--chart-zoom-selection-fill,rgba(120,120,120,.12))} :where(.xy [data-fc-slot="crosshair_x"],.xy [data-fc-slot="crosshair_y"]){background:var(--chart-crosshair,rgba(15,23,42,.42))} :where(.xy [data-fc-slot="tick_label"]){color:var(--chart-text,inherit)} :where(.xy [data-fc-slot="axis_title"]){color:var(--chart-text,inherit);font-size:12px} @@ -1918,7 +1919,7 @@ try { el.classList.add(token); } catch (_) { } } _stylePropertyName(key) { if (key.startsWith("--")) return key; -return key.replace(/[A-Z]/g, (match) => `-${match.toLowerCase()}`); +return key.replace(/_/g, "-").replace(/[A-Z]/g, (match) => `-${match.toLowerCase()}`); } _stylePropertyValue(property, value) { if (typeof value !== "number") return String(value); @@ -1953,7 +1954,10 @@ _slotStyleValue(slot, property) { const styles = this.spec.dom?.styles; const style = styles && typeof styles === "object" ? styles[slot] : null; if (!style || typeof style !== "object" || Array.isArray(style)) return null; -if (Object.prototype.hasOwnProperty.call(style, property)) return style[property]; +const want = this._stylePropertyName(property); +for (const key of Object.keys(style)) { +if (this._stylePropertyName(key) === want) return style[key]; +} return null; } _syncContainerSize() { @@ -2155,11 +2159,7 @@ this.chrome.style.width = this.size.w + "px"; this.chrome.style.height = this.size.h + "px"; this.chrome.width = this.size.w * this.dpr; this.chrome.height = this.size.h * this.dpr; -if ( -this._legend && -this._slotStyleValue("legend", "max-height") == null && -this._slotStyleValue("legend", "maxHeight") == null -) { +if (this._legend && this._slotStyleValue("legend", "max-height") == null) { this._legend.style.maxHeight = p.h - 12 + "px"; } this._positionReductionBadges(); @@ -4874,13 +4874,7 @@ const h = Math.abs(e.clientY - band.sy); const px = this.plot.x, py = this.plot.y; const x2 = Math.min(x + w, px + this.plot.w), y2 = Math.min(y + h, py + this.plot.h); const cx = Math.max(x, px), cy = Math.max(y, py); -if (band.mode === "zoom") { -this.selRect.style.border = "1px solid var(--chart-zoom-selection, rgba(120,120,120,.9))"; -this.selRect.style.background = "var(--chart-zoom-selection-fill, rgba(120,120,120,.12))"; -} else { -this.selRect.style.border = "1px solid var(--chart-selection, rgba(90,140,240,.9))"; -this.selRect.style.background = "var(--chart-selection-fill, rgba(90,140,240,.15))"; -} +this.selRect.dataset.fcBand = band.mode === "zoom" ? "zoom" : "select"; this.selRect.style.display = "block"; this.selRect.style.left = cx + "px"; this.selRect.style.top = cy + "px"; diff --git a/python/xy/static/standalone.js b/python/xy/static/standalone.js index 2f157d17..b26a6630 100644 --- a/python/xy/static/standalone.js +++ b/python/xy/static/standalone.js @@ -263,6 +263,7 @@ const FC_CHROME_CSS = ` :where(.xy [data-fc-slot="modebar_button"]){width:26px;height:24px;padding:0;border:none;background:transparent;border-radius:3px;color:var(--chart-axis,currentColor);cursor:pointer} :where(.xy [data-fc-slot="modebar_button"].fc-active){background:var(--chart-modebar-active,rgba(128,128,128,.22))} :where(.xy [data-fc-slot="selection"]){border:1px solid var(--chart-selection,rgba(90,140,240,.9));background:var(--chart-selection-fill,rgba(90,140,240,.15))} +:where(.xy [data-fc-slot="selection"][data-fc-band="zoom"]){border-color:var(--chart-zoom-selection,rgba(120,120,120,.9));background:var(--chart-zoom-selection-fill,rgba(120,120,120,.12))} :where(.xy [data-fc-slot="crosshair_x"],.xy [data-fc-slot="crosshair_y"]){background:var(--chart-crosshair,rgba(15,23,42,.42))} :where(.xy [data-fc-slot="tick_label"]){color:var(--chart-text,inherit)} :where(.xy [data-fc-slot="axis_title"]){color:var(--chart-text,inherit);font-size:12px} @@ -1919,7 +1920,7 @@ try { el.classList.add(token); } catch (_) { } } _stylePropertyName(key) { if (key.startsWith("--")) return key; -return key.replace(/[A-Z]/g, (match) => `-${match.toLowerCase()}`); +return key.replace(/_/g, "-").replace(/[A-Z]/g, (match) => `-${match.toLowerCase()}`); } _stylePropertyValue(property, value) { if (typeof value !== "number") return String(value); @@ -1954,7 +1955,10 @@ _slotStyleValue(slot, property) { const styles = this.spec.dom?.styles; const style = styles && typeof styles === "object" ? styles[slot] : null; if (!style || typeof style !== "object" || Array.isArray(style)) return null; -if (Object.prototype.hasOwnProperty.call(style, property)) return style[property]; +const want = this._stylePropertyName(property); +for (const key of Object.keys(style)) { +if (this._stylePropertyName(key) === want) return style[key]; +} return null; } _syncContainerSize() { @@ -2156,11 +2160,7 @@ this.chrome.style.width = this.size.w + "px"; this.chrome.style.height = this.size.h + "px"; this.chrome.width = this.size.w * this.dpr; this.chrome.height = this.size.h * this.dpr; -if ( -this._legend && -this._slotStyleValue("legend", "max-height") == null && -this._slotStyleValue("legend", "maxHeight") == null -) { +if (this._legend && this._slotStyleValue("legend", "max-height") == null) { this._legend.style.maxHeight = p.h - 12 + "px"; } this._positionReductionBadges(); @@ -4875,13 +4875,7 @@ const h = Math.abs(e.clientY - band.sy); const px = this.plot.x, py = this.plot.y; const x2 = Math.min(x + w, px + this.plot.w), y2 = Math.min(y + h, py + this.plot.h); const cx = Math.max(x, px), cy = Math.max(y, py); -if (band.mode === "zoom") { -this.selRect.style.border = "1px solid var(--chart-zoom-selection, rgba(120,120,120,.9))"; -this.selRect.style.background = "var(--chart-zoom-selection-fill, rgba(120,120,120,.12))"; -} else { -this.selRect.style.border = "1px solid var(--chart-selection, rgba(90,140,240,.9))"; -this.selRect.style.background = "var(--chart-selection-fill, rgba(90,140,240,.15))"; -} +this.selRect.dataset.fcBand = band.mode === "zoom" ? "zoom" : "select"; this.selRect.style.display = "block"; this.selRect.style.left = cx + "px"; this.selRect.style.top = cy + "px"; diff --git a/tests/test_legend_resize_regression.py b/tests/test_legend_resize_regression.py new file mode 100644 index 00000000..c957fe71 --- /dev/null +++ b/tests/test_legend_resize_regression.py @@ -0,0 +1,172 @@ +"""Behavioral regression: an explicit legend max-height survives a resize. + +The client's responsive legend cap (`_resize`) re-applies an automatic +`max-height` unless the author set one. The guard reads the chrome `styles` +spec, which ships the author's *raw* key — and the Python API form is +snake_case (`max_height`). A raw-key-only lookup missed that form, so on a +responsive resize the automatic cap clobbered an explicit `max_height` +(browser-verified: 50px became the plot height). The source-grep suites cannot +see this — it only manifests in a live browser after a resize — so this test +renders the standalone export in headless Chromium and asserts the computed +`max-height` before *and* after a forced resize. + +Skips (never fails) when no Chromium is available or the headless GL context +can't come up, matching the repo's other browser probes. +""" + +from __future__ import annotations + +import html as html_lib +import json +import re +import subprocess +import sys +import tempfile +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "python")) + +import xy as fc # noqa: E402 +from xy.export import find_chromium # noqa: E402 + +# Capture the standalone render call's return value so the probe can drive the +# view directly (the same swap the visual-regression smoke uses). +_RENDER_CALLS = ( + 'xy.renderStandalone(document.getElementById("chart"), spec, bytes.buffer);', + 'xy.renderStandalone(document.getElementById("chart"), spec, buf);', +) + +# Async probe: wait for the legend, record its computed max-height, force a +# responsive height change (so _resize runs its legend-cap branch), then record +# the max-height again. Result lands on a body attribute for `--dump-dom`. +_PROBE = """ + +""" + + +def _dump_dom(chromium: str, page: Path) -> str | None: + """One headless render pass; None on a chromium-level failure (retryable).""" + try: + proc = subprocess.run( + [ + chromium, + "--headless=new", + "--no-sandbox", + "--disable-dev-shm-usage", + "--allow-file-access-from-files", + "--use-angle=swiftshader", + "--enable-unsafe-swiftshader", + "--hide-scrollbars", + "--window-size=640,480", + "--virtual-time-budget=8000", + "--dump-dom", + page.as_uri(), + ], + capture_output=True, + text=True, + timeout=120, + ) + except subprocess.TimeoutExpired: + return None + return proc.stdout if proc.returncode == 0 else None + + +def _probe_maxheight(chromium: str, document: str, page: Path) -> dict | None: + """Render + probe with retries. Returns the parsed result payload, or None + if every attempt hit an environmental miss (no DOM / GL error / no result). + + Headless probes on shared runners have transient warm-up misses (virtual + time / GL init) that a relaunch clears; a genuine regression fails every + attempt with a *value* mismatch, which we surface — never retry away. + """ + page.write_text(document, encoding="utf-8") + last: str | None = None + for _ in range(3): + dom = _dump_dom(chromium, page) + if dom is None: + continue + error = re.search(r'data-fc-legend-maxheight-error="([^"]*)"', dom) + if error: + last = f"probe error: {html_lib.unescape(error.group(1))}" + continue + match = re.search(r'data-fc-legend-maxheight="([^"]*)"', dom) + if match: + return json.loads(html_lib.unescape(match.group(1))) + last = "probe did not finish (no result attribute)" + if last: + pytest.skip(f"legend resize probe could not run after retries: {last}") + pytest.skip("headless chromium unavailable/failed after retries") + return None # unreachable; keeps type-checkers happy + + +def test_snake_case_legend_max_height_survives_resize() -> None: + chromium = find_chromium() + if not chromium: + pytest.skip("no chromium available for the resize regression probe") + + fig = fc.chart( + fc.line(x=[0, 1, 2, 3], y=[0, 1, 0, 1], name="alpha"), + fc.line(x=[0, 1, 2, 3], y=[1, 0, 1, 0], name="beta"), + fc.legend(), + # snake_case is the documented Python API form; it must reach the client + # AND be honored by the responsive-cap guard on resize. + styles={"legend": {"max_height": 50}}, + width=480, + height=320, + ) + + document = fig.to_html() + render_call = next((call for call in _RENDER_CALLS if call in document), None) + assert render_call is not None, "to_html render call shape changed; update the probe swap" + capture_call = render_call.replace( + "xy.renderStandalone(", "window.__fcProbeView = xy.renderStandalone(", 1 + ) + document = document.replace(render_call, capture_call, 1) + document = document.replace("", _PROBE + "\n", 1) + + with tempfile.TemporaryDirectory() as td: + page = Path(td) / "legend_resize.html" + payload = _probe_maxheight(chromium, document, page) + + # Before the fix, the auto-cap overwrote the explicit 50px with the (grown) + # plot height on resize. It must stay pinned at the author's value. + assert payload["initial"] == "50px", f"explicit max_height not applied at build: {payload}" + assert payload["afterResize"] == "50px", ( + f"resize clobbered the explicit snake_case max_height: {payload}" + ) diff --git a/tests/test_static_client_security.py b/tests/test_static_client_security.py index 412e1350..4012d366 100644 --- a/tests/test_static_client_security.py +++ b/tests/test_static_client_security.py @@ -111,15 +111,26 @@ def test_client_user_text_surfaces_use_text_nodes_not_html() -> None: def test_client_respects_user_legend_max_height_style() -> None: - """The responsive legend cap must not overwrite explicit component styles.""" + """The responsive legend cap must not overwrite explicit component styles. + + _slotStyleValue canonicalizes the stored key, so a single canonical-name + guard honors snake_case (`max_height`, the Python API form), camelCase, and + kebab alike. A raw-key-only lookup let the snake_case form slip past the + guard, so the auto-cap clobbered it on resize (browser-verified 50px → plot + height).""" required_guards = ( 'this._slotStyleValue("legend", "max-height") == null', - 'this._slotStyleValue("legend", "maxHeight") == null', + # the canonical-name match inside _slotStyleValue is what makes the guard + # snake_case-aware — its removal would reintroduce the resize regression. + "if (this._stylePropertyName(key) === want) return style[key];", ) for path, text in CLIENT_FILES: for guard in required_guards: assert guard in text, f"{path} no longer preserves explicit legend max-height" + assert 'this._slotStyleValue("legend", "maxHeight")' not in text, ( + f"{path} still uses the old raw-key double guard; _slotStyleValue now normalizes" + ) def test_client_numeric_styles_default_to_pixels_for_lengths() -> None: @@ -127,6 +138,11 @@ def test_client_numeric_styles_default_to_pixels_for_lengths() -> None: required_style_helpers = ( "const UNITLESS_STYLE_PROPS = new Set([", 'if (key.startsWith("--")) return key;', + # snake_case (the Python API form, e.g. `font_size`) must normalize to + # the CSS property name, not reach setProperty("font_size", …) verbatim + # (a silent no-op in the browser). The underscore pass runs before the + # unitless check so `line_height`/`z_index` are recognized as unitless. + 'key.replace(/_/g, "-").replace(/[A-Z]/g,', 'if (property.startsWith("--") || UNITLESS_STYLE_PROPS.has(property)) return String(value);', "return `${value}px`;", ) @@ -136,6 +152,25 @@ def test_client_numeric_styles_default_to_pixels_for_lengths() -> None: assert helper in text, f"{path} no longer normalizes numeric component styles" +def test_client_selection_band_paint_is_a_defeatable_stylesheet_default() -> None: + """The box-select/zoom band must paint via the zero-specificity :where() + stylesheet (keyed on data-fc-band), never inline — otherwise a + `class_names={"selection": …}` utility or `styles[selection]` would lose to + the inline style, the one slot that breaks the "your styles always win" + contract (§36).""" + for path, text in CLIENT_FILES: + assert "this.selRect.dataset.fcBand =" in text, ( + f"{path} no longer drives the selection band via a data attribute" + ) + assert "selRect.style.border" not in text and "selRect.style.background" not in text, ( + f"{path} pins selection band paint inline; it must be a stylesheet default" + ) + for path, text in THEME_FILES: + assert '[data-fc-slot="selection"][data-fc-band="zoom"]){' in text, ( + f"{path} is missing the defeatable zoom-band :where() default" + ) + + def test_client_applies_every_public_dom_slot() -> None: slot_snippets = { "root": '_applySlot(root, "root")',