rusackas commented on code in PR #45585:
URL: https://github.com/apache/superset/pull/45585#discussion_r4356310398
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Scatter/transformProps.ts:
##########
@@ -155,9 +156,10 @@ function processScatterData(
}
}
- const categoryValue = getSourceValue(categoryColumn);
- if (categoryValue != null) {
- scatterPoint.cat_color = String(categoryValue);
+ if (categoryColumn) {
+ const categoryValue = getSourceValue(categoryColumn);
+ scatterPoint.cat_color =
Review Comment:
Good catch. Scatter and Path now assign `cat_color` after the property copy,
same as Arc, so a `cat_color` column can no longer clobber it (864f1c849f).
##########
superset-frontend/plugins/preset-chart-deckgl/src/utilities/tooltipUtils.tsx:
##########
@@ -58,7 +59,7 @@ export const CommonTooltipRows = {
o.object?.cat_color ? (
<TooltipRow
label={`${t('Category')}: `}
- value={`${o.object.cat_color}`}
+ value={formatNullCategory(`${o.object.cat_color}`)}
Review Comment:
Yep, fair. `createHandlebarsTooltipData` now runs `cat_color` through
`formatNullCategory`, so templates show N/A to match the legend (864f1c849f).
##########
superset-frontend/plugins/preset-chart-deckgl/src/CategoricalDeckGLContainer.test.tsx:
##########
@@ -275,3 +276,25 @@ test('with autozoom a tilt written back by Explore keeps
the viewer on the same
);
expect(mockViewports.at(-1)).toEqual(fitted);
});
+
+test('rows with a null dimension get their own legend category', () => {
+ const fd = {
+ datasource: '1__table',
+ viz_type: 'deck_scatter',
+ color_scheme_type: COLOR_SCHEME_TYPES.categorical_palette,
+ color_scheme: 'supersetColors',
+ dimension: 'category',
+ slice_id: 42,
+ color_picker: { r: 0, g: 0, b: 0, a: 1 },
+ } as unknown as QueryFormData;
+ const data = [{ cat_color: 'A' }, { cat_color: NULL_CATEGORY_KEY }];
+
+ const categories = getCategories(fd, data);
+
+ expect(categories[NULL_CATEGORY_KEY]?.enabled).toBe(true);
+ expect(categories[NULL_CATEGORY_KEY]?.color).toBeDefined();
+
+ // Without a category entry the container drops the feature entirely, which
is
+ // how null-dimension rows used to disappear from the map.
+ expect(data.filter(d => categories[d.cat_color]?.enabled)).toHaveLength(2);
Review Comment:
Agreed, the old test was mostly restating the filter. Added a render test
that mounts `CategoricalDeckGLContainer` with `A` and null rows, checks both
reach the layer (null row with its own color), and that clicking N/A in the
legend drops only the null row. I could not run Jest locally here, so CI will
be the first real run (864f1c849f).
--
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]
