You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/06/17 03:48:00 UTC
(superset) branch feat/glyph-single-file updated: feat(glyph): remove dead generators module, add SIP doc and defineChart refactor
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch feat/glyph-single-file
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/feat/glyph-single-file by this push:
new d646598f2f2 feat(glyph): remove dead generators module, add SIP doc and defineChart refactor
d646598f2f2 is described below
commit d646598f2f29a83d5bc1586fe8f0f5081ce8a95b
Author: Evan Rusackas <ev...@rusackas.com>
AuthorDate: Tue Jun 16 20:46:34 2026 -0700
feat(glyph): remove dead generators module, add SIP doc and defineChart refactor
Deletes the unused generators.ts/generators.test.ts duplicate code surfaced in
the glyph audit, adds the SIP-GLYPH-SINGLE-FILE-CHARTS design doc, and reworks
defineChart with expanded test coverage. Also threads through related explore
panel and preset updates.
Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
---
superset-frontend/package-lock.json | 1 +
superset-frontend/package.json | 1 +
.../superset-ui-chart-controls/src/types.ts | 2 +
.../SIP-GLYPH-SINGLE-FILE-CHARTS.md | 267 +++++++++++++
.../superset-ui-glyph-core/src/arguments.ts | 57 +--
.../superset-ui-glyph-core/src/defineChart.tsx | 276 +++++++-------
.../superset-ui-glyph-core/src/generators.ts | 419 --------------------
.../packages/superset-ui-glyph-core/src/index.ts | 1 -
.../packages/superset-ui-glyph-core/src/presets.ts | 14 +-
.../packages/superset-ui-glyph-core/src/types.ts | 49 ---
.../test/defineChart.test.tsx | 235 +++++++++++-
.../superset-ui-glyph-core/test/generators.test.ts | 424 ---------------------
.../src/BigNumber/BigNumberWithTrendline/index.tsx | 3 +-
.../plugin-chart-echarts/src/Funnel/index.tsx | 4 +-
.../plugin-chart-echarts/src/Gauge/index.tsx | 3 +
.../src/MixedTimeseries/index.tsx | 2 +
.../plugins/plugin-chart-echarts/src/Pie/index.tsx | 3 +-
.../plugin-chart-point-cluster-map/src/index.tsx | 3 +-
superset-frontend/scripts/check-custom-rules.js | 92 ++---
.../explore/components/ControlPanelsContainer.tsx | 10 +-
.../components/ExploreViewContainer/index.tsx | 20 +-
.../src/explore/components/GlyphOptionsPanel.tsx | 90 ++++-
.../src/visualizations/presets/MainPreset.ts | 4 +-
superset/models/helpers.py | 7 +
24 files changed, 823 insertions(+), 1164 deletions(-)
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
index e5db463e5ea..6bf6ee9ef02 100644
--- a/superset-frontend/package-lock.json
+++ b/superset-frontend/package-lock.json
@@ -50,6 +50,7 @@
"@scarf/scarf": "^1.4.0",
"@superset-ui/chart-controls": "file:./packages/superset-ui-chart-controls",
"@superset-ui/core": "file:./packages/superset-ui-core",
+ "@superset-ui/glyph-core": "file:./packages/superset-ui-glyph-core",
"@superset-ui/legacy-plugin-chart-calendar": "file:./plugins/legacy-plugin-chart-calendar",
"@superset-ui/legacy-plugin-chart-chord": "file:./plugins/legacy-plugin-chart-chord",
"@superset-ui/legacy-plugin-chart-country-map": "file:./plugins/legacy-plugin-chart-country-map",
diff --git a/superset-frontend/package.json b/superset-frontend/package.json
index 2988ab27a74..37d5fefb227 100644
--- a/superset-frontend/package.json
+++ b/superset-frontend/package.json
@@ -133,6 +133,7 @@
"@scarf/scarf": "^1.4.0",
"@superset-ui/chart-controls": "file:./packages/superset-ui-chart-controls",
"@superset-ui/core": "file:./packages/superset-ui-core",
+ "@superset-ui/glyph-core": "file:./packages/superset-ui-glyph-core",
"@superset-ui/legacy-plugin-chart-calendar": "file:./plugins/legacy-plugin-chart-calendar",
"@superset-ui/legacy-plugin-chart-chord": "file:./plugins/legacy-plugin-chart-chord",
"@superset-ui/legacy-plugin-chart-country-map": "file:./plugins/legacy-plugin-chart-country-map",
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
index 0165a280f5b..c3e183ccd62 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
@@ -409,6 +409,8 @@ export interface ControlPanelSectionConfig {
props: ControlPanelsContainerProps,
controlData: AnyDict,
) => boolean;
+ /** @internal Marks the auto-generated glyph "Chart Options" section */
+ _glyphChartOptions?: boolean;
}
export interface StandardizedControls {
diff --git a/superset-frontend/packages/superset-ui-glyph-core/SIP-GLYPH-SINGLE-FILE-CHARTS.md b/superset-frontend/packages/superset-ui-glyph-core/SIP-GLYPH-SINGLE-FILE-CHARTS.md
new file mode 100644
index 00000000000..be20441e2c5
--- /dev/null
+++ b/superset-frontend/packages/superset-ui-glyph-core/SIP-GLYPH-SINGLE-FILE-CHARTS.md
@@ -0,0 +1,267 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# [SIP] Proposal for single-file chart plugins via `defineChart()` (the "Glyph" pattern)
+
+> Draft — to be filed per the [SIP process](https://github.com/apache/superset/issues/5602).
+> The SIP will be numbered by a committer upon acceptance.
+
+### Motivation
+
+Building a Superset visualization plugin today requires authoring and keeping in
+sync five or more files per chart — `index.ts`, `controlPanel.ts`,
+`transformProps.ts`, `buildQuery.ts`, `types.ts`, plus the component — wired
+together through several layers of incidental complexity:
+
+- **String-based control references.** Controls are referenced by name strings
+ (`'metric'`, `'groupby'`, `'y_axis_format'`) that are resolved at runtime
+ through registries and `expandControlConfig`. Typos fail silently; renames
+ require grep-driven archaeology.
+- **Array-of-arrays layouts.** Control panel layout is expressed as
+ `controlSetRows: [[control], [control, control], [<JSX/>]]` — positional
+ nesting that conflates layout with configuration and is hostile to both
+ reading and tooling.
+- **Triplicated knowledge.** The same fact (e.g. "this chart has a `showLegend`
+ boolean defaulting to true") is restated in the control panel (control
+ config), in transformProps (formData extraction + defaulting), and in the
+ component's prop types. The three routinely drift.
+- **No type safety across the seam.** `transformProps` output and component
+ props are connected only by convention; mismatches surface as silently
+ `undefined` props at runtime, not compile errors.
+
+The result is a high floor for contributing a chart, and a maintenance burden
+proportional to (charts × files × duplicated facts).
+
+### Proposed Change
+
+A new workspace package, **`@superset-ui/glyph-core`**, provides a
+**`defineChart()`** function that defines a complete chart plugin in a single
+file. The core idea: **argument definitions are the single source of truth**
+for the control panel, the query, the transform, and the render props.
+
+```tsx
+export default defineChart({
+ metadata: { name: t('My Chart'), thumbnail, behaviors: [Behavior.InteractiveChart] },
+ arguments: {
+ metric: Metric.with({ label: t('Metric') }),
+ groupby: Dimension.with({ label: t('Dimension') }),
+ showLegend: ShowLegend, // reusable preset
+ legendType: { arg: LegendType, visibleWhen: { showLegend: true } },
+ },
+ transform: (chartProps, argValues) => ({ echartOptions: build(chartProps) }),
+ render: props => <Echart {...props} />,
+});
+```
+
+Key elements:
+
+- **Argument classes** (`Metric`, `Dimension`, `Temporal`, `Select`, `Checkbox`,
+ `Int`, `Slider`, `Color`, `ColorPicker`, `RadioButton`, `Bounds`, `Text`,
+ `NumberFormat`, `Currency`, `TimeFormat`, `ConditionalFormatting`) declare
+ what a chart needs; glyph-core generates the matching control configs,
+ formData extraction with defaults, and typed render props.
+- **Declarative conditions** — `visibleWhen` / `disabledWhen` replace
+ imperative `visibility:` functions reaching into Redux controls state, with a
+ single shared evaluator (`evaluateGlyphCondition`) usable from both the
+ legacy pipeline and native rendering.
+- **Generated `buildQuery`** for the common case (built on the core
+ `getXAxisColumn`/`normalizeTimeColumn` helpers so time grain, adhoc x-axis
+ columns, and the `DTTM_ALIAS` fallback behave identically to hand-written
+ queries); charts with post-processing pass their own `buildQuery`.
+- **Generated `transformProps`** maps formData → typed render props (metric
+ label resolution via core `getMetricLabel`); charts with heavy transforms
+ supply a `transform` whose return value is merged into render props.
+- **Escape hatches for incremental migration** — `additionalControls`,
+ `prependSections` / `middleSections` / `additionalSections`,
+ `controlOverrides`, `formDataOverrides`, `onInit`, and
+ `chartOptionsTabOverride` let complex charts keep hand-crafted sections while
+ adopting the pattern for everything else.
+- **Reusable presets** (`presets.ts`) — `ShowLegend`, `LegendType`,
+ `LegendOrientation`, `HeaderFontSize`, `Subtitle`, etc. — shared,
+ pre-translated argument configurations that keep label strings and defaults
+ consistent across chart families.
+- **Native Customize-tab rendering** — `GlyphOptionsPanel` renders glyph
+ arguments directly from formData (value + visibility), bypassing the string
+ expansion pipeline. The generated "Chart Options" section is identified by a
+ structural `_glyphChartOptions` marker (never by label matching). Controls
+ whose config requires `mapStateToProps` (e.g. ConditionalFormatting) and JSX
+ rows (sub-section headers) route through the existing legacy renderer for
+ full compatibility.
+
+### Current Status (what has been done)
+
+All work lives on `feat/glyph-single-file`. **Every chart family in the main
+preset has been consolidated** to `defineChart()`:
+
+- **plugin-chart-echarts**: Pie, Funnel, Gauge, Sankey, Waterfall, Histogram,
+ Tree, Bubble, BoxPlot, Sunburst, Radar, Treemap, Graph, Heatmap, Gantt,
+ BigNumber, BigNumberWithTrendline, BigNumberTotal, BigNumberPeriodOverPeriod,
+ Timeseries (Generic, Area, Bar, Line, Scatter, SmoothLine, Step),
+ MixedTimeseries
+- **preset-chart-deckgl**: all 11 layers (Arc, Contour, Geojson, Grid, Heatmap,
+ Hex, Path, Polygon, Scatter, Screengrid, Multi)
+- **legacy plugins**: calendar, horizon, chord, country-map, world-map,
+ paired-t-test, parallel-coordinates, partition, rose
+- **legacy-preset-chart-nvd3**: Bubble, Bullet, Compare, TimePivot
+- **others**: table, pivot-table, ag-grid-table (light-touch), word-cloud,
+ point-cluster-map, handlebars, cartodiagram
+
+Net effect: **−13,000 lines** (+60k/−73k across 482 files).
+
+**Test coverage added**: a 168-test suite spanning glyph-core unit tests
+(arguments, defineChart, presets, crossFilter — including regression tests for
+every audit finding below), `GlyphOptionsPanel` component tests, plugin-level
+smoke tests for migrated charts, and a Playwright E2E test for the Customize
+tab.
+
+**A full-branch audit (2026-06-11)** ran a seven-angle review over the diff;
+17 findings were confirmed and 14 fixed in-tree, notably:
+
+- `behaviors` no longer defaults to `[Behavior.InteractiveChart]` — charts must
+ opt in, matching `ChartMetadata`'s own default (21 charts had acquired dead
+ cross-filter UI).
+- The real `BigNumberTotalChartPlugin` is registered under
+ `big_number_total` again (a demo plugin had displaced it, breaking saved
+ charts' formats and subheaders).
+- `@superset-ui/glyph-core` added to root `package.json` so webpack's
+ workspace-alias loop resolves live `src/` (fresh builds previously failed).
+- Explore's initial-query effect is one-shot again — filling in the last
+ invalid control no longer auto-fires a query for any chart type.
+- `ChartDefinition.metadata` can now express `queryObjectCount`,
+ `dynamicQueryObjectCount`, `parseMethod`, `suppressContextMenu`, and
+ `enableNoResults` (Mixed Chart's second Results tab returned).
+- `disabledWhen` reads controls from the correct `mapStateToProps` argument
+ (the API was previously non-functional).
+- Generated queries carry `timeGrain` on the x-axis column; generated metric
+ extraction uses `getMetricLabel` and no longer guesses among multiple numeric
+ columns.
+- `NumberFormat`/`TimeFormat` options derive from chart-controls'
+ `D3_FORMAT_OPTIONS` (translated, with previews) instead of stale copies.
+- Dead parallel implementation (`generators.ts`, 419 lines + tests + orphaned
+ types) deleted; condition evaluation deduplicated to a single function.
+- `scripts/check-custom-rules.js` suppression matching is line-based (real
+ ESLint semantics) — one disable comment can no longer silence an entire
+ subtree; the leaks it had hidden were fixed with properly scoped disables.
+
+Branch health: full-monorepo `tsc --noEmit` clean, all 24 packages build, all
+tests and the complete pre-commit suite (mypy, ruff, pylint, oxlint, prettier,
+frontend type-check, custom rules) pass.
+
+### New or Changed Public Interfaces
+
+- **New package** `@superset-ui/glyph-core` (workspace package; public API:
+ `defineChart`, argument classes, presets, `evaluateGlyphCondition`,
+ `getGlyphControlConfig`, `resolveArgClass`, cross-filter utilities).
+- **`@superset-ui/chart-controls`**: `ControlPanelConfig._glyphArgs` and
+ `ControlPanelSectionConfig._glyphChartOptions` — `@internal` structural
+ markers consumed by explore.
+- **`@superset-ui/core`**: `Behavior.AllowsEmptyResults` added to the
+ `Behavior` enum (no producer yet — see Remaining Work).
+- **Explore**: new `GlyphOptionsPanel` component; `ControlPanelsContainer`,
+ `ExploreChartPanel`, `ExploreViewContainer`, `ChartRenderer`, and
+ `getSectionsToRender` learned the glyph path alongside the legacy one.
+- **REST API**: `ChartDataExtrasSchema` accepts an optional
+ `allow_empty_query` boolean (`load_default=False`); `get_sqla_query` honors
+ it for validation while refusing to compile a zero-column SELECT.
+- No new CLI surface, no deployment changes.
+
+### New dependencies
+
+None. `@superset-ui/glyph-core` is a new in-repo workspace package depending
+only on existing workspace packages (`@superset-ui/core`,
+`@superset-ui/chart-controls`). No new npm or PyPI dependencies.
+
+### Migration Plan and Compatibility
+
+- **No database migrations.** Saved charts keep their `viz_type` keys and
+ form data; `defineChart` plugins register under the same keys.
+- **Form-data compatibility** is preserved through generated controls using the
+ same shared control names (`metric`, `groupby`, `adhoc_filters`, `x_axis`,
+ `time_grain_sqla`) and per-chart transforms reading legacy keys. One known
+ gap remains (camelCase arg keys vs. saved snake_case keys — see Remaining
+ Work item 2).
+- **Incremental by design**: the legacy `controlPanel` pipeline still works
+ untouched; glyph charts coexist with non-migrated charts. Removal of legacy
+ controlPanel support is an explicit later phase, gated on the roadmap below.
+
+### Remaining Work / Roadmap
+
+Ordered by priority; items 1–3 should land before this branch merges or
+immediately after.
+
+1. **Restore code-splitting (perf regression).**
+ `loadChart: () => Promise.resolve(Component)` makes every migrated
+ renderer — including maplibre-gl (~800 KB) and deck.gl — eager in the
+ initial bundle, where master lazy-loaded them via
+ `loadChart: () => import('./Chart')`. Add lazy-render support to
+ `defineChart` (e.g. `render: () => import('./Render')` or a
+ `loadRender` field) and move heavy renderers back behind dynamic imports.
+2. **Per-arg `formDataKey` aliases (saved-chart compatibility).**
+ Some migrated charts renamed keys (e.g. BigNumberTotal's `yAxisFormat` /
+ `subtitle` vs. saved `y_axis_format` / `subheader`). Rendering works via
+ camelCase conversion and transform fallbacks, but explore controls show
+ defaults for saved charts and write parallel keys. A
+ `formDataKey: 'y_axis_format'` alias on argument definitions fixes this
+ generally — and is the foundation for settings transfer between viz types
+ (arguments sharing a semantic key carry over on viz-type switch).
+3. **Typed escape hatch for legacy transforms.**
+ ~14 plugins (table, pivot-table, ag-grid, all deckgl layers,
+ point-cluster-map) use `transform: p => transformProps(p as any) as any`
+ plus `render: props => <Comp {...(props as any)} />`. This double-cast class
+ is what previously hid a silent render-props bug. Provide a
+ `defineChart<Props>` overload (typed transform fully replaces render props)
+ or a `wrapLegacyTransform(transformProps, Component)` helper so the next
+ shape mismatch is a compile error.
+4. **Collapse the dual control-panel pipeline.**
+ `defineChart` currently emits a full legacy `ControlPanelConfig` *and*
+ native `_glyphArgs`; visibility/disabled/validation exist in both paths
+ with different data sources (Redux controls vs. formData). Make the native
+ definition canonical and derive the legacy config as a thin adapter — this
+ is the enabler for the planned consistent section/area rendering schema.
+5. **Extract shared timeseries transform logic.**
+ The six Timeseries-family charts each inline ~1,200 lines of largely
+ duplicated transform code. Hoist a shared `timeseriesTransform` into
+ glyph-core (or a glyph-echarts helper) before the copies drift.
+6. **Resolve the `allow_empty_query` plumbing.**
+ Backend accepts the flag and `Behavior.AllowsEmptyResults` exists, but
+ nothing produces either. Either wire the drag-and-drop empty-chart flow
+ end-to-end (derive the extras flag from chart metadata in one place) or
+ drop the speculative plumbing until the feature lands.
+7. **Finish the migration and remove legacy controlPanel support** once 1–4
+ are in place, per the original plan: every plugin on `defineChart`, the
+ string-expansion pipeline (`expandControlConfig`, array-of-arrays sections)
+ deleted, and chart wrappers reduced to enable animation/realtime rendering.
+
+### Rejected Alternatives
+
+- **`createGlyphPlugin()` builder API** (a Map-based generator predating
+ `defineChart`): shipped briefly as `generators.ts`, never adopted by any
+ chart, drifted from the real implementation, and was deleted during the
+ audit in favor of the single `defineChart()` entry point.
+- **JSON/YAML chart manifests**: a pure-data format can describe controls but
+ not transforms or renderers, forcing a second mechanism anyway; a typed
+ TypeScript API keeps definition, transform, and render in one
+ compiler-checked file.
+- **Fixing the legacy controlPanel format in place** (e.g. typed control
+ references over the existing array-of-arrays): preserves the triplication of
+ knowledge across controlPanel/transformProps/component and the runtime
+ string-resolution layer that motivates this SIP.
+- **Big-bang removal of the legacy pipeline**: rejected in favor of
+ coexistence + escape hatches; third-party plugins and complex charts need a
+ migration window.
diff --git a/superset-frontend/packages/superset-ui-glyph-core/src/arguments.ts b/superset-frontend/packages/superset-ui-glyph-core/src/arguments.ts
index 58c60a55e1c..70dc57d367d 100644
--- a/superset-frontend/packages/superset-ui-glyph-core/src/arguments.ts
+++ b/superset-frontend/packages/superset-ui-glyph-core/src/arguments.ts
@@ -17,6 +17,10 @@
* under the License.
*/
+import {
+ D3_FORMAT_OPTIONS,
+ D3_TIME_FORMAT_OPTIONS,
+} from '@superset-ui/chart-controls';
import {
ColumnType,
SelectOptions,
@@ -263,30 +267,13 @@ export class NumberFormat extends Argument {
static override controlType = 'NumberFormatControl';
static default: string = 'SMART_NUMBER';
- // Standard D3 format options
- static readonly FORMAT_OPTIONS: SelectOption[] = [
- { label: 'Adaptive formatting', value: 'SMART_NUMBER' },
- { label: 'Original value', value: '~g' },
- { label: '12,345.432', value: ',.3f' },
- { label: '12,345.43', value: ',.2f' },
- { label: '12,345.4', value: ',.1f' },
- { label: '12,345', value: ',.0f' },
- { label: '12345.432', value: '.3f' },
- { label: '12345.43', value: '.2f' },
- { label: '12345.4', value: '.1f' },
- { label: '12345', value: '.0f' },
- { label: '12K', value: '.0s' },
- { label: '12.3K', value: '.1s' },
- { label: '12.35K', value: '.2s' },
- { label: '12.346K', value: '.3s' },
- { label: '1234543.21%', value: '.2%' },
- { label: '1234543%', value: '.0%' },
- { label: '12.34%', value: '.2r' },
- { label: '+12,345.4', value: '+,.1f' },
- { label: '$12,345.43', value: '$,.2f' },
- { label: 'Duration (1m 6s)', value: 'DURATION' },
- { label: 'Duration (1ms 400µs)', value: 'DURATION_SUB' },
- ];
+ // Standard D3 format options — derived from the canonical list in
+ // @superset-ui/chart-controls so glyph charts offer the same formats
+ // (with previews and translations) as legacy charts, and new formats
+ // propagate automatically.
+ static readonly FORMAT_OPTIONS: SelectOption[] = D3_FORMAT_OPTIONS.map(
+ ([value, label]) => ({ value, label }),
+ );
static with(options: NumberFormatOptions): typeof NumberFormat {
const Base = this;
@@ -334,23 +321,11 @@ export class TimeFormat extends Argument {
static override controlType = 'TimeFormatControl';
static default: string = 'smart_date';
- // Standard D3 time format options
- static readonly FORMAT_OPTIONS: SelectOption[] = [
- { label: 'Adaptive formatting', value: 'smart_date' },
- { label: '%d/%m/%Y | 14/01/2019', value: '%d/%m/%Y' },
- { label: '%m/%d/%Y | 01/14/2019', value: '%m/%d/%Y' },
- { label: '%d.%m.%Y | 14.01.2019', value: '%d.%m.%Y' },
- { label: '%Y-%m-%d | 2019-01-14', value: '%Y-%m-%d' },
- {
- label: '%Y-%m-%d %H:%M:%S | 2019-01-14 01:32:10',
- value: '%Y-%m-%d %H:%M:%S',
- },
- {
- label: '%d-%m-%Y %H:%M:%S | 14-01-2019 01:32:10',
- value: '%d-%m-%Y %H:%M:%S',
- },
- { label: '%H:%M:%S | 01:32:10', value: '%H:%M:%S' },
- ];
+ // Standard D3 time format options — derived from the canonical list in
+ // @superset-ui/chart-controls (see NumberFormat.FORMAT_OPTIONS).
+ static readonly FORMAT_OPTIONS: SelectOption[] = D3_TIME_FORMAT_OPTIONS.map(
+ ([value, label]) => ({ value, label }),
+ );
static with(options: TimeFormatOptions): typeof TimeFormat {
const Base = this;
diff --git a/superset-frontend/packages/superset-ui-glyph-core/src/defineChart.tsx b/superset-frontend/packages/superset-ui-glyph-core/src/defineChart.tsx
index 761a953d0fc..b793cf226be 100644
--- a/superset-frontend/packages/superset-ui-glyph-core/src/defineChart.tsx
+++ b/superset-frontend/packages/superset-ui-glyph-core/src/defineChart.tsx
@@ -38,6 +38,11 @@ import {
ChartLabel,
ChartProps,
buildQueryContext,
+ ensureIsArray,
+ getMetricLabel,
+ getXAxisColumn,
+ isXAxisSet,
+ ParseMethod,
QueryFormData,
} from '@superset-ui/core';
import type {
@@ -53,6 +58,9 @@ import {
Checkbox,
Int,
Color,
+ ColorPicker,
+ RadioButton,
+ Bounds,
Metric,
Dimension,
Temporal,
@@ -64,6 +72,9 @@ import {
isCheckboxArg,
isIntArg,
isColorArg,
+ isColorPickerArg,
+ isRadioButtonArg,
+ isBoundsArg,
isMetricArg,
isDimensionArg,
isTemporalArg,
@@ -202,6 +213,12 @@ export interface ChartDefinition<
canBeAnnotationTypes?: string[];
useLegacyApi?: boolean;
label?: ChartLabel;
+ /** Number of query objects this chart issues (e.g. 2 for Mixed Chart) */
+ queryObjectCount?: number;
+ dynamicQueryObjectCount?: boolean;
+ parseMethod?: ParseMethod;
+ suppressContextMenu?: boolean;
+ enableNoResults?: boolean;
};
/**
@@ -305,7 +322,12 @@ export interface ChartDefinition<
// ============================================================================
function hexToRgba(hex: string): RgbaColor {
- const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hex);
+ // Expand 3-digit shorthand (#fff -> #ffffff) before parsing
+ const normalized = hex.replace(
+ /^#?([a-f\d])([a-f\d])([a-f\d])$/i,
+ (_, r: string, g: string, b: string) => `${r}${r}${g}${g}${b}${b}`,
+ );
+ const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(normalized);
if (result?.[1] && result[2] && result[3]) {
return {
r: parseInt(result[1], 16),
@@ -341,38 +363,6 @@ export function getArgVisibleWhen(
return 'arg' in argDef ? argDef.visibleWhen : undefined;
}
-/**
- * Convert a declarative ArgumentCondition to a visibility function.
- *
- * @example
- * // { showMetricName: true } becomes:
- * ({ controls }) => controls?.showMetricName?.value === true
- *
- * @example
- * // { subtitle: (val) => !!val } becomes:
- * ({ controls }) => !!controls?.subtitle?.value
- */
-function conditionToVisibilityFn(condition: ArgumentCondition): VisibilityFn {
- return ({ controls }) => {
- for (const [argName, expectedValue] of Object.entries(condition)) {
- const actualValue = controls?.[argName]?.value;
-
- if (typeof expectedValue === 'function') {
- // Function check
- if (!expectedValue(actualValue)) {
- return false;
- }
- } else {
- // Equality check
- if (actualValue !== expectedValue) {
- return false;
- }
- }
- }
- return true;
- };
-}
-
/**
* Evaluate an ArgumentCondition directly from formData (no Redux controls state needed).
*
@@ -400,30 +390,35 @@ export function evaluateGlyphCondition(
}
/**
- * Convert a declarative ArgumentCondition to a disabled function.
- * Similar to visibility but returns true when condition IS met (to disable).
+ * Convert a declarative ArgumentCondition to a visibility function.
+ *
+ * @example
+ * // { showMetricName: true } becomes:
+ * ({ controls }) => controls?.showMetricName?.value === true
+ *
+ * @example
+ * // { subtitle: (val) => !!val } becomes:
+ * ({ controls }) => !!controls?.subtitle?.value
*/
-function conditionToDisabledFn(
- condition: ArgumentCondition,
-): (state: { controls: Record<string, { value: unknown }> }) => boolean {
+function conditionToVisibilityFn(condition: ArgumentCondition): VisibilityFn {
return ({ controls }) => {
- for (const [argName, expectedValue] of Object.entries(condition)) {
- const actualValue = controls?.[argName]?.value;
-
- if (typeof expectedValue === 'function') {
- if (!expectedValue(actualValue)) {
- return false;
- }
- } else {
- if (actualValue !== expectedValue) {
- return false;
- }
- }
+ const values: Record<string, unknown> = {};
+ for (const argName of Object.keys(condition)) {
+ values[argName] = controls?.[argName]?.value;
}
- return true;
+ return evaluateGlyphCondition(condition, values);
};
}
+/**
+ * Convert a declarative ArgumentCondition to a disabled function.
+ * Same evaluation as visibility: returns true when the condition IS met
+ * (the caller disables the control when this returns true).
+ */
+const conditionToDisabledFn = conditionToVisibilityFn as (
+ condition: ArgumentCondition,
+) => (state: { controls: Record<string, { value: unknown }> }) => boolean;
+
interface ControlVisibilityConfig {
visibility?: VisibilityFn;
disabled?: (state: {
@@ -466,6 +461,19 @@ function getControlConfig(
const label = argClass.label || paramName;
const description = argClass.description || '';
+ // RadioButton must be checked before Select ('options' in class matches both)
+ if (isRadioButtonArg(argClass)) {
+ const radioClass = argClass as typeof RadioButton;
+ return {
+ type: 'RadioButtonControl',
+ label,
+ description,
+ default: radioClass.default,
+ options: radioClass.options.map(opt => [opt.value, opt.label]),
+ renderTrigger: true,
+ };
+ }
+
if (isSelectArg(argClass)) {
return {
type: 'SelectControl',
@@ -473,7 +481,7 @@ function getControlConfig(
description,
default: argClass.default,
options: argClass.options,
- clearable: false,
+ clearable: argClass.clearable ?? false,
renderTrigger: true,
};
}
@@ -501,6 +509,18 @@ function getControlConfig(
};
}
+ // ColorPicker (RGBA object default) must be checked before Color (hex string
+ // default) — both use controlType 'ColorPickerControl'.
+ if (isColorPickerArg(argClass)) {
+ return {
+ type: 'ColorPickerControl',
+ label,
+ description,
+ default: (argClass as typeof ColorPicker).default,
+ renderTrigger: true,
+ };
+ }
+
if (isColorArg(argClass)) {
// eslint-disable-next-line theme-colors/no-literal-colors
const hexDefault = argClass.default ?? '#000000';
@@ -513,6 +533,16 @@ function getControlConfig(
};
}
+ if (isBoundsArg(argClass)) {
+ return {
+ type: 'BoundsControl',
+ label,
+ description,
+ default: (argClass as typeof Bounds).default,
+ renderTrigger: true,
+ };
+ }
+
if (isNumberFormatArg(argClass)) {
const formatClass = argClass as typeof NumberFormat;
return {
@@ -630,19 +660,22 @@ export const getGlyphControlConfig = getControlConfig;
// Control Panel Generator
// ============================================================================
-function generateControlPanel<TArgs extends ChartArguments>(
- args: TArgs,
- additionalControls?: ChartDefinition<TArgs>['additionalControls'],
- controlOverrides?: ChartDefinition<TArgs>['controlOverrides'],
- additionalControlOverrides?: ChartDefinition<TArgs>['additionalControlOverrides'],
- formDataOverrides?: ChartDefinition<TArgs>['formDataOverrides'],
- onInit?: ChartDefinition<TArgs>['onInit'],
- additionalSections?: ControlPanelSectionConfig[],
- prependSections?: ControlPanelSectionConfig[],
- chartOptionsTabOverride?: 'customize' | 'data',
- middleSections?: ControlPanelSectionConfig[],
- suppressQuerySection?: boolean,
+function generateControlPanel<TArgs extends ChartArguments, TExtra>(
+ definition: ChartDefinition<TArgs, TExtra>,
): ControlPanelConfig {
+ const {
+ arguments: args,
+ additionalControls,
+ controlOverrides,
+ additionalControlOverrides,
+ formDataOverrides,
+ onInit,
+ additionalSections,
+ prependSections,
+ chartOptionsTabOverride,
+ middleSections,
+ suppressQuerySection,
+ } = definition;
const queryControls: ControlSetRow[] = [];
const chartOptionsControls: ControlSetRow[] = [];
@@ -675,16 +708,16 @@ function generateControlPanel<TArgs extends ChartArguments>(
controlConfig.resetOnHide = resetOnHide ?? false;
}
if (disabled) {
- // Superset uses shouldMapStateToProps + mapStateToProps for dynamic disabled state
+ // Superset uses shouldMapStateToProps + mapStateToProps for dynamic disabled state.
+ // Both call sites (controlUtils/getControlState and ControlPanelsContainer)
+ // pass the state holding `controls` as the FIRST argument.
controlConfig.shouldMapStateToProps = () => true;
controlConfig.mapStateToProps = (
- _explore: unknown,
+ state: unknown,
control: { value: unknown },
- chart: unknown,
) => {
- // Get current controls state from the control panel
const controls =
- (chart as { controls?: Record<string, { value: unknown }> })
+ (state as { controls?: Record<string, { value: unknown }> })
?.controls || {};
return {
disabled: disabled({ controls }),
@@ -735,6 +768,9 @@ function generateControlPanel<TArgs extends ChartArguments>(
{
label: t('Chart Options'),
expanded: true,
+ // Structural marker so explore can identify the auto-generated
+ // section without comparing translated label strings.
+ _glyphChartOptions: true,
...(chartOptionsTabOverride
? { tabOverride: chartOptionsTabOverride }
: {}),
@@ -783,15 +819,13 @@ function generateBuildQuery<TArgs extends ChartArguments>(args: TArgs) {
buildQueryContext(formData, baseQueryObject => {
const query = { ...baseQueryObject };
- // Add temporal axis if needed
- if (hasTemporal && formData.x_axis) {
+ // Add the temporal axis column. Using the core helpers (rather than a
+ // hand-built BASE_AXIS object) lets buildQueryContext's
+ // normalizeTimeColumn pass attach timeGrain from extras.time_grain_sqla
+ // and handles adhoc x_axis columns and the DTTM_ALIAS fallback.
+ if (hasTemporal && isXAxisSet(formData)) {
query.columns = [
- {
- columnType: 'BASE_AXIS',
- sqlExpression: formData.x_axis,
- label: formData.x_axis,
- expressionType: 'SQL',
- },
+ ...ensureIsArray(getXAxisColumn(formData)),
...(query.columns || []),
];
}
@@ -807,11 +841,7 @@ function generateBuildQuery<TArgs extends ChartArguments>(args: TArgs) {
function generateTransformProps<
TArgs extends ChartArguments,
TExtra = Record<string, unknown>,
->(
- args: TArgs,
- RenderComponent: FC<RenderProps<TArgs> & TExtra>,
- customTransform?: TransformFn<TArgs, TExtra>,
-) {
+>(args: TArgs, customTransform?: TransformFn<TArgs, TExtra>) {
return (chartProps: ChartProps) => {
const { width, height, queriesData, formData, theme } = chartProps;
const data = queriesData[0]?.data || [];
@@ -831,30 +861,21 @@ function generateTransformProps<
// Handle different argument types
if (isMetricArg(argClass)) {
const metricValue = formData.metric || formData.metrics?.[0];
- // Extract metric label from various formats
- let metricLabel: string;
- if (typeof metricValue === 'string') {
- metricLabel = metricValue;
- } else if (metricValue?.label) {
- metricLabel = metricValue.label;
- } else if (metricValue?.column?.column_name && metricValue?.aggregate) {
- metricLabel = `${metricValue.aggregate}(${metricValue.column.column_name})`;
- } else {
- metricLabel = 'value';
- }
+ // getMetricLabel matches the backend's column-key derivation
+ // (saved metric strings, adhoc labels, aggregate(column), sqlExpression)
+ let metricLabel = metricValue ? getMetricLabel(metricValue) : 'value';
- // Find the actual value - try the label first, then look for first numeric column
+ // Find the actual value by label. Only when the result set has exactly
+ // one numeric column do we fall back to it — guessing among multiple
+ // numeric columns could silently render a dimension value as the metric.
let rawValue = data[0]?.[metricLabel];
if (rawValue === undefined && data[0]) {
- // Try to find the metric value by looking for numeric columns
- const dataKeys = Object.keys(data[0]);
- for (const key of dataKeys) {
- const val = data[0][key];
- if (typeof val === 'number') {
- rawValue = val;
- metricLabel = key;
- break;
- }
+ const numericKeys = Object.keys(data[0]).filter(
+ key => typeof data[0][key] === 'number',
+ );
+ if (numericKeys.length === 1) {
+ [metricLabel] = numericKeys;
+ rawValue = data[0][metricLabel];
}
}
@@ -884,7 +905,10 @@ function generateTransformProps<
);
let value = formData[camelParamName] ?? formData[paramName];
- if (isColorArg(argClass)) {
+ if (isColorPickerArg(argClass)) {
+ // ColorPicker values stay in RGBA-object form (no hex conversion)
+ value = value ?? (argClass as typeof ColorPicker).default;
+ } else if (isColorArg(argClass)) {
const colorClass = argClass as typeof Color;
// eslint-disable-next-line theme-colors/no-literal-colors
const defaultRgba = hexToRgba(colorClass.default ?? '#000000');
@@ -899,6 +923,10 @@ function generateTransformProps<
// eslint-disable-next-line theme-colors/no-literal-colors
value = colorClass.default ?? '#000000';
}
+ } else if (isRadioButtonArg(argClass)) {
+ value = value ?? (argClass as typeof RadioButton).default;
+ } else if (isBoundsArg(argClass)) {
+ value = value ?? (argClass as typeof Bounds).default;
} else if (isNumberFormatArg(argClass)) {
value =
value ?? (argClass as typeof NumberFormat).default ?? 'SMART_NUMBER';
@@ -974,16 +1002,6 @@ export function defineChart<
const {
metadata,
arguments: args,
- additionalControls,
- additionalSections,
- prependSections,
- middleSections,
- chartOptionsTabOverride,
- suppressQuerySection,
- controlOverrides,
- additionalControlOverrides,
- formDataOverrides,
- onInit,
buildQuery: customBuildQuery,
transform,
render,
@@ -994,27 +1012,12 @@ export function defineChart<
render(props);
// Generate everything from arguments (or use custom if provided)
- const controlPanel = generateControlPanel(
- args,
- additionalControls,
- controlOverrides,
- additionalControlOverrides,
- formDataOverrides,
- onInit,
- additionalSections,
- prependSections,
- chartOptionsTabOverride,
- middleSections,
- suppressQuerySection,
- );
- const transformProps = generateTransformProps(
- args,
- ChartComponent,
- transform,
- );
+ const controlPanel = generateControlPanel(definition);
+ const transformProps = generateTransformProps(args, transform);
const buildQuery = customBuildQuery ?? generateBuildQuery(args);
- // Create metadata
+ // Create metadata. behaviors defaults to [] to match ChartMetadata's own
+ // default — charts must opt in to InteractiveChart (cross-filter) etc.
const chartMetadata = new ChartMetadata({
name: metadata.name,
description: metadata.description,
@@ -1022,13 +1025,18 @@ export function defineChart<
tags: metadata.tags,
thumbnail: metadata.thumbnail,
thumbnailDark: metadata.thumbnailDark,
- behaviors: metadata.behaviors || [Behavior.InteractiveChart],
+ behaviors: metadata.behaviors ?? [],
credits: metadata.credits,
exampleGallery: metadata.exampleGallery,
supportedAnnotationTypes: metadata.supportedAnnotationTypes,
canBeAnnotationTypes: metadata.canBeAnnotationTypes,
useLegacyApi: metadata.useLegacyApi,
label: metadata.label,
+ queryObjectCount: metadata.queryObjectCount,
+ dynamicQueryObjectCount: metadata.dynamicQueryObjectCount,
+ parseMethod: metadata.parseMethod,
+ suppressContextMenu: metadata.suppressContextMenu,
+ enableNoResults: metadata.enableNoResults,
});
// Return a ChartPlugin class
diff --git a/superset-frontend/packages/superset-ui-glyph-core/src/generators.ts b/superset-frontend/packages/superset-ui-glyph-core/src/generators.ts
deleted file mode 100644
index 17aa5c70761..00000000000
--- a/superset-frontend/packages/superset-ui-glyph-core/src/generators.ts
+++ /dev/null
@@ -1,419 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-import { t } from '@apache-superset/core/translation';
-import type {
- ControlPanelConfig,
- ControlSetRow,
-} from '@superset-ui/chart-controls';
-import type { ChartProps } from '@superset-ui/core';
-import {
- Argument,
- Select,
- Text,
- Checkbox,
- Int,
- Color,
- isSelectArg,
- isCheckboxArg,
- isIntArg,
- isColorArg,
- isMetricArg,
- isDimensionArg,
- isTemporalArg,
-} from './arguments';
-import type { VisibilityFn, RgbaColor } from './types';
-
-/**
- * Configuration for a glyph argument with optional visibility control
- */
-export interface GlyphArgConfig {
- arg: typeof Argument;
- visibility?: VisibilityFn;
- resetOnHide?: boolean;
-}
-
-/**
- * Arguments map - parameter name to argument class or config
- */
-export type GlyphArguments = Map<string, typeof Argument | GlyphArgConfig>;
-
-/**
- * Convert hex color string to RGBA object for Superset's ColorPickerControl
- */
-function hexToRgba(hex: string): RgbaColor {
- const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hex);
- if (result && result[1] && result[2] && result[3]) {
- return {
- r: parseInt(result[1], 16),
- g: parseInt(result[2], 16),
- b: parseInt(result[3], 16),
- a: 1,
- };
- }
- return { r: 0, g: 0, b: 0, a: 1 };
-}
-
-/**
- * Convert RGBA object to hex color string
- */
-function rgbaToHex(rgba: RgbaColor): string {
- const toHex = (n: number) => n.toString(16).padStart(2, '0');
- return `#${toHex(rgba.r)}${toHex(rgba.g)}${toHex(rgba.b)}`;
-}
-
-/**
- * Get the argument class from a config (handles both direct class and config object)
- */
-function getArgClass(
- argOrConfig: typeof Argument | GlyphArgConfig,
-): typeof Argument {
- return 'arg' in argOrConfig ? argOrConfig.arg : argOrConfig;
-}
-
-/**
- * Get visibility config if present
- */
-function getVisibilityConfig(argOrConfig: typeof Argument | GlyphArgConfig): {
- visibility?: VisibilityFn;
- resetOnHide?: boolean;
-} {
- if ('arg' in argOrConfig) {
- return {
- visibility: argOrConfig.visibility,
- resetOnHide: argOrConfig.resetOnHide,
- };
- }
- return {};
-}
-
-/**
- * Generate Superset control config from a glyph Argument class
- */
-export function getControlConfig(
- argClass: typeof Argument,
- paramName: string,
-): Record<string, unknown> & { type: string } {
- const label = argClass.label || paramName;
- const description = argClass.description || '';
-
- // Select control
- if (isSelectArg(argClass)) {
- return {
- type: 'SelectControl',
- label,
- description,
- default: argClass.default,
- options: argClass.options,
- clearable: argClass.clearable ?? false,
- renderTrigger: true,
- };
- }
-
- // Checkbox control
- if (isCheckboxArg(argClass)) {
- return {
- type: 'CheckboxControl',
- label,
- description,
- default: argClass.default,
- renderTrigger: true,
- };
- }
-
- // Int/Slider control
- if (isIntArg(argClass)) {
- return {
- type: 'SliderControl',
- label,
- description,
- default: argClass.default,
- min: argClass.min,
- max: argClass.max,
- step: argClass.step ?? 1,
- renderTrigger: true,
- };
- }
-
- // Color control
- if (isColorArg(argClass)) {
- // eslint-disable-next-line theme-colors/no-literal-colors
- const hexDefault = argClass.default ?? '#000000';
- return {
- type: 'ColorPickerControl',
- label,
- description,
- default: hexToRgba(hexDefault),
- renderTrigger: true,
- };
- }
-
- // Default to TextControl
- const textClass = argClass as typeof Text;
- return {
- type: 'TextControl',
- label,
- description,
- default: textClass.default ?? '',
- placeholder: textClass.placeholder ?? '',
- renderTrigger: true,
- };
-}
-
-/**
- * Options for control panel generation
- */
-export interface ControlPanelOptions {
- /** Additional control rows for the query section */
- queryControls?: ControlSetRow[];
- /** Additional control rows for the chart options section */
- chartOptionsControls?: ControlSetRow[];
- /** Control overrides */
- controlOverrides?: Record<string, Record<string, unknown>>;
- /** Form data overrides function */
- formDataOverrides?: (
- formData: Record<string, unknown>,
- ) => Record<string, unknown>;
-}
-
-/**
- * Generate a complete ControlPanelConfig from glyph arguments
- *
- * This is the core function that converts semantic argument definitions
- * into Superset's control panel format.
- */
-export function generateControlPanel(
- glyphArguments: GlyphArguments,
- options: ControlPanelOptions = {},
-): ControlPanelConfig {
- const queryControls: ControlSetRow[] = [];
- const chartOptionsControls: ControlSetRow[] = [];
-
- // Process each argument
- for (const [paramName, argOrConfig] of glyphArguments) {
- const argClass = getArgClass(argOrConfig);
- const { visibility, resetOnHide } = getVisibilityConfig(argOrConfig);
-
- // Data arguments go in Query section
- if (isMetricArg(argClass)) {
- queryControls.push(['metric']);
- continue;
- }
-
- if (isDimensionArg(argClass)) {
- queryControls.push(['groupby']);
- continue;
- }
-
- if (isTemporalArg(argClass)) {
- queryControls.push(['x_axis'], ['time_grain_sqla']);
- continue;
- }
-
- // Style/visual arguments go in Chart Options section
- const controlConfig = getControlConfig(argClass, paramName);
-
- // Add visibility if specified
- if (visibility) {
- controlConfig.visibility = visibility;
- controlConfig.resetOnHide = resetOnHide ?? false;
- }
-
- chartOptionsControls.push([
- {
- name: paramName,
- config: controlConfig,
- },
- ]);
- }
-
- // Add adhoc_filters to query section
- queryControls.push(['adhoc_filters']);
-
- // Merge with additional controls from options
- const finalQueryControls = [
- ...queryControls,
- ...(options.queryControls || []),
- ];
- const finalChartOptionsControls = [
- ...chartOptionsControls,
- ...(options.chartOptionsControls || []),
- ];
-
- const config: ControlPanelConfig = {
- controlPanelSections: [
- {
- label: t('Query'),
- expanded: true,
- controlSetRows: finalQueryControls,
- },
- {
- label: t('Chart Options'),
- expanded: true,
- controlSetRows: finalChartOptionsControls,
- },
- ],
- };
-
- if (options.controlOverrides) {
- config.controlOverrides = options.controlOverrides;
- }
-
- if (options.formDataOverrides) {
- // Type assertion needed because SqlaFormData is more specific than Record<string, unknown>
- config.formDataOverrides =
- options.formDataOverrides as ControlPanelConfig['formDataOverrides'];
- }
-
- return config;
-}
-
-/**
- * Options for transformProps generation
- */
-export interface TransformPropsOptions<TResult> {
- /** Custom transformation function that receives extracted values */
- transform?: (
- values: Record<string, unknown>,
- chartProps: ChartProps,
- ) => TResult;
- /** Additional props to pass through from chartProps */
- passthrough?: (keyof ChartProps)[];
-}
-
-/**
- * Generate a transformProps function from glyph arguments
- *
- * This extracts values from formData based on argument definitions,
- * applying type conversions as needed (e.g., RGBA to hex for colors).
- */
-export function generateTransformProps<TResult = Record<string, unknown>>(
- glyphArguments: GlyphArguments,
- options: TransformPropsOptions<TResult> = {},
-): (chartProps: ChartProps) => TResult {
- return (chartProps: ChartProps) => {
- const { formData, width, height, queriesData } = chartProps;
- const values: Record<string, unknown> = {
- width,
- height,
- queriesData,
- };
-
- // Add passthrough props
- if (options.passthrough) {
- for (const key of options.passthrough) {
- values[key] = chartProps[key];
- }
- }
-
- // Extract values from formData based on argument definitions
- for (const [paramName, argOrConfig] of glyphArguments) {
- const argClass = getArgClass(argOrConfig);
-
- // Skip data arguments (metric, dimension, temporal) - these are handled differently
- if (
- isMetricArg(argClass) ||
- isDimensionArg(argClass) ||
- isTemporalArg(argClass)
- ) {
- continue;
- }
-
- // Get value from formData, using default if not present
- let value = formData[paramName];
-
- // Color control: convert RGBA object to hex string
- if (isColorArg(argClass)) {
- const colorClass = argClass as typeof Color;
- // eslint-disable-next-line theme-colors/no-literal-colors
- const defaultRgba = hexToRgba(colorClass.default ?? '#000000');
- const colorValue = value ?? defaultRgba;
-
- if (
- typeof colorValue === 'object' &&
- colorValue !== null &&
- 'r' in colorValue
- ) {
- value = rgbaToHex(colorValue as RgbaColor);
- } else if (typeof colorValue === 'string') {
- value = colorValue;
- } else {
- // eslint-disable-next-line theme-colors/no-literal-colors
- value = colorClass.default ?? '#000000';
- }
- }
- // Select control: use default if no value
- else if (isSelectArg(argClass)) {
- const selectClass = argClass as typeof Select;
- value = value ?? selectClass.default;
- }
- // Checkbox control: use default if no value
- else if (isCheckboxArg(argClass)) {
- const checkboxClass = argClass as typeof Checkbox;
- value = value ?? checkboxClass.default ?? false;
- }
- // Int control: use default if no value
- else if (isIntArg(argClass)) {
- const intClass = argClass as typeof Int;
- value = value ?? intClass.default ?? 0;
- }
- // Text control: use default if no value
- else {
- const textClass = argClass as typeof Text;
- value = value ?? textClass.default ?? '';
- }
-
- values[paramName] = value;
- }
-
- // Apply custom transformation if provided
- if (options.transform) {
- return options.transform(values, chartProps);
- }
-
- return values as TResult;
- };
-}
-
-/**
- * Combined result of creating a glyph plugin
- */
-export interface GlyphPluginDef<TProps> {
- controlPanel: ControlPanelConfig;
- transformProps: (chartProps: ChartProps) => TProps;
-}
-
-/**
- * Create both controlPanel and transformProps from a single argument definition
- *
- * This is the main entry point for the single-file viz pattern.
- */
-export function createGlyphPlugin<TProps = Record<string, unknown>>(
- glyphArguments: GlyphArguments,
- controlPanelOptions: ControlPanelOptions = {},
- transformPropsOptions: TransformPropsOptions<TProps> = {},
-): GlyphPluginDef<TProps> {
- return {
- controlPanel: generateControlPanel(glyphArguments, controlPanelOptions),
- transformProps: generateTransformProps(
- glyphArguments,
- transformPropsOptions,
- ),
- };
-}
diff --git a/superset-frontend/packages/superset-ui-glyph-core/src/index.ts b/superset-frontend/packages/superset-ui-glyph-core/src/index.ts
index 7f281876233..e7c40e07fcc 100644
--- a/superset-frontend/packages/superset-ui-glyph-core/src/index.ts
+++ b/superset-frontend/packages/superset-ui-glyph-core/src/index.ts
@@ -65,7 +65,6 @@
// Re-export everything
export * from './types';
export * from './arguments';
-export * from './generators';
export * from './defineChart';
export * from './presets';
export * from './crossFilter';
diff --git a/superset-frontend/packages/superset-ui-glyph-core/src/presets.ts b/superset-frontend/packages/superset-ui-glyph-core/src/presets.ts
index b445fbf11ec..1ece388adec 100644
--- a/superset-frontend/packages/superset-ui-glyph-core/src/presets.ts
+++ b/superset-frontend/packages/superset-ui-glyph-core/src/presets.ts
@@ -110,8 +110,10 @@ export const Subtitle = Text.with({
* Common toggle for charts with legends
*/
export const ShowLegend = Checkbox.with({
- label: t('Show Legend'),
- description: t('Whether to display the chart legend'),
+ // Strings match plugin-chart-echarts' legend controls so existing i18n
+ // catalogs apply and legend UX stays consistent across chart families.
+ label: t('Show legend'),
+ description: t('Whether to display a legend for the chart'),
default: true,
});
@@ -154,8 +156,8 @@ export const LEGEND_SORT_OPTIONS: SelectOption[] = [
* Choose between scrollable or plain list legend
*/
export const LegendType = Select.with({
- label: t('Legend Type'),
- description: t('Type of legend display'),
+ label: t('Type'),
+ description: t('Legend type'),
options: LEGEND_TYPE_OPTIONS,
default: 'scroll',
});
@@ -165,8 +167,8 @@ export const LegendType = Select.with({
* Position the legend relative to the chart
*/
export const LegendOrientation = Select.with({
- label: t('Legend Orientation'),
- description: t('Position of the legend'),
+ label: t('Orientation'),
+ description: t('Legend Orientation'),
options: LEGEND_ORIENTATION_OPTIONS,
default: 'top',
});
diff --git a/superset-frontend/packages/superset-ui-glyph-core/src/types.ts b/superset-frontend/packages/superset-ui-glyph-core/src/types.ts
index d90b6c16bd6..bee1d9aea10 100644
--- a/superset-frontend/packages/superset-ui-glyph-core/src/types.ts
+++ b/superset-frontend/packages/superset-ui-glyph-core/src/types.ts
@@ -17,9 +17,6 @@
* under the License.
*/
-import type { ControlPanelConfig } from '@superset-ui/chart-controls';
-import type { ChartProps } from '@superset-ui/core';
-
/**
* Option for Select controls
*/
@@ -216,16 +213,6 @@ export enum ColumnType {
Argument = 'argument',
}
-/**
- * Base argument class interface
- */
-export interface ArgumentClass {
- label: string | null;
- description: string | null;
- columnType?: ColumnType;
- controlType?: string;
-}
-
/**
* RGBA color format used by Superset's ColorPickerControl
*/
@@ -268,39 +255,3 @@ export type ArgumentCondition = Record<
string,
unknown | ((value: unknown) => boolean)
>;
-
-/**
- * Extended control configuration with visibility
- */
-export interface ControlConfig {
- name: string;
- config: Record<string, unknown>;
- visibility?: VisibilityFn;
- resetOnHide?: boolean;
-}
-
-/**
- * Glyph chart definition
- */
-export interface GlyphChartDef<TArgs extends Record<string, ArgumentClass>> {
- arguments: TArgs;
- sections?: {
- query?: ControlConfig[][];
- chartOptions?: ControlConfig[][];
- };
-}
-
-/**
- * Result of createGlyphPlugin
- */
-export interface GlyphPluginResult<TFormData> {
- controlPanel: ControlPanelConfig;
- transformProps: (chartProps: ChartProps) => TFormData;
-}
-
-/**
- * Type helper to extract form data types from argument definitions
- */
-export type ArgumentsToFormData<TArgs extends Record<string, ArgumentClass>> = {
- [K in keyof TArgs]: TArgs[K] extends { default: infer D } ? D : unknown;
-};
diff --git a/superset-frontend/packages/superset-ui-glyph-core/test/defineChart.test.tsx b/superset-frontend/packages/superset-ui-glyph-core/test/defineChart.test.tsx
index c3b84f103a9..90c30f7b880 100644
--- a/superset-frontend/packages/superset-ui-glyph-core/test/defineChart.test.tsx
+++ b/superset-frontend/packages/superset-ui-glyph-core/test/defineChart.test.tsx
@@ -23,14 +23,19 @@ import {
ChartPlugin,
} from '@superset-ui/core';
import {
+ Bounds,
Checkbox,
+ ColorPicker,
defineChart,
Dimension,
evaluateGlyphCondition,
getArgVisibleWhen,
+ getGlyphControlConfig,
Metric,
+ RadioButton,
resolveArgClass,
Select,
+ Temporal,
Text,
} from '@superset-ui/glyph-core';
import type { ChartDefinition } from '@superset-ui/glyph-core/defineChart';
@@ -42,8 +47,9 @@ function instantiate(PluginClass: ReturnType<typeof defineChart>) {
// (set via super({ controlPanel }) in defineChart's GlyphChartPlugin).
return {
plugin,
- controlPanel: (plugin as unknown as { controlPanel: Record<string, unknown> })
- .controlPanel,
+ controlPanel: (
+ plugin as unknown as { controlPanel: Record<string, unknown> }
+ ).controlPanel,
metadata: (plugin as unknown as { metadata: ChartMetadata }).metadata,
};
}
@@ -144,7 +150,7 @@ describe('defineChart - basic plugin construction', () => {
expect(metadata.thumbnail).toBe(MIN_THUMBNAIL);
});
- test('metadata defaults Behavior.InteractiveChart when omitted', () => {
+ test('metadata defaults behaviors to [] when omitted (matching ChartMetadata)', () => {
const Plugin = defineChart({
metadata: { name: 'Test', thumbnail: MIN_THUMBNAIL },
arguments: {},
@@ -152,7 +158,28 @@ describe('defineChart - basic plugin construction', () => {
render: () => null as unknown as React.ReactElement,
});
const { metadata } = instantiate(Plugin);
- expect(metadata.behaviors).toContain(Behavior.InteractiveChart);
+ // Charts must opt in to InteractiveChart — defaulting it on would enroll
+ // non-interactive charts in dashboard cross-filter UI and configuration.
+ expect(metadata.behaviors).toEqual([]);
+ });
+
+ test('metadata forwards queryObjectCount and related fields', () => {
+ const Plugin = defineChart({
+ metadata: {
+ name: 'Test',
+ thumbnail: MIN_THUMBNAIL,
+ queryObjectCount: 2,
+ suppressContextMenu: true,
+ enableNoResults: false,
+ },
+ arguments: {},
+ transform: () => ({}),
+ render: () => null as unknown as React.ReactElement,
+ });
+ const { metadata } = instantiate(Plugin);
+ expect(metadata.queryObjectCount).toBe(2);
+ expect(metadata.suppressContextMenu).toBe(true);
+ expect(metadata.enableNoResults).toBe(false);
});
test('metadata behaviors override the default when provided', () => {
@@ -481,8 +508,9 @@ describe('defineChart - custom buildQuery / transform', () => {
});
const p = new Plugin();
// ChartPlugin stores it as a sanitized loader
- const loader = (p as unknown as { loadBuildQuery?: () => Promise<Function> })
- .loadBuildQuery;
+ const loader = (
+ p as unknown as { loadBuildQuery?: () => Promise<Function> }
+ ).loadBuildQuery;
expect(loader).toBeDefined();
const fn = await (loader as () => Promise<Function>)();
fn({ viz_type: 'test', datasource: '1__table' });
@@ -564,3 +592,198 @@ describe('defineChart - visibleWhen with object-form ArgDef', () => {
expect(lp.visibleWhen).toEqual({ showLegend: true });
});
});
+
+describe('defineChart - getGlyphControlConfig argument class coverage', () => {
+ test('RadioButton maps to RadioButtonControl (not SelectControl)', () => {
+ const Choice = RadioButton.with({
+ label: 'Mode',
+ default: 'a',
+ options: [
+ { label: 'A', value: 'a' },
+ { label: 'B', value: 'b' },
+ ],
+ });
+ const config = getGlyphControlConfig(Choice, 'mode');
+ expect(config.type).toBe('RadioButtonControl');
+ expect(config.default).toBe('a');
+ expect(config.options).toEqual([
+ ['a', 'A'],
+ ['b', 'B'],
+ ]);
+ });
+
+ test('ColorPicker keeps its RGBA object default (no hex coercion)', () => {
+ const red = { r: 255, g: 0, b: 0, a: 1 };
+ const Picker = ColorPicker.with({ label: 'Stroke', default: red });
+ const config = getGlyphControlConfig(Picker, 'stroke');
+ expect(config.type).toBe('ColorPickerControl');
+ expect(config.default).toEqual(red);
+ });
+
+ test('Bounds maps to BoundsControl (not TextControl)', () => {
+ const Range = Bounds.with({ label: 'Y bounds', default: [0, 100] });
+ const config = getGlyphControlConfig(Range, 'y_bounds');
+ expect(config.type).toBe('BoundsControl');
+ expect(config.default).toEqual([0, 100]);
+ });
+
+ test('Select honors clearable from the argument class', () => {
+ const config = getGlyphControlConfig(
+ Select.with({ label: 'S', options: [{ label: 'A', value: 'a' }] }),
+ 's',
+ );
+ expect(config.clearable).toBe(false);
+ });
+});
+
+describe('defineChart - disabledWhen wiring', () => {
+ test('mapStateToProps reads controls from the state (first) argument', () => {
+ const Plugin = defineChart({
+ metadata: { name: 'D', thumbnail: MIN_THUMBNAIL },
+ arguments: {
+ subtitle: Text.with({ label: 'Subtitle', default: '' }),
+ subtitleFontSize: {
+ arg: Select.with({
+ label: 'Size',
+ default: 'm',
+ options: [{ label: 'M', value: 'm' }],
+ }),
+ disabledWhen: { subtitle: (val: unknown) => !val },
+ },
+ },
+ transform: () => ({}),
+ render: () => null as unknown as React.ReactElement,
+ });
+ const { controlPanel } = instantiate(Plugin);
+ const sections = controlPanel.controlPanelSections as Array<{
+ controlSetRows: Array<
+ Array<{ name: string; config: Record<string, Function> }>
+ >;
+ }>;
+ const control = sections
+ .flatMap(s => s.controlSetRows)
+ .flat()
+ .find(c => c?.name === 'subtitleFontSize');
+ expect(control).toBeDefined();
+ const { mapStateToProps, shouldMapStateToProps } = control!.config;
+ expect(shouldMapStateToProps()).toBe(true);
+
+ // Both real call sites pass the controls-bearing state as the FIRST arg
+ const disabledResult = mapStateToProps(
+ { controls: { subtitle: { value: '' } } },
+ { value: 'm' },
+ );
+ expect(disabledResult.disabled).toBe(true);
+
+ const enabledResult = mapStateToProps(
+ { controls: { subtitle: { value: 'hello' } } },
+ { value: 'm' },
+ );
+ expect(enabledResult.disabled).toBe(false);
+ });
+});
+
+describe('defineChart - generated buildQuery temporal axis', () => {
+ test('x_axis column is normalized to BASE_AXIS with timeGrain', async () => {
+ const Plugin = defineChart({
+ metadata: { name: 'T', thumbnail: MIN_THUMBNAIL },
+ arguments: { time: Temporal, metric: Metric },
+ transform: () => ({}),
+ render: () => null as unknown as React.ReactElement,
+ });
+ const p = new Plugin();
+ const loader = (p as unknown as { loadBuildQuery: () => Promise<Function> })
+ .loadBuildQuery;
+ const buildQuery = await loader();
+ const queryContext = buildQuery({
+ datasource: '1__table',
+ viz_type: 'test',
+ x_axis: 'ds',
+ time_grain_sqla: 'P1D',
+ metrics: ['count'],
+ });
+ const axisColumn = queryContext.queries[0].columns.find(
+ (col: { columnType?: string }) =>
+ typeof col === 'object' && col?.columnType === 'BASE_AXIS',
+ );
+ expect(axisColumn).toBeDefined();
+ expect(axisColumn.sqlExpression).toBe('ds');
+ // The grain selected in the control panel must reach the query
+ expect(axisColumn.timeGrain).toBe('P1D');
+ });
+});
+
+describe('defineChart - generated metric extraction', () => {
+ async function getTransformProps(
+ PluginClass: ReturnType<typeof defineChart>,
+ ) {
+ const p = new PluginClass();
+ const loader = (
+ p as unknown as { loadTransformProps: () => Promise<Function> }
+ ).loadTransformProps;
+ return loader();
+ }
+
+ const MetricPlugin = defineChart({
+ metadata: { name: 'M', thumbnail: MIN_THUMBNAIL },
+ arguments: { metric: Metric },
+ render: () => null as unknown as React.ReactElement,
+ });
+
+ test('label-less SQL adhoc metrics resolve via sqlExpression', async () => {
+ const transformProps = await getTransformProps(MetricPlugin);
+ const props = transformProps({
+ width: 1,
+ height: 1,
+ formData: {
+ metric: { expressionType: 'SQL', sqlExpression: 'SUM(x)/SUM(y)' },
+ },
+ queriesData: [{ data: [{ 'SUM(x)/SUM(y)': 42 }] }],
+ });
+ expect(props.metric.name).toBe('SUM(x)/SUM(y)');
+ expect(props.metric.value).toBe(42);
+ });
+
+ test('does NOT guess among multiple numeric columns on label miss', async () => {
+ const transformProps = await getTransformProps(MetricPlugin);
+ const props = transformProps({
+ width: 1,
+ height: 1,
+ formData: { metric: 'count' },
+ queriesData: [{ data: [{ year: 2024, total: 9 }] }],
+ });
+ // Guessing could render a numeric dimension (e.g. a year) as the metric
+ expect(props.metric.value).toBeUndefined();
+ });
+
+ test('falls back to the only numeric column when unambiguous', async () => {
+ const transformProps = await getTransformProps(MetricPlugin);
+ const props = transformProps({
+ width: 1,
+ height: 1,
+ formData: { metric: 'count' },
+ queriesData: [{ data: [{ name: 'a', total: 9 }] }],
+ });
+ expect(props.metric.value).toBe(9);
+ expect(props.metric.name).toBe('total');
+ });
+});
+
+describe('defineChart - Chart Options section marker', () => {
+ test('the generated section carries the structural _glyphChartOptions flag', () => {
+ const Plugin = defineChart({
+ metadata: { name: 'S', thumbnail: MIN_THUMBNAIL },
+ arguments: { title: Text.with({ label: 'Title', default: '' }) },
+ transform: () => ({}),
+ render: () => null as unknown as React.ReactElement,
+ });
+ const { controlPanel } = instantiate(Plugin);
+ const sections = controlPanel.controlPanelSections as Array<{
+ label?: string;
+ _glyphChartOptions?: boolean;
+ }>;
+ const chartOptions = sections.find(s => s?._glyphChartOptions);
+ expect(chartOptions).toBeDefined();
+ expect(chartOptions?.label).toBe('Chart Options');
+ });
+});
diff --git a/superset-frontend/packages/superset-ui-glyph-core/test/generators.test.ts b/superset-frontend/packages/superset-ui-glyph-core/test/generators.test.ts
deleted file mode 100644
index 808468afcb8..00000000000
--- a/superset-frontend/packages/superset-ui-glyph-core/test/generators.test.ts
+++ /dev/null
@@ -1,424 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-import type { ChartProps } from '@superset-ui/core';
-import {
- Checkbox,
- Color,
- createGlyphPlugin,
- Dimension,
- generateControlPanel,
- generateTransformProps,
- getControlConfig,
- Int,
- Metric,
- Select,
- Temporal,
- Text,
-} from '@superset-ui/glyph-core';
-import type { Argument } from '@superset-ui/glyph-core';
-import type {
- GlyphArgConfig,
- GlyphArguments,
-} from '@superset-ui/glyph-core/generators';
-
-// `new Map([...])` narrows its value type from the first tuple, which trips
-// up TS when an args map mixes Metric / Checkbox / Select entries. This
-// helper widens the value type to typeof Argument | GlyphArgConfig.
-type GlyphArgValue = typeof Argument | GlyphArgConfig;
-const glyphMap = (entries: Array<readonly [string, GlyphArgValue]>) =>
- new Map<string, GlyphArgValue>(entries) as GlyphArguments;
-
-describe('getControlConfig - per argument type', () => {
- test('Select → SelectControl with options and clearable=false', () => {
- const S = Select.with({
- label: 'Choose',
- default: 'a',
- options: [{ label: 'A', value: 'a' }],
- });
- const cfg = getControlConfig(S, 'myParam');
- expect(cfg.type).toBe('SelectControl');
- expect(cfg.label).toBe('Choose');
- expect(cfg.default).toBe('a');
- expect(cfg.options).toEqual([{ label: 'A', value: 'a' }]);
- expect(cfg.clearable).toBe(false);
- expect(cfg.renderTrigger).toBe(true);
- });
-
- test('Checkbox → CheckboxControl with default', () => {
- const C = Checkbox.with({ label: 'Show', default: true });
- const cfg = getControlConfig(C, 'show');
- expect(cfg.type).toBe('CheckboxControl');
- expect(cfg.label).toBe('Show');
- expect(cfg.default).toBe(true);
- expect(cfg.renderTrigger).toBe(true);
- });
-
- test('Int → SliderControl with min/max/step', () => {
- const I = Int.with({
- label: 'Limit',
- default: 50,
- min: 0,
- max: 1000,
- step: 5,
- });
- const cfg = getControlConfig(I, 'limit');
- expect(cfg.type).toBe('SliderControl');
- expect(cfg.label).toBe('Limit');
- expect(cfg.default).toBe(50);
- expect(cfg.min).toBe(0);
- expect(cfg.max).toBe(1000);
- expect(cfg.step).toBe(5);
- });
-
- test('Color (hex) → ColorPickerControl with RGBA default', () => {
- const C = Color.with({ label: 'Fill', default: '#ff0000' });
- const cfg = getControlConfig(C, 'fill');
- expect(cfg.type).toBe('ColorPickerControl');
- expect(cfg.label).toBe('Fill');
- expect(cfg.default).toEqual({ r: 255, g: 0, b: 0, a: 1 });
- });
-
- test('Text → TextControl with placeholder', () => {
- const T = Text.with({
- label: 'Title',
- default: 'Untitled',
- placeholder: 'Enter…',
- });
- const cfg = getControlConfig(T, 'title');
- expect(cfg.type).toBe('TextControl');
- expect(cfg.label).toBe('Title');
- expect(cfg.default).toBe('Untitled');
- expect(cfg.placeholder).toBe('Enter…');
- });
-
- test('falls back to paramName when label is unset', () => {
- // Use the raw Text class (label: null on Argument)
- class Bare extends Text {
- static override label = null;
- }
- const cfg = getControlConfig(Bare, 'fallback_name');
- expect(cfg.label).toBe('fallback_name');
- });
-});
-
-describe('generateControlPanel', () => {
- test('produces Query and Chart Options sections', () => {
- const args = glyphMap([
- ['metric', Metric],
- ['showLegend', Checkbox.with({ label: 'Legend', default: true })],
- ]);
- const cp = generateControlPanel(args);
- const labels = cp.controlPanelSections.map(s =>
- s && 'label' in s ? s.label : undefined,
- );
- expect(labels).toContain('Query');
- expect(labels).toContain('Chart Options');
- });
-
- test('Metric args produce a [metric] row in Query', () => {
- const args: GlyphArguments = new Map([['m', Metric]]);
- const cp = generateControlPanel(args);
- const querySection = cp.controlPanelSections.find(
- s => s && 'label' in s && s.label === 'Query',
- );
- expect(querySection).toBeDefined();
- const rows = (querySection as { controlSetRows: unknown[][] })
- .controlSetRows;
- expect(rows).toContainEqual(['metric']);
- });
-
- test('Dimension args produce a [groupby] row in Query', () => {
- const args: GlyphArguments = new Map([['d', Dimension]]);
- const cp = generateControlPanel(args);
- const querySection = cp.controlPanelSections.find(
- s => s && 'label' in s && s.label === 'Query',
- );
- const rows = (querySection as { controlSetRows: unknown[][] })
- .controlSetRows;
- expect(rows).toContainEqual(['groupby']);
- });
-
- test('Temporal args produce [x_axis] and [time_grain_sqla] rows in Query', () => {
- const args: GlyphArguments = new Map([['t', Temporal]]);
- const cp = generateControlPanel(args);
- const querySection = cp.controlPanelSections.find(
- s => s && 'label' in s && s.label === 'Query',
- );
- const rows = (querySection as { controlSetRows: unknown[][] })
- .controlSetRows;
- expect(rows).toContainEqual(['x_axis']);
- expect(rows).toContainEqual(['time_grain_sqla']);
- });
-
- test('adhoc_filters is always added to Query', () => {
- const args: GlyphArguments = new Map();
- const cp = generateControlPanel(args);
- const querySection = cp.controlPanelSections.find(
- s => s && 'label' in s && s.label === 'Query',
- );
- const rows = (querySection as { controlSetRows: unknown[][] })
- .controlSetRows;
- expect(rows).toContainEqual(['adhoc_filters']);
- });
-
- test('Non-data args become controls in Chart Options', () => {
- const args: GlyphArguments = new Map([
- ['showLegend', Checkbox.with({ label: 'Legend', default: true })],
- ]);
- const cp = generateControlPanel(args);
- const chartOpts = cp.controlPanelSections.find(
- s => s && 'label' in s && s.label === 'Chart Options',
- );
- const rows = (chartOpts as { controlSetRows: unknown[][] }).controlSetRows;
- expect(rows).toHaveLength(1);
- expect(rows[0]).toEqual([
- {
- name: 'showLegend',
- config: expect.objectContaining({ type: 'CheckboxControl' }),
- },
- ]);
- });
-
- test('GlyphArgConfig with visibility wires onto control', () => {
- const visibility = jest.fn(() => true);
- const args: GlyphArguments = new Map([
- [
- 'subtitleSize',
- {
- arg: Select.with({
- label: 'Size',
- default: 'm',
- options: [{ label: 'M', value: 'm' }],
- }),
- visibility,
- resetOnHide: true,
- },
- ],
- ]);
- const cp = generateControlPanel(args);
- const chartOpts = cp.controlPanelSections.find(
- s => s && 'label' in s && s.label === 'Chart Options',
- );
- const row = (chartOpts as { controlSetRows: unknown[][] })
- .controlSetRows[0];
- const item = (row as Array<{ config: Record<string, unknown> }>)[0];
- expect(item.config.visibility).toBe(visibility);
- expect(item.config.resetOnHide).toBe(true);
- });
-
- test('controlOverrides and formDataOverrides options pass through', () => {
- const fdo = (fd: Record<string, unknown>) => ({ ...fd, x: 1 });
- const cp = generateControlPanel(new Map(), {
- controlOverrides: { metric: { label: 'M' } },
- formDataOverrides: fdo,
- });
- expect(cp.controlOverrides).toEqual({ metric: { label: 'M' } });
- expect(cp.formDataOverrides).toBe(fdo);
- });
-
- test('extra queryControls and chartOptionsControls are appended', () => {
- const args: GlyphArguments = new Map();
- const cp = generateControlPanel(args, {
- queryControls: [['custom_filter']] as never,
- chartOptionsControls: [['custom_chart_opt']] as never,
- });
- const querySection = cp.controlPanelSections.find(
- s => s && 'label' in s && s.label === 'Query',
- );
- const queryRows = (querySection as { controlSetRows: unknown[][] })
- .controlSetRows;
- expect(queryRows).toContainEqual(['custom_filter']);
-
- const chartOpts = cp.controlPanelSections.find(
- s => s && 'label' in s && s.label === 'Chart Options',
- );
- const optRows = (chartOpts as { controlSetRows: unknown[][] })
- .controlSetRows;
- expect(optRows).toContainEqual(['custom_chart_opt']);
- });
-});
-
-describe('generateTransformProps', () => {
- function makeChartProps(formData: Record<string, unknown>): ChartProps {
- return {
- width: 400,
- height: 300,
- queriesData: [{ data: [] }],
- formData,
- } as unknown as ChartProps;
- }
-
- test('returns width/height/queriesData passthrough', () => {
- const transform = generateTransformProps(new Map());
- const out = transform(makeChartProps({}));
- expect(out).toMatchObject({ width: 400, height: 300 });
- });
-
- test('extracts Select value from formData', () => {
- const args: GlyphArguments = new Map([
- [
- 'size',
- Select.with({
- label: 'Size',
- default: 'm',
- options: [
- { label: 'S', value: 's' },
- { label: 'M', value: 'm' },
- ],
- }),
- ],
- ]);
- const transform = generateTransformProps(args);
- const out = transform(makeChartProps({ size: 's' }));
- expect((out as { size: unknown }).size).toBe('s');
- });
-
- test('Select falls back to default when value missing', () => {
- const args: GlyphArguments = new Map([
- ['size', Select.with({ label: 'Size', default: 'm', options: [] })],
- ]);
- const transform = generateTransformProps(args);
- const out = transform(makeChartProps({}));
- expect((out as { size: unknown }).size).toBe('m');
- });
-
- test('Checkbox uses formData value when present', () => {
- const args: GlyphArguments = new Map([
- ['flag', Checkbox.with({ label: 'F', default: false })],
- ]);
- const transform = generateTransformProps(args);
- const out = transform(makeChartProps({ flag: true }));
- expect((out as { flag: unknown }).flag).toBe(true);
- });
-
- test('Checkbox falls back to default when value missing', () => {
- const args: GlyphArguments = new Map([
- ['flag', Checkbox.with({ label: 'F', default: true })],
- ]);
- const transform = generateTransformProps(args);
- const out = transform(makeChartProps({}));
- expect((out as { flag: unknown }).flag).toBe(true);
- });
-
- test('Color: RGBA in formData → hex string', () => {
- const args: GlyphArguments = new Map([
- ['fill', Color.with({ label: 'Fill', default: '#000000' })],
- ]);
- const transform = generateTransformProps(args);
- const out = transform(
- makeChartProps({ fill: { r: 255, g: 0, b: 0, a: 1 } }),
- );
- expect((out as { fill: unknown }).fill).toBe('#ff0000');
- });
-
- test('Color: string value in formData passes through', () => {
- const args: GlyphArguments = new Map([
- ['fill', Color.with({ label: 'Fill', default: '#000000' })],
- ]);
- const transform = generateTransformProps(args);
- const out = transform(makeChartProps({ fill: '#abcdef' }));
- expect((out as { fill: unknown }).fill).toBe('#abcdef');
- });
-
- test('Color: falls back to class default when value missing', () => {
- const args: GlyphArguments = new Map([
- ['fill', Color.with({ label: 'Fill', default: '#112233' })],
- ]);
- const transform = generateTransformProps(args);
- const out = transform(makeChartProps({}));
- // default is hex string; transform converts the RGBA-formatted default back to hex
- expect((out as { fill: unknown }).fill).toBe('#112233');
- });
-
- test('Int uses default when value missing', () => {
- const args: GlyphArguments = new Map([
- ['n', Int.with({ label: 'N', default: 7, min: 0, max: 100 })],
- ]);
- const transform = generateTransformProps(args);
- const out = transform(makeChartProps({}));
- expect((out as { n: unknown }).n).toBe(7);
- });
-
- test('Text uses default when value missing', () => {
- const args: GlyphArguments = new Map([
- ['s', Text.with({ label: 'S', default: 'hi' })],
- ]);
- const transform = generateTransformProps(args);
- const out = transform(makeChartProps({}));
- expect((out as { s: unknown }).s).toBe('hi');
- });
-
- test('Metric/Dimension/Temporal args are NOT extracted (handled elsewhere)', () => {
- const args: GlyphArguments = new Map([
- ['metric', Metric],
- ['groupby', Dimension],
- ['t', Temporal],
- ]);
- const transform = generateTransformProps(args);
- const out = transform(
- makeChartProps({ metric: 'count', groupby: 'a', t: 'date' }),
- );
- expect((out as Record<string, unknown>).metric).toBeUndefined();
- expect((out as Record<string, unknown>).groupby).toBeUndefined();
- expect((out as Record<string, unknown>).t).toBeUndefined();
- });
-
- test('passthrough option copies named ChartProps fields onto the result', () => {
- const args: GlyphArguments = new Map();
- const transform = generateTransformProps(args, {
- passthrough: ['formData'],
- });
- const out = transform(makeChartProps({ marker: 1 })) as Record<
- string,
- unknown
- >;
- expect(out.formData).toEqual({ marker: 1 });
- });
-
- test('custom transform option receives extracted values and chartProps', () => {
- const args: GlyphArguments = new Map([
- ['flag', Checkbox.with({ label: 'F', default: false })],
- ]);
- const transform = generateTransformProps(args, {
- transform: (values, chartProps) => ({
- values,
- gotChartProps: !!chartProps,
- }),
- });
- const out = transform(makeChartProps({ flag: true })) as {
- values: { flag: boolean };
- gotChartProps: boolean;
- };
- expect(out.values.flag).toBe(true);
- expect(out.gotChartProps).toBe(true);
- });
-});
-
-describe('createGlyphPlugin', () => {
- test('returns both controlPanel and transformProps', () => {
- const args = glyphMap([
- ['metric', Metric],
- ['show', Checkbox.with({ label: 'S', default: true })],
- ]);
- const plugin = createGlyphPlugin(args);
- expect(plugin.controlPanel).toBeDefined();
- expect(plugin.controlPanel.controlPanelSections.length).toBe(2);
- expect(typeof plugin.transformProps).toBe('function');
- });
-});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/index.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/index.tsx
index 55ef834634d..f2254a89bcc 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/index.tsx
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/index.tsx
@@ -633,10 +633,11 @@ export default defineChart<
},
{
offset: 1,
- // eslint-disable-next-line theme-colors/no-literal-colors
+ /* eslint-disable theme-colors/no-literal-colors */
color:
(theme as { colorBgContainer?: string })
?.colorBgContainer ?? '#fff',
+ /* eslint-enable theme-colors/no-literal-colors */
},
]),
},
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/index.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/index.tsx
index 02d4169c517..d4329147fbd 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/index.tsx
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/index.tsx
@@ -481,11 +481,11 @@ export default defineChart<
const defaultLabel = {
formatter,
show: showLabels,
- // eslint-disable-next-line theme-colors/no-literal-colors
+ /* eslint-disable theme-colors/no-literal-colors */
color: (theme as { colorText?: string })?.colorText ?? '#000',
- // eslint-disable-next-line theme-colors/no-literal-colors
textBorderColor:
(theme as { colorBgBase?: string })?.colorBgBase ?? '#fff',
+ /* eslint-enable theme-colors/no-literal-colors */
textBorderWidth: 1,
};
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/index.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/index.tsx
index 514f789c950..42d358de89f 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/index.tsx
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/index.tsx
@@ -427,9 +427,11 @@ export default defineChart<
`${index * titleOffsetFromTitle + OFFSETS.titleFromCenter}%`,
],
fontSize,
+ /* eslint-disable theme-colors/no-literal-colors */
color:
(theme as { colorTextSecondary?: string })?.colorTextSecondary ??
'#666',
+ /* eslint-enable theme-colors/no-literal-colors */
},
detail: {
offsetCenter: [
@@ -441,6 +443,7 @@ export default defineChart<
}%`,
],
fontSize: FONT_SIZE_MULTIPLIERS.detailFontSize * fontSize,
+ // eslint-disable-next-line theme-colors/no-literal-colors
color: (theme as { colorText?: string })?.colorText ?? '#000',
},
};
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/index.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/index.tsx
index 5fcde678ea7..7e9521e7ddd 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/index.tsx
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/index.tsx
@@ -1397,6 +1397,8 @@ export default defineChart({
AnnotationType.Interval,
AnnotationType.Timeseries,
],
+ // Two queries (A and B) — drives the Results tab count in the Data panel
+ queryObjectCount: 2,
},
arguments: {},
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/index.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/index.tsx
index 1818a139a01..10350cbb9cc 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/index.tsx
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/index.tsx
@@ -688,10 +688,11 @@ export default defineChart<
label: {
show: true,
fontWeight: 'bold',
- // eslint-disable-next-line theme-colors/no-literal-colors
+ /* eslint-disable theme-colors/no-literal-colors */
backgroundColor:
(theme as { colorBgContainer?: string })?.colorBgContainer ??
'#fff',
+ /* eslint-enable theme-colors/no-literal-colors */
},
},
data: transformedData,
diff --git a/superset-frontend/plugins/plugin-chart-point-cluster-map/src/index.tsx b/superset-frontend/plugins/plugin-chart-point-cluster-map/src/index.tsx
index de7d2cb9690..4f4e6e58542 100644
--- a/superset-frontend/plugins/plugin-chart-point-cluster-map/src/index.tsx
+++ b/superset-frontend/plugins/plugin-chart-point-cluster-map/src/index.tsx
@@ -386,7 +386,7 @@ export function transformProps(chartProps: ChartProps) {
const columnsConfig = sharedControls.entity;
-// eslint-disable-next-line theme-colors/no-literal-colors
+/* eslint-disable theme-colors/no-literal-colors */
const colorChoices = [
['#008b8b', t('Dark Cyan')],
['#800080', t('Purple')],
@@ -395,6 +395,7 @@ const colorChoices = [
['#dc143c', t('Crimson')],
['#228b22', t('Forest Green')],
];
+/* eslint-enable theme-colors/no-literal-colors */
// ─── Plugin definition ───────────────────────────────────────────────────────
diff --git a/superset-frontend/scripts/check-custom-rules.js b/superset-frontend/scripts/check-custom-rules.js
index 5cb288f398c..ec8ba9e8d4d 100755
--- a/superset-frontend/scripts/check-custom-rules.js
+++ b/superset-frontend/scripts/check-custom-rules.js
@@ -38,49 +38,46 @@ let errorCount = 0;
let warningCount = 0;
/**
- * Check if a node has an eslint-disable comment
+ * Build a line-based suppression index from a file's comments, mirroring real
+ * ESLint semantics:
+ * - `eslint-disable-next-line <rule>` covers only the line directly below it
+ * - `eslint-disable-line <rule>` covers only its own line
+ * - block-form `eslint-disable <rule>` covers every line until the matching
+ * `eslint-enable <rule>` (or end of file)
+ * Line-based (not AST-attachment-based) scoping means one disable comment on a
+ * large declaration cannot silence color literals nested hundreds of lines
+ * deeper inside it.
*/
-function hasEslintDisable(path, ruleName = 'theme-colors/no-literal-colors') {
- const { node, parent } = path;
-
- // Check leadingComments on the node itself
- if (node.leadingComments) {
- const hasDisable = node.leadingComments.some(
- comment =>
- (comment.value.includes('eslint-disable-next-line') ||
- comment.value.includes('eslint-disable')) &&
- comment.value.includes(ruleName),
- );
- if (hasDisable) return true;
- }
-
- // Check leadingComments on parent nodes (for expressions in assignments, etc.)
- if (parent && parent.leadingComments) {
- const hasDisable = parent.leadingComments.some(
- comment =>
- (comment.value.includes('eslint-disable-next-line') ||
- comment.value.includes('eslint-disable')) &&
- comment.value.includes(ruleName),
- );
- if (hasDisable) return true;
- }
-
- // Walk up the ancestor chain using parentPath (NodePath, not raw node)
- let current = path.parentPath;
- while (current) {
- if (current.node && current.node.leadingComments) {
- const hasDisable = current.node.leadingComments.some(
- comment =>
- (comment.value.includes('eslint-disable-next-line') ||
- comment.value.includes('eslint-disable')) &&
- comment.value.includes(ruleName),
- );
- if (hasDisable) return true;
+function buildDisabledLineIndex(
+ comments,
+ ruleName = 'theme-colors/no-literal-colors',
+) {
+ const singleLines = new Set();
+ const ranges = [];
+ let openRangeStart = null;
+
+ (comments || []).forEach(comment => {
+ if (!comment.value.includes(ruleName)) return;
+ if (comment.value.includes('eslint-disable-next-line')) {
+ singleLines.add(comment.loc.end.line + 1);
+ } else if (comment.value.includes('eslint-disable-line')) {
+ singleLines.add(comment.loc.start.line);
+ } else if (comment.value.includes('eslint-enable')) {
+ if (openRangeStart !== null) {
+ ranges.push([openRangeStart, comment.loc.start.line]);
+ openRangeStart = null;
+ }
+ } else if (comment.value.includes('eslint-disable')) {
+ openRangeStart = comment.loc.start.line;
}
- current = current.parentPath;
+ });
+ if (openRangeStart !== null) {
+ ranges.push([openRangeStart, Infinity]);
}
- return false;
+ return line =>
+ singleLines.has(line) ||
+ ranges.some(([start, end]) => line >= start && line <= end);
}
/**
@@ -93,12 +90,14 @@ function checkNoLiteralColors(ast, filepath) {
/^rgba\(/, // RGBA colors
];
+ const isLineDisabled = buildDisabledLineIndex(ast.comments);
+
traverse(ast, {
StringLiteral(path) {
const { value } = path.node;
if (colorPatterns.some(pattern => pattern.test(value))) {
// Check if this line has an eslint-disable comment
- if (hasEslintDisable(path)) {
+ if (isLineDisabled(path.node.loc.start.line)) {
return; // Skip this violation
}
@@ -120,7 +119,7 @@ function checkNoLiteralColors(ast, filepath) {
)
) {
// Check if this line has an eslint-disable comment
- if (hasEslintDisable(path)) {
+ if (isLineDisabled(path.node.loc.start.line)) {
return; // Skip this violation
}
@@ -470,6 +469,11 @@ function isWrappedInTranslation(node) {
* Check for untranslated user-facing strings
*/
function checkUntranslatedStrings(ast, filepath) {
+ const isLineDisabled = buildDisabledLineIndex(
+ ast.comments,
+ 'i18n/no-untranslated-string',
+ );
+
traverse(ast, {
// Check JSX attributes for untranslated strings
JSXAttribute(path) {
@@ -498,7 +502,7 @@ function checkUntranslatedStrings(ast, filepath) {
// String literal value
if (value && value.type === 'StringLiteral') {
if (needsTranslation(value.value)) {
- if (hasEslintDisable(path, 'i18n/no-untranslated-string')) {
+ if (isLineDisabled(path.node.loc.start.line)) {
return;
}
// eslint-disable-next-line no-console
@@ -517,7 +521,7 @@ function checkUntranslatedStrings(ast, filepath) {
needsTranslation(expression.value)
) {
if (!isWrappedInTranslation(value)) {
- if (hasEslintDisable(path, 'i18n/no-untranslated-string')) {
+ if (isLineDisabled(path.node.loc.start.line)) {
return;
}
// eslint-disable-next-line no-console
@@ -535,7 +539,7 @@ function checkUntranslatedStrings(ast, filepath) {
const text = path.node.value.trim();
if (needsTranslation(text)) {
- if (hasEslintDisable(path, 'i18n/no-untranslated-string')) {
+ if (isLineDisabled(path.node.loc.start.line)) {
return;
}
// eslint-disable-next-line no-console
diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
index a1dbacd4789..a1df8917bfe 100644
--- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
+++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
@@ -940,17 +940,19 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
} | null;
const glyphArgs = currentConfig?._glyphArgs as ChartArguments | undefined;
- const CHART_OPTIONS_LABEL = t('Chart Options');
- // The auto-generated Chart Options section (rendered by GlyphOptionsPanel when present)
+ // The auto-generated Chart Options section (rendered by GlyphOptionsPanel when
+ // present). Identified by the structural _glyphChartOptions marker stamped by
+ // generateControlPanel — never by label, so hand-written sections that happen
+ // to be named "Chart Options" are not hijacked.
const glyphChartOptionsSection = glyphArgs
- ? (customizeSections.find(s => s.label === CHART_OPTIONS_LABEL) as
+ ? (customizeSections.find(s => s._glyphChartOptions) as
| ExpandedControlPanelSectionConfig
| undefined)
: undefined;
// Sections in the Customize tab other than the Chart Options one
// (e.g. additionalSections with custom tabOverrides, Time Comparison, etc.)
const remainingCustomizeSections = glyphArgs
- ? customizeSections.filter(s => s.label !== CHART_OPTIONS_LABEL)
+ ? customizeSections.filter(s => !s._glyphChartOptions)
: customizeSections;
return (
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.tsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.tsx
index d476baf5a44..a23ccc50edf 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer/index.tsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.tsx
@@ -23,6 +23,7 @@ import {
useCallback,
useEffect,
useMemo,
+ useRef,
useState,
} from 'react';
import { bindActionCreators, Dispatch } from 'redux';
@@ -579,17 +580,19 @@ function ExploreViewContainer(props: ExploreViewContainerProps) {
}
}, [isDynamicPluginLoading]);
- // Track if we've already triggered initial query
- const [hasTriggeredInitialQuery, setHasTriggeredInitialQuery] =
- useState(false);
+ // Track if the initial-query decision has been made (a ref: no extra render)
+ const hasEvaluatedInitialQuery = useRef(false);
- // Auto-trigger query when there are no validation errors
- // This effect runs on mount and when controls change (e.g., after dynamic plugin loads)
+ // Trigger the initial query once, as soon as dynamic plugin loading (if any)
+ // has settled. This is a one-shot evaluation: if the chart opens with
+ // validation errors, no query fires — then or later. Auto-running a query
+ // when the user later fixes the last invalid control would bypass the
+ // explicit "Create chart" flow and fire unrequested queries.
useEffect(() => {
- // Skip if already triggered or still loading dynamic plugin
- if (hasTriggeredInitialQuery || isDynamicPluginLoading) {
+ if (hasEvaluatedInitialQuery.current || isDynamicPluginLoading) {
return;
}
+ hasEvaluatedInitialQuery.current = true;
const hasError = Object.values(props.controls).some(
control =>
@@ -597,9 +600,8 @@ function ExploreViewContainer(props: ExploreViewContainerProps) {
);
if (!hasError) {
props.actions.triggerQuery(true, props.chart.id);
- setHasTriggeredInitialQuery(true);
}
- }, [props.controls, isDynamicPluginLoading, hasTriggeredInitialQuery]);
+ }, [props.controls, isDynamicPluginLoading]);
const reRenderChart = useCallback(
(controlsChanged?: string[]) => {
diff --git a/superset-frontend/src/explore/components/GlyphOptionsPanel.tsx b/superset-frontend/src/explore/components/GlyphOptionsPanel.tsx
index 69cf5053daf..cbb000576fa 100644
--- a/superset-frontend/src/explore/components/GlyphOptionsPanel.tsx
+++ b/superset-frontend/src/explore/components/GlyphOptionsPanel.tsx
@@ -36,7 +36,13 @@
* correctly while glyph-defined args get the simplified formData-based path.
*/
-import { ReactNode } from 'react';
+import {
+ cloneElement,
+ isValidElement,
+ ReactElement,
+ ReactNode,
+ useMemo,
+} from 'react';
import { t } from '@apache-superset/core/translation';
import { JsonValue, QueryFormData } from '@superset-ui/core';
import {
@@ -81,6 +87,8 @@ interface GlyphOptionsPanelProps {
defaultExpandedKeys?: string[];
}
+const EMPTY_ERRORS: unknown[] = [];
+
/**
* Render a single glyph arg control natively (value + visibility from formData).
*/
@@ -98,19 +106,26 @@ function GlyphArgControl({
actions: Pick<ExploreActions, 'setControlValue'>;
}) {
const formDataRecord = formData as Record<string, unknown>;
- const argClass = resolveArgClass(argDef);
- const visibleWhen = getArgVisibleWhen(argDef);
- const controlConfig = getGlyphControlConfig(argClass, name) as Record<
- string,
- unknown
- > & { type: string };
+ // The config is deterministic per (argDef, name) — don't rebuild it (and its
+ // nested option arrays) on every keystroke-driven re-render.
+ const { visibleWhen, controlConfig } = useMemo(
+ () => ({
+ visibleWhen: getArgVisibleWhen(argDef),
+ controlConfig: getGlyphControlConfig(
+ resolveArgClass(argDef),
+ name,
+ ) as Record<string, unknown> & { type: string },
+ }),
+ [argDef, name],
+ );
const isVisible = visibleWhen
? evaluateGlyphCondition(visibleWhen, formDataRecord)
: undefined;
const { type, label, description, ...restConfig } = controlConfig;
- const validationErrors = controls[name]?.validationErrors ?? [];
+ // Stable empty-array reference so PureComponent controls can bail out
+ const validationErrors = controls[name]?.validationErrors ?? EMPTY_ERRORS;
return (
<StashFormDataContainer
@@ -148,17 +163,21 @@ export default function GlyphOptionsPanel({
defaultExpandedKeys,
}: GlyphOptionsPanelProps) {
// Build the set of arg names that are data args (go in the Query/Data tab, not here)
- const dataArgNames = new Set(
- Object.entries(glyphArgs)
- .filter(([, argDef]) => {
- const argClass = resolveArgClass(argDef as ArgDef);
- return (
- isMetricArg(argClass) ||
- isDimensionArg(argClass) ||
- isTemporalArg(argClass)
- );
- })
- .map(([name]) => name),
+ const dataArgNames = useMemo(
+ () =>
+ new Set(
+ Object.entries(glyphArgs)
+ .filter(([, argDef]) => {
+ const argClass = resolveArgClass(argDef as ArgDef);
+ return (
+ isMetricArg(argClass) ||
+ isDimensionArg(argClass) ||
+ isTemporalArg(argClass)
+ );
+ })
+ .map(([name]) => name),
+ ),
+ [glyphArgs],
);
const rows = chartOptionsSection.controlSetRows
@@ -167,11 +186,42 @@ export default function GlyphOptionsPanel({
.map(item => {
if (!item) return null;
+ // JSX rows (sub-section headers, dividers) — render them like the
+ // legacy section renderer does instead of silently dropping them.
+ if (isValidElement(item)) {
+ const element = item as ReactElement<Record<string, unknown>>;
+ const controlName = (element.props as { name?: string }).name;
+ if (!controlName) {
+ return element;
+ }
+ const controlState = controls[controlName];
+ return cloneElement(element, {
+ ...(element.props as Record<string, unknown>),
+ actions,
+ controls,
+ form_data: formData,
+ ...(controlState && {
+ value: controlState.value,
+ validationErrors: controlState.validationErrors,
+ default: controlState.default,
+ onChange: (value: unknown, errors: unknown[]) =>
+ actions.setControlValue(controlName, value, errors),
+ }),
+ });
+ }
+
if (isCustomControlItem(item)) {
const { name } = item;
const argDef = glyphArgs[name] as ArgDef | undefined;
- if (argDef && !dataArgNames.has(name)) {
+ // Controls whose config needs mapStateToProps (e.g. conditional
+ // formatting deriving column options from the chart response, or
+ // disabledWhen) must go through the legacy renderControl path —
+ // the native path has no exploreState/chart to map from.
+ const needsStateMapping =
+ item.config && 'mapStateToProps' in item.config;
+
+ if (argDef && !dataArgNames.has(name) && !needsStateMapping) {
// Native glyph rendering: value from formData, visibility from formData
return (
<GlyphArgControl
diff --git a/superset-frontend/src/visualizations/presets/MainPreset.ts b/superset-frontend/src/visualizations/presets/MainPreset.ts
index b1a9fc22022..f5f3b84834c 100644
--- a/superset-frontend/src/visualizations/presets/MainPreset.ts
+++ b/superset-frontend/src/visualizations/presets/MainPreset.ts
@@ -44,7 +44,7 @@ import ScatterMapChartPlugin from '@superset-ui/plugin-chart-point-cluster-map';
import { createCartodiagramPlugin } from '@superset-ui/plugin-chart-cartodiagram';
import {
BigNumberChartPlugin,
- BigNumberGlyphChartPlugin,
+ BigNumberTotalChartPlugin,
EchartsPieChartPlugin,
EchartsBoxPlotChartPlugin,
EchartsAreaChartPlugin,
@@ -110,7 +110,7 @@ export default class MainPreset extends Preset {
presets: [new DeckGLChartPreset()],
plugins: [
new BigNumberChartPlugin().configure({ key: VizType.BigNumber }),
- new BigNumberGlyphChartPlugin().configure({
+ new BigNumberTotalChartPlugin().configure({
key: VizType.BigNumberTotal,
}),
new EchartsBoxPlotChartPlugin().configure({ key: VizType.BoxPlot }),
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 021507c03a4..7612d5f73fd 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -3029,6 +3029,13 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
if not db_engine_spec.allows_hidden_orderby_agg:
select_exprs = remove_duplicates(select_exprs + orderby_exprs)
+ # An empty SELECT list would compile to invalid SQL. The allow_empty_query
+ # bypass above is only safe for callers that short-circuit before building
+ # SQL (ExploreMixin.query); any other path reaching this point with no
+ # select expressions must still fail validation cleanly.
+ if not select_exprs:
+ raise QueryObjectValidationError(_("Empty query?"))
+
qry = sa.select(select_exprs)
if groupby_all_columns: