chore(explore): update Explore icons and icon colors (#20612)

* Update Explore icons and icon colors. * Change shade of blue and make blue only appear when fields have never been filled in. * Fix Cypress test. * Update non-error validation color from blue to yellow. * Unpack ternary. * Replace direct AntD imports with our Icons component.
2022-07-29 11:05:15 -04:00 · 2022-07-29 11:05:15 -04:00 · e7acb1a79d
parent 90460f1333
commit e7acb1a79d
3 changed files with 133 additions and 28 deletions
--- a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
+++ b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
@ -40,7 +40,10 @@ describe('Visualization > Line', () => {
    cy.get('.panel-body').contains(
      `Add required control values to preview chart`,
    );
-    cy.get('.text-danger').contains('Metrics');
+    cy.get('[data-test="metrics-header"]').contains('Metrics');
+    cy.get('[data-test="metrics-header"] [data-test="error-tooltip"]').should(
+      'exist',
+    );

    cy.get('[data-test=metrics]')
      .contains('Drop columns/metrics here or click')
@ -55,7 +58,11 @@ describe('Visualization > Line', () => {
      .type('sum{enter}');
    cy.get('[data-test="AdhocMetricEdit#save"]').contains('Save').click();

-    cy.get('.text-danger').should('not.exist');
+    cy.get('[data-test="metrics-header"]').contains('Metrics');
+    cy.get('[data-test="metrics-header"] [data-test="error-tooltip"]').should(
+      'not.exist',
+    );
+
    cy.get('.ant-alert-warning').should('not.exist');
  });

--- a/superset-frontend/src/explore/components/ControlHeader.tsx
+++ b/superset-frontend/src/explore/components/ControlHeader.tsx
@ -16,7 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import React, { FC, ReactNode } from 'react';
+import React, { FC, ReactNode, useMemo, useRef } from 'react';
 import { t, css, useTheme, SupersetTheme } from '@superset-ui/core';
 import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
 import { Tooltip } from 'src/components/Tooltip';
@ -40,6 +40,16 @@ export type ControlHeaderProps = {
  danger?: string;
 };

+const iconStyles = css`
+  &.anticon {
+    font-size: unset;
+    .anticon {
+      line-height: unset;
+      vertical-align: unset;
+    }
+  }
+`;
+
 const ControlHeader: FC<ControlHeaderProps> = ({
  name,
  label,
@ -55,6 +65,22 @@ const ControlHeader: FC<ControlHeaderProps> = ({
  danger,
 }) => {
  const { gridUnit, colors } = useTheme();
+  const hasHadNoErrors = useRef(false);
+  const labelColor = useMemo(() => {
+    if (!validationErrors.length) {
+      hasHadNoErrors.current = true;
+    }
+
+    if (hasHadNoErrors.current) {
+      if (validationErrors.length) {
+        return colors.error.base;
+      }
+
+      return 'unset';
+    }
+
+    return colors.alert.base;
+  }, [colors.error.base, colors.alert.base, validationErrors.length]);

  if (!label) {
    return null;
@ -78,12 +104,16 @@ const ControlHeader: FC<ControlHeaderProps> = ({
      >
        {description && (
          <span>
-            <InfoTooltipWithTrigger
-              label={t('description')}
-              tooltip={description}
+            <Tooltip
+              id={`${t('description')}-tooltip`}
+              title={description}
              placement="top"
-              onClick={tooltipOnClick}
-            />{' '}
+            >
+              <Icons.InfoCircleOutlined
+                css={iconStyles}
+                onClick={tooltipOnClick}
+              />
+            </Tooltip>{' '}
          </span>
        )}
        {renderTrigger && (
@ -100,8 +130,6 @@ const ControlHeader: FC<ControlHeaderProps> = ({
    );
  };

-  const labelClass = validationErrors?.length > 0 ? 'text-danger' : '';
-
  return (
    <div className="ControlHeader" data-test={`${name}-header`}>
      <div className="pull-left">
@ -118,7 +146,6 @@ const ControlHeader: FC<ControlHeaderProps> = ({
            role="button"
            tabIndex={0}
            onClick={onClick}
-            className={labelClass}
            style={{ cursor: onClick ? 'pointer' : '' }}
          >
            {label}
@ -138,13 +165,18 @@ const ControlHeader: FC<ControlHeaderProps> = ({
            </span>
          )}
          {validationErrors?.length > 0 && (
-            <span>
+            <span data-test="error-tooltip">
              <Tooltip
                id="error-tooltip"
                placement="top"
                title={validationErrors?.join(' ')}
              >
-                <Icons.ErrorSolid iconColor={colors.error.base} iconSize="s" />
+                <Icons.ExclamationCircleOutlined
+                  css={css`
+                    ${iconStyles}
+                    color: ${labelColor};
+                  `}
+                />
              </Tooltip>{' '}
            </span>
          )}
--- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
+++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
@ -35,6 +35,7 @@ import {
  DatasourceType,
  css,
  SupersetTheme,
+  useTheme,
 } from '@superset-ui/core';
 import {
  ControlPanelSectionConfig,
@ -42,7 +43,6 @@ import {
  CustomControlItem,
  Dataset,
  ExpandedControlItem,
-  InfoTooltipWithTrigger,
  sections,
 } from '@superset-ui/chart-controls';

@ -56,8 +56,10 @@ import { getSectionsToRender } from 'src/explore/controlUtils';
 import { ExploreActions } from 'src/explore/actions/exploreActions';
 import { ChartState, ExplorePageState } from 'src/explore/types';
 import { Tooltip } from 'src/components/Tooltip';
+import Icons from 'src/components/Icons';

 import { rgba } from 'emotion-rgba';
+import { kebabCase } from 'lodash';
 import ControlRow from './ControlRow';
 import Control from './Control';
 import { ExploreAlert } from './ExploreAlert';
@ -85,6 +87,16 @@ export type ExpandedControlPanelSectionConfig = Omit<
  controlSetRows: ExpandedControlItem[][];
 };

+const iconStyles = css`
+  &.anticon {
+    font-size: unset;
+    .anticon {
+      line-height: unset;
+      vertical-align: unset;
+    }
+  }
+`;
+
 const actionButtonsContainerStyles = (theme: SupersetTheme) => css`
  display: flex;
  position: sticky;
@ -235,7 +247,19 @@ function getState(
  };
 }

+function useResetOnChangeRef(initialValue: () => any, resetOnChangeValue: any) {
+  const value = useRef(initialValue());
+  const prevResetOnChangeValue = useRef(resetOnChangeValue);
+  if (prevResetOnChangeValue.current !== resetOnChangeValue) {
+    value.current = initialValue();
+    prevResetOnChangeValue.current = resetOnChangeValue;
+  }
+
+  return value;
+}
+
 export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
+  const { colors } = useTheme();
  const pluginContext = useContext(PluginContext);

  const prevState = usePrevious(props.exploreState);
@ -367,6 +391,11 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
    );
  };

+  const sectionHasHadNoErrors = useResetOnChangeRef(
+    () => ({}),
+    props.form_data.viz_type,
+  );
+
  const renderControlPanelSection = (
    section: ExpandedControlPanelSectionConfig,
  ) => {
@ -394,6 +423,15 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
        );
      }),
    );
+
+    if (!hasErrors) {
+      sectionHasHadNoErrors.current[sectionId] = true;
+    }
+
+    const errorColor = sectionHasHadNoErrors.current[sectionId]
+      ? colors.error.base
+      : colors.alert.base;
+
    const PanelHeader = () => (
      <span data-test="collapsible-control-panel-header">
        <span
@ -405,15 +443,22 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
          {label}
        </span>{' '}
        {description && (
-          // label is only used in tooltip id (should probably call this prop `id`)
-          <InfoTooltipWithTrigger label={sectionId} tooltip={description} />
+          <Tooltip id={sectionId} title={description}>
+            <Icons.InfoCircleOutlined css={iconStyles} />
+          </Tooltip>
        )}
        {hasErrors && (
-          <InfoTooltipWithTrigger
-            label="validation-errors"
-            bsStyle="danger"
-            tooltip="This section contains validation errors"
-          />
+          <Tooltip
+            id={`${kebabCase('validation-errors')}-tooltip`}
+            title="This section contains validation errors"
+          >
+            <Icons.InfoCircleOutlined
+              css={css`
+                ${iconStyles}
+                color: ${errorColor};
+              `}
+            />
+          </Tooltip>
        )}
      </span>
    );
@ -514,14 +559,26 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
    [handleClearFormClick, handleContinueClick, hasControlsTransferred],
  );

-  const dataTabTitle = useMemo(
-    () => (
+  const dataTabHasHadNoErrors = useResetOnChangeRef(
+    () => false,
+    props.form_data.viz_type,
+  );
+
+  const dataTabTitle = useMemo(() => {
+    if (!props.errorMessage) {
+      dataTabHasHadNoErrors.current = true;
+    }
+
+    const errorColor = dataTabHasHadNoErrors.current
+      ? colors.error.base
+      : colors.alert.base;
+
+    return (
      <>
        <span>{t('Data')}</span>
        {props.errorMessage && (
          <span
            css={(theme: SupersetTheme) => css`
-              font-size: ${theme.typography.sizes.xs}px;
              margin-left: ${theme.gridUnit * 2}px;
            `}
          >
@ -531,14 +588,23 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
              placement="right"
              title={props.errorMessage}
            >
-              <i className="fa fa-exclamation-circle text-danger fa-lg" />
+              <Icons.ExclamationCircleOutlined
+                css={css`
+                  ${iconStyles}
+                  color: ${errorColor};
+                `}
+              />
            </Tooltip>
          </span>
        )}
      </>
-    ),
-    [props.errorMessage],
-  );
+    );
+  }, [
+    colors.error.base,
+    colors.alert.base,
+    dataTabHasHadNoErrors,
+    props.errorMessage,
+  ]);

  const controlPanelRegistry = getChartControlPanelRegistry();
  if (