[GitHub] [superset] diegomedina248 commented on a change in pull request #19259: feat(explore): Dataset panel option tooltips

[GitBox <gi...@apache.org>]

diegomedina248 commented on a change in pull request #19259:
URL: https://github.com/apache/superset/pull/19259#discussion_r830942092



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx
##########
@@ -60,22 +96,27 @@ export const getMetricTooltipNode = (
   labelRef?: React.RefObject<any>,
 ): ReactNode => {
   // don't show tooltip if it hasn't verbose_name, label and hasn't truncated
-  if (!metric.verbose_name && !metric.label && !isLabelTruncated(labelRef)) {
+  if (
+    !metric.verbose_name &&
+    !metric.description &&
+    !metric.label &&
+    !isLabelTruncated(labelRef)
+  ) {
     return null;
   }
 
-  if (metric.verbose_name) {
-    return (
-      <>
-        <div>{t('metric name: %s', metric.metric_name)}</div>
-        <div>{t('verbose name: %s', metric.verbose_name)}</div>
-      </>
-    );
-  }
-
-  if (isLabelTruncated(labelRef) && metric.label) {
-    return t('label name: %s', metric.label);
-  }
-
-  return t('metric name: %s', metric.metric_name);
+  return (
+    <>
+      <TooltipSection label={t('Metric name')} text={metric.metric_name} />
+      {(metric.label || metric.verbose_name) && (

Review comment:
       If both are falsy, but not empty, it could potentially render something.
   Even if they're typed as strings (which would be fine), it's better to be sure and assert this short circuit checks to boolean values, either with `!!` or `Boolean(condition)`

##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx
##########
@@ -36,21 +68,25 @@ export const getColumnTooltipNode = (
   labelRef?: React.RefObject<any>,
 ): ReactNode => {
   // don't show tooltip if it hasn't verbose_name and hasn't truncated

Review comment:
       is this comment adding anything? (we should add the description property in any rate)
   

##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx
##########
@@ -60,22 +96,27 @@ export const getMetricTooltipNode = (
   labelRef?: React.RefObject<any>,
 ): ReactNode => {
   // don't show tooltip if it hasn't verbose_name, label and hasn't truncated

Review comment:
       is this comment adding anything? (we should add the description property in any rate)




-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org