sfirke commented on code in PR #45505:
URL: https://github.com/apache/superset/pull/45505#discussion_r4351097151
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -3311,9 +3311,9 @@ describe('EchartsTimeseries tooltip truncation', () => {
expect(html).not.toContain(longSeriesName);
});
- test('preserves the echarts marker in every mode', () => {
+ test('keeps the series color marker in every mode', () => {
(['off', 'end', 'start', 'middle'] as const).forEach(mode => {
- expect(buildTooltip(mode)).toContain('background-color:#1f77b4');
+ expect(buildTooltip(mode).replace(/\s/g, '')).toContain('color:#1f77b4');
Review Comment:
This assertion got looser than it looks. The old ECharts marker was
`<span style="background-color:#1f77b4;"></span>`, and after the
whitespace strip that string still `.includes('color:#1f77b4')`. So if
`value.marker` ever leaks back into the row (which is exactly the regression
this PR fixes), this test keeps passing.
Matching on something only `tooltipMarker` emits closes the gap, e.g.
```ts
expect(buildTooltip(mode).replace(/\s/g,
'')).toContain('style="color:#1f77b4;');
```
Same substring issue in `MixedTimeseries/transformProps.test.ts` (~L1617) and
the new Histogram test.
---
🤖 _Drafted by Claude Code, reviewed and approved by @sfirke._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
