EnxDev commented on code in PR #45505:
URL: https://github.com/apache/superset/pull/45505#discussion_r4328527567
##########
superset-frontend/plugins/plugin-chart-echarts/src/Histogram/transformProps.ts:
##########
@@ -123,8 +124,11 @@ export default function transformProps(
const tooltipFormatter = (params: CallbackDataParams[]) => {
const title = params[0].name;
const rows = params.map(param => {
- const { marker, seriesName, value } = param;
- return [`${marker}${seriesName}`, yAxisFormatter.format(value as
number)];
+ const { color, seriesName, value } = param;
+ return [
+ `${tooltipMarker(color)}${seriesName}`,
Review Comment:
This is the only call site with no test behind it (codecov flags these two
lines), and the Histogram is one of the three charts the issue reports.
Worth a small test that calls `echartOptions.tooltip.formatter` with a param
carrying `color` and checks `color:` makes it into the output, same as the
Timeseries truncation tests do.
##########
superset-frontend/packages/superset-ui-core/test/utils/tooltip.test.ts:
##########
@@ -272,3 +273,30 @@ describe('tooltipHtml truncation modes', () => {
});
});
});
+
+describe('tooltipMarker', () => {
+ test('keeps its color and shape through sanitizeHtml', () => {
+ const html = removeWhitespaces(
+ sanitizeHtml(tooltipMarker('#1f77b4', 'pin')),
+ );
+ expect(html).toContain('color:#1f77b4');
+ expect(html).toContain('border:5pxsolidcurrentcolor');
+ expect(html).toContain('border-radius:50%50%050%');
+ });
+
+ test('draws the triangle with borders and the diamond as a glyph', () => {
+ expect(tooltipMarker('#1f77b4', 'triangle')).toContain(
Review Comment:
Only the pin goes through `sanitizeHtml` here. The triangle relies on
`vertical-align:calc(...)` and the transparent `border-color` surviving it, and
that's the part most likely to break if someone tightens value validation in
the allowlist later.
Could we wrap this one in `removeWhitespaces(sanitizeHtml(...))` like the
pin test and assert on the `vertical-align` and `border-color` too?
--
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]
