rusackas commented on code in PR #45585:
URL: https://github.com/apache/superset/pull/45585#discussion_r4367112720


##########
superset-frontend/plugins/preset-chart-deckgl/src/utilities/tooltipUtils.tsx:
##########
@@ -348,6 +349,12 @@ export function createHandlebarsTooltipData(
     data = { ...data, ...tooltipData };
   }
 
+  // Applied after the tooltip-contents merge so a configured `cat_color` item
+  // cannot reintroduce the raw null sentinel.
+  if (data.cat_color != null) {
+    data = { ...data, cat_color: formatNullCategory(`${data.cat_color}`) };

Review Comment:
   @sadpandajoe good catch. The replacement now only runs when `cat_color` is 
the null sentinel; other values pass through untouched. Added a test with an 
array `cat_color`.



##########
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:
   @sadpandajoe yes, the field-based branch now formats the sentinel through 
`formatNullCategory` too, with a test.



##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Scatter/Scatter.tsx:
##########
@@ -67,7 +67,7 @@ function setTooltipContent(
         {o.object?.cat_color && (
           <TooltipRow
             label={`${t('Category')}: `}
-            value={`${o.object?.cat_color}`}
+            value={formatNullCategory(`${o.object?.cat_color}`)}

Review Comment:
   @sadpandajoe added `Scatter.test.tsx`: calls `onHover` through `getLayer` 
with no template or contents and a sentinel `cat_color`, then asserts `N/A` 
shows and `<NULL>` does not.



-- 
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]

Reply via email to