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 da20cfab85a95..55ad6015c0d65 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 16bf93d047c4a..71f0f4b8385b9 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 = ({ name, label, @@ -55,6 +65,22 @@ const ControlHeader: FC = ({ 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 = ({ > {description && ( - {' '} + > + + {' '} )} {renderTrigger && ( @@ -100,8 +130,6 @@ const ControlHeader: FC = ({ ); }; - const labelClass = validationErrors?.length > 0 ? 'text-danger' : ''; - return (
@@ -118,7 +146,6 @@ const ControlHeader: FC = ({ role="button" tabIndex={0} onClick={onClick} - className={labelClass} style={{ cursor: onClick ? 'pointer' : '' }} > {label} @@ -138,13 +165,18 @@ const ControlHeader: FC = ({ )} {validationErrors?.length > 0 && ( - + - + {' '} )} diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx index b29369d4e560d..0bd3fc0d46442 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 = () => ( { {label} {' '} {description && ( - // label is only used in tooltip id (should probably call this prop `id`) - + + + )} {hasErrors && ( - + + + )} ); @@ -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 ( <> {t('Data')} {props.errorMessage && ( 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} > - + )} - ), - [props.errorMessage], - ); + ); + }, [ + colors.error.base, + colors.alert.base, + dataTabHasHadNoErrors, + props.errorMessage, + ]); const controlPanelRegistry = getChartControlPanelRegistry(); if (