Skip to content

Commit fbf2126

Browse files
committed
fix(bullet): tolerate partial input, place markers below the bar, tooltip range labels
Tokenizing is lenient instead of throwing: the controls re-render on every keystroke, so partial input like a trailing comma must not error the chart (or leave it stuck in the error state). Empty and non-numeric tokens are dropped. The marker offset is computed in pixels from the chart height since a percentage offset of the symbol size lands inside a tall measure bar. Range labels, which markArea labels never rendered, now surface as lines on the measure bar's tooltip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7021004 commit fbf2126

3 files changed

Lines changed: 38 additions & 22 deletions

File tree

‎superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,7 @@ export default function transformProps(
8585
itemStyle: {
8686
color: bandFills[Math.min(i, bandFills.length - 1)],
8787
},
88-
// Surface the range label inside the band's right edge
89-
label: {
90-
show: Boolean(label),
91-
position: 'insideRight',
92-
color: theme.colorTextSecondary,
93-
fontSize: theme.fontSizeSM,
94-
},
88+
label: { show: false },
9589
// markArea item: [{start}, {end}]
9690
coords: [{ xAxis: axisMin }, { xAxis: value }],
9791
}));
@@ -102,6 +96,21 @@ export default function transformProps(
10296
const markerLabelAt = (index: number, labels: string[], values: number[]) =>
10397
labels[index] ? labels[index] : formatter(values[index]);
10498

99+
// The measure bar spans MEASURE_BAR_FRACTION of the category band
100+
// (roughly the grid height), so a percentage symbolOffset lands inside a
101+
// tall bar. Compute pixels: half the bar plus a small gap below it.
102+
const gridHeight = Math.max(height - theme.sizeUnit * 10, 40);
103+
const markerOffsetPx = Math.round(
104+
(MEASURE_BAR_FRACTION / 2) * gridHeight + 12,
105+
);
106+
107+
// Surface range labels on the measure bar's tooltip
108+
const rangeTooltipLines = [...ranges]
109+
.map((value, i) => ({ value, label: rangeLabels[i] }))
110+
.filter(({ label }) => Boolean(label))
111+
.sort((a, b) => a.value - b.value)
112+
.map(({ value, label }) => `${label}: \u2264 ${formatter(value)}`);
113+
105114
const echartOptions: EChartsCoreOption = {
106115
// A single-measure chart needs no legend; the default one collides
107116
// with the x-axis labels.
@@ -133,7 +142,12 @@ export default function transformProps(
133142
tooltip: {
134143
confine: true,
135144
formatter: () =>
136-
sanitizeHtml(`${metricLabel}: <b>${formatter(measure)}</b>`),
145+
sanitizeHtml(
146+
[
147+
`${metricLabel}: <b>${formatter(measure)}</b>`,
148+
...rangeTooltipLines,
149+
].join('<br />'),
150+
),
137151
},
138152
series: [
139153
{
@@ -191,7 +205,7 @@ export default function transformProps(
191205
symbol: 'triangle',
192206
symbolSize: 14,
193207
// Sit below the measure bar pointing up at the marked value
194-
symbolOffset: [0, '160%'],
208+
symbolOffset: [0, markerOffsetPx],
195209
itemStyle: { color: theme.colorText },
196210
z: 20,
197211
},

‎superset-frontend/plugins/plugin-chart-echarts/src/Bullet/utils.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,19 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19-
import { validateNumber } from '@superset-ui/core';
20-
2119
// Ported from the nvd3 bullet chart so saved comma-separated control values
2220
// keep their exact semantics.
2321
export function tokenizeToNumericArray(value?: string): number[] | null {
2422
if (!value?.trim()) return null;
25-
const tokens = value.split(',');
26-
if (tokens.some(token => validateNumber(token)))
27-
throw new Error('All values should be numeric');
28-
return tokens.map(token => parseFloat(token));
23+
// Lenient by design: this runs on every keystroke, so partial input like
24+
// "50," must not throw. Empty and non-numeric tokens are dropped.
25+
const numbers = value
26+
.split(',')
27+
.map(token => token.trim())
28+
.filter(token => token !== '')
29+
.map(token => Number(token))
30+
.filter(n => !Number.isNaN(n));
31+
return numbers.length ? numbers : null;
2932
}
3033

3134
export function tokenizeToStringArray(value?: string): string[] | null {

‎superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,14 @@ test('renders the measure bar with markers, marker lines and range bands', () =>
6666
// axis spans all values
6767
expect((echartOptions as any).xAxis.max).toBe(300);
6868

69-
// markers sit below the bar; no legend on a single-measure chart
70-
expect(series[1].symbolOffset).toEqual([0, '160%']);
69+
// markers sit below the bar (pixel offset); no legend on this chart
70+
expect(series[1].symbolOffset[1]).toBeGreaterThan(0);
7171
expect((echartOptions as any).legend.show).toBe(false);
7272

73-
// range labels surface inside their bands
74-
const labeled = series[0].markArea.data.find(
75-
(d: any) => d[0].name === 'high',
76-
);
77-
expect(labeled[0].label.show).toBe(true);
73+
// range labels surface in the measure tooltip
74+
const tooltip = (echartOptions as any).tooltip.formatter();
75+
expect(tooltip).toContain('low');
76+
expect(tooltip).toContain('high');
7877
});
7978

8079
test('defaults the band to 110% of the measure when no ranges are set', () => {

0 commit comments

Comments
 (0)