This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new e7acb1a79d chore(explore): update Explore icons and icon colors
(#20612)
e7acb1a79d is described below
commit e7acb1a79dc72d7e3fa1969d63eb8a3560e697a9
Author: Cody Leff <[email protected]>
AuthorDate: Fri Jul 29 11:05:15 2022 -0400
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.
---
.../explore/visualizations/line.test.ts | 11 ++-
.../src/explore/components/ControlHeader.tsx | 54 +++++++++---
.../explore/components/ControlPanelsContainer.tsx | 96 ++++++++++++++++++----
3 files changed, 133 insertions(+), 28 deletions(-)
diff --git
a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
index da20cfab85..55ad6015c0 100644
---
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');
});
diff --git a/superset-frontend/src/explore/components/ControlHeader.tsx
b/superset-frontend/src/explore/components/ControlHeader.tsx
index 16bf93d047..71f0f4b838 100644
--- 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>
)}
diff --git
a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
index b29369d4e5..0bd3fc0d46 100644
--- 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 (
