You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2022/04/19 12:57:19 UTC
[superset] branch master updated: feat(explore): Replace overlay with alert banner when chart controls change (#19696)
This is an automated email from the ASF dual-hosted git repository.
kgabryje 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 6f4480a06c feat(explore): Replace overlay with alert banner when chart controls change (#19696)
6f4480a06c is described below
commit 6f4480a06cf4b48f7ab69a55016a0c9ad2c3790b
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Tue Apr 19 14:57:06 2022 +0200
feat(explore): Replace overlay with alert banner when chart controls change (#19696)
* Rename explore alert
* Rename refreshOverlayVisible to chartIsStale
* Implement banners
* Add tests
* Add clickable text to empty state
* Fix viz type switching
* styling changes
* Fixes after rebasing
* Code review fixes
* Fix bug
* Fix redundant refreshing
---
superset-frontend/src/components/Chart/Chart.jsx | 64 +++--------
.../src/components/Chart/ChartRenderer.jsx | 28 ++---
.../src/components/Chart/ChartRenderer.test.jsx | 19 +--
.../src/explore/components/ControlPanelAlert.tsx | 98 ----------------
.../explore/components/ControlPanelsContainer.tsx | 7 +-
.../src/explore/components/ExploreAlert.tsx | 127 +++++++++++++++++++++
.../src/explore/components/ExploreChartPanel.jsx | 89 ++++++++++++---
.../explore/components/ExploreChartPanel.test.jsx | 75 +++++++++---
.../ExploreViewContainer.test.tsx | 5 +-
.../components/ExploreViewContainer/index.jsx | 41 ++++---
.../controlUtils/getFormDataFromControls.ts | 7 +-
.../getChartRequiredFieldsMissingMessage.ts} | 22 ++--
12 files changed, 353 insertions(+), 229 deletions(-)
diff --git a/superset-frontend/src/components/Chart/Chart.jsx b/superset-frontend/src/components/Chart/Chart.jsx
index 35209bb94a..7df33d0c5d 100644
--- a/superset-frontend/src/components/Chart/Chart.jsx
+++ b/superset-frontend/src/components/Chart/Chart.jsx
@@ -22,7 +22,6 @@ import { styled, logging, t, ensureIsArray } from '@superset-ui/core';
import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants';
-import Button from 'src/components/Button';
import Loading from 'src/components/Loading';
import { EmptyStateBig } from 'src/components/EmptyState';
import ErrorBoundary from 'src/components/ErrorBoundary';
@@ -32,6 +31,7 @@ import { getUrlParam } from 'src/utils/urlUtils';
import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
import ChartRenderer from './ChartRenderer';
import { ChartErrorMessage } from './ChartErrorMessage';
+import { getChartRequiredFieldsMissingMessage } from '../../utils/getChartRequiredFieldsMissingMessage';
const propTypes = {
annotationData: PropTypes.object,
@@ -64,7 +64,7 @@ const propTypes = {
chartStackTrace: PropTypes.string,
queriesResponse: PropTypes.arrayOf(PropTypes.object),
triggerQuery: PropTypes.bool,
- refreshOverlayVisible: PropTypes.bool,
+ chartIsStale: PropTypes.bool,
errorMessage: PropTypes.node,
// dashboard callbacks
addFilter: PropTypes.func,
@@ -108,20 +108,8 @@ const Styles = styled.div`
}
`;
-const RefreshOverlayWrapper = styled.div`
- position: absolute;
- top: 0;
- left: 0;
- width: 100%;
- height: 100%;
- display: flex;
- align-items: center;
- justify-content: center;
-`;
-
const MonospaceDiv = styled.div`
font-family: ${({ theme }) => theme.typography.families.monospace};
- white-space: pre;
word-break: break-word;
overflow-x: auto;
white-space: pre-wrap;
@@ -255,34 +243,23 @@ class Chart extends React.PureComponent {
chartAlert,
chartStatus,
errorMessage,
- onQuery,
- refreshOverlayVisible,
+ chartIsStale,
queriesResponse = [],
isDeactivatedViz = false,
width,
} = this.props;
const isLoading = chartStatus === 'loading';
- const isFaded = refreshOverlayVisible && !errorMessage;
this.renderContainerStartTime = Logger.getTimestamp();
if (chartStatus === 'failed') {
return queriesResponse.map(item => this.renderErrorMessage(item));
}
- if (errorMessage) {
- const description = isFeatureEnabled(
- FeatureFlag.ENABLE_EXPLORE_DRAG_AND_DROP,
- )
- ? t(
- 'Drag and drop values into highlighted field(s) on the left control panel and run query',
- )
- : t(
- 'Select values in highlighted field(s) on the left control panel and run query',
- );
+ if (errorMessage && ensureIsArray(queriesResponse).length === 0) {
return (
<EmptyStateBig
title={t('Add required control values to preview chart')}
- description={description}
+ description={getChartRequiredFieldsMissingMessage(true)}
image="chart.svg"
/>
);
@@ -291,15 +268,24 @@ class Chart extends React.PureComponent {
if (
!isLoading &&
!chartAlert &&
- isFaded &&
+ !errorMessage &&
+ chartIsStale &&
ensureIsArray(queriesResponse).length === 0
) {
return (
<EmptyStateBig
title={t('Your chart is ready to go!')}
- description={t(
- 'Click on "Create chart" button in the control panel on the left to preview a visualization',
- )}
+ description={
+ <span>
+ {t(
+ 'Click on "Create chart" button in the control panel on the left to preview a visualization or',
+ )}{' '}
+ <span role="button" tabIndex={0} onClick={this.props.onQuery}>
+ {t('click here')}
+ </span>
+ .
+ </span>
+ }
image="chart.svg"
/>
);
@@ -317,25 +303,13 @@ class Chart extends React.PureComponent {
height={height}
width={width}
>
- <div
- className={`slice_container ${isFaded ? ' faded' : ''}`}
- data-test="slice-container"
- >
+ <div className="slice_container" data-test="slice-container">
<ChartRenderer
{...this.props}
source={this.props.dashboardId ? 'dashboard' : 'explore'}
data-test={this.props.vizType}
/>
</div>
-
- {!isLoading && !chartAlert && isFaded && (
- <RefreshOverlayWrapper>
- <Button onClick={onQuery} buttonStyle="primary">
- {t('Run query')}
- </Button>
- </RefreshOverlayWrapper>
- )}
-
{isLoading && !isDeactivatedViz && <Loading />}
</Styles>
</ErrorBoundary>
diff --git a/superset-frontend/src/components/Chart/ChartRenderer.jsx b/superset-frontend/src/components/Chart/ChartRenderer.jsx
index b814b6fde6..45feb6ffd5 100644
--- a/superset-frontend/src/components/Chart/ChartRenderer.jsx
+++ b/superset-frontend/src/components/Chart/ChartRenderer.jsx
@@ -30,6 +30,7 @@ const propTypes = {
datasource: PropTypes.object,
initialValues: PropTypes.object,
formData: PropTypes.object.isRequired,
+ latestQueryFormData: PropTypes.object,
labelColors: PropTypes.object,
sharedLabelColors: PropTypes.object,
height: PropTypes.number,
@@ -42,7 +43,7 @@ const propTypes = {
chartStatus: PropTypes.string,
queriesResponse: PropTypes.arrayOf(PropTypes.object),
triggerQuery: PropTypes.bool,
- refreshOverlayVisible: PropTypes.bool,
+ chartIsStale: PropTypes.bool,
// dashboard callbacks
addFilter: PropTypes.func,
setDataMask: PropTypes.func,
@@ -58,6 +59,8 @@ const BLANK = {};
const BIG_NO_RESULT_MIN_WIDTH = 300;
const BIG_NO_RESULT_MIN_HEIGHT = 220;
+const behaviors = [Behavior.INTERACTIVE_CHART];
+
const defaultProps = {
addFilter: () => BLANK,
onFilterMenuOpen: () => BLANK,
@@ -93,8 +96,7 @@ class ChartRenderer extends React.Component {
const resultsReady =
nextProps.queriesResponse &&
['success', 'rendered'].indexOf(nextProps.chartStatus) > -1 &&
- !nextProps.queriesResponse?.[0]?.error &&
- !nextProps.refreshOverlayVisible;
+ !nextProps.queriesResponse?.[0]?.error;
if (resultsReady) {
this.hasQueryResponseChange =
@@ -170,16 +172,10 @@ class ChartRenderer extends React.Component {
}
render() {
- const { chartAlert, chartStatus, vizType, chartId, refreshOverlayVisible } =
- this.props;
+ const { chartAlert, chartStatus, chartId } = this.props;
// Skip chart rendering
- if (
- refreshOverlayVisible ||
- chartStatus === 'loading' ||
- !!chartAlert ||
- chartStatus === null
- ) {
+ if (chartStatus === 'loading' || !!chartAlert || chartStatus === null) {
return null;
}
@@ -193,11 +189,17 @@ class ChartRenderer extends React.Component {
initialValues,
ownState,
filterState,
+ chartIsStale,
formData,
+ latestQueryFormData,
queriesResponse,
postTransformProps,
} = this.props;
+ const currentFormData =
+ chartIsStale && latestQueryFormData ? latestQueryFormData : formData;
+ const vizType = currentFormData.viz_type || this.props.vizType;
+
// It's bad practice to use unprefixed `vizType` as classnames for chart
// container. It may cause css conflicts as in the case of legacy table chart.
// When migrating charts, we should gradually add a `superset-chart-` prefix
@@ -255,11 +257,11 @@ class ChartRenderer extends React.Component {
annotationData={annotationData}
datasource={datasource}
initialValues={initialValues}
- formData={formData}
+ formData={currentFormData}
ownState={ownState}
filterState={filterState}
hooks={this.hooks}
- behaviors={[Behavior.INTERACTIVE_CHART]}
+ behaviors={behaviors}
queriesData={queriesResponse}
onRenderSuccess={this.handleRenderSuccess}
onRenderFailure={this.handleRenderFailure}
diff --git a/superset-frontend/src/components/Chart/ChartRenderer.test.jsx b/superset-frontend/src/components/Chart/ChartRenderer.test.jsx
index 7e3a455631..f3ce041517 100644
--- a/superset-frontend/src/components/Chart/ChartRenderer.test.jsx
+++ b/superset-frontend/src/components/Chart/ChartRenderer.test.jsx
@@ -25,22 +25,25 @@ import ChartRenderer from 'src/components/Chart/ChartRenderer';
const requiredProps = {
chartId: 1,
datasource: {},
- formData: {},
- vizType: 'foo',
+ formData: { testControl: 'foo' },
+ latestQueryFormData: {
+ testControl: 'bar',
+ },
+ vizType: 'table',
};
describe('ChartRenderer', () => {
it('should render SuperChart', () => {
const wrapper = shallow(
- <ChartRenderer {...requiredProps} refreshOverlayVisible={false} />,
+ <ChartRenderer {...requiredProps} chartIsStale={false} />,
);
expect(wrapper.find(SuperChart)).toExist();
});
- it('should not render SuperChart when refreshOverlayVisible is true', () => {
- const wrapper = shallow(
- <ChartRenderer {...requiredProps} refreshOverlayVisible />,
- );
- expect(wrapper.find(SuperChart)).not.toExist();
+ it('should use latestQueryFormData instead of formData when chartIsStale is true', () => {
+ const wrapper = shallow(<ChartRenderer {...requiredProps} chartIsStale />);
+ expect(wrapper.find(SuperChart).prop('formData')).toEqual({
+ testControl: 'bar',
+ });
});
});
diff --git a/superset-frontend/src/explore/components/ControlPanelAlert.tsx b/superset-frontend/src/explore/components/ControlPanelAlert.tsx
deleted file mode 100644
index 142e074b55..0000000000
--- a/superset-frontend/src/explore/components/ControlPanelAlert.tsx
+++ /dev/null
@@ -1,98 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-import React from 'react';
-import { styled } from '@superset-ui/core';
-import Button from 'src/components/Button';
-
-interface ControlPanelAlertProps {
- title: string;
- bodyText: string;
- primaryButtonAction: (e: React.MouseEvent) => void;
- secondaryButtonAction?: (e: React.MouseEvent) => void;
- primaryButtonText: string;
- secondaryButtonText?: string;
- type: 'info' | 'warning';
-}
-
-const AlertContainer = styled.div`
- margin: ${({ theme }) => theme.gridUnit * 4}px;
- padding: ${({ theme }) => theme.gridUnit * 4}px;
-
- border: ${({ theme }) => `1px solid ${theme.colors.info.base}`};
- background-color: ${({ theme }) => theme.colors.info.light2};
- border-radius: 2px;
-
- color: ${({ theme }) => theme.colors.info.dark2};
- font-size: ${({ theme }) => theme.typography.sizes.s};
-
- &.alert-type-warning {
- border-color: ${({ theme }) => theme.colors.alert.base};
- background-color: ${({ theme }) => theme.colors.alert.light2};
-
- p {
- color: ${({ theme }) => theme.colors.alert.dark2};
- }
- }
-`;
-
-const ButtonContainer = styled.div`
- display: flex;
- justify-content: flex-end;
- button {
- line-height: 1;
- }
-`;
-
-const Title = styled.p`
- font-weight: ${({ theme }) => theme.typography.weights.bold};
-`;
-
-export const ControlPanelAlert = ({
- title,
- bodyText,
- primaryButtonAction,
- secondaryButtonAction,
- primaryButtonText,
- secondaryButtonText,
- type = 'info',
-}: ControlPanelAlertProps) => (
- <AlertContainer className={`alert-type-${type}`}>
- <Title>{title}</Title>
- <p>{bodyText}</p>
- <ButtonContainer>
- {secondaryButtonAction && secondaryButtonText && (
- <Button
- buttonStyle="link"
- buttonSize="small"
- onClick={secondaryButtonAction}
- >
- {secondaryButtonText}
- </Button>
- )}
- <Button
- buttonStyle={type === 'warning' ? 'warning' : 'primary'}
- buttonSize="small"
- onClick={primaryButtonAction}
- >
- {primaryButtonText}
- </Button>
- </ButtonContainer>
- </AlertContainer>
-);
diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
index e20e8574e3..d4ef269027 100644
--- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
+++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
@@ -60,7 +60,7 @@ import { Tooltip } from 'src/components/Tooltip';
import ControlRow from './ControlRow';
import Control from './Control';
-import { ControlPanelAlert } from './ControlPanelAlert';
+import { ExploreAlert } from './ExploreAlert';
import { RunQueryButton } from './RunQueryButton';
export type ControlPanelsContainerProps = {
@@ -92,6 +92,7 @@ const actionButtonsContainerStyles = (theme: SupersetTheme) => css`
flex-direction: column;
align-items: center;
padding: ${theme.gridUnit * 4}px;
+ z-index: 999;
background: linear-gradient(
transparent,
${theme.colors.grayscale.light5} ${theme.opacity.mediumLight}
@@ -443,7 +444,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
const DatasourceAlert = useCallback(
() =>
hasControlsTransferred ? (
- <ControlPanelAlert
+ <ExploreAlert
title={t('Keep control settings?')}
bodyText={t(
"You've changed datasets. Any controls with data (columns, metrics) that match this new dataset have been retained.",
@@ -455,7 +456,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
type="info"
/>
) : (
- <ControlPanelAlert
+ <ExploreAlert
title={t('No form settings were maintained')}
bodyText={t(
'We were unable to carry over any controls when switching to this new dataset.',
diff --git a/superset-frontend/src/explore/components/ExploreAlert.tsx b/superset-frontend/src/explore/components/ExploreAlert.tsx
new file mode 100644
index 0000000000..34c4cf070e
--- /dev/null
+++ b/superset-frontend/src/explore/components/ExploreAlert.tsx
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React, { forwardRef, RefObject } from 'react';
+import { css, styled } from '@superset-ui/core';
+import Button from 'src/components/Button';
+
+interface ControlPanelAlertProps {
+ title: string;
+ bodyText: string;
+ primaryButtonAction?: (e: React.MouseEvent) => void;
+ secondaryButtonAction?: (e: React.MouseEvent) => void;
+ primaryButtonText?: string;
+ secondaryButtonText?: string;
+ type: 'info' | 'warning';
+ className?: string;
+}
+
+const AlertContainer = styled.div`
+ ${({ theme }) => css`
+ margin: ${theme.gridUnit * 4}px;
+ padding: ${theme.gridUnit * 4}px;
+
+ border: 1px solid ${theme.colors.info.base};
+ background-color: ${theme.colors.info.light2};
+ border-radius: 2px;
+
+ color: ${theme.colors.info.dark2};
+ font-size: ${theme.typography.sizes.m}px;
+
+ p {
+ margin-bottom: ${theme.gridUnit}px;
+ }
+
+ & a,
+ & span[role='button'] {
+ color: inherit;
+ text-decoration: underline;
+ &:hover {
+ color: ${theme.colors.info.dark1};
+ }
+ }
+
+ &.alert-type-warning {
+ border-color: ${theme.colors.alert.base};
+ background-color: ${theme.colors.alert.light2};
+
+ p {
+ color: ${theme.colors.alert.dark2};
+ }
+
+ & a:hover,
+ & span[role='button']:hover {
+ color: ${theme.colors.alert.dark1};
+ }
+ }
+ `}
+`;
+
+const ButtonContainer = styled.div`
+ display: flex;
+ justify-content: flex-end;
+ button {
+ line-height: 1;
+ }
+`;
+
+const Title = styled.p`
+ font-weight: ${({ theme }) => theme.typography.weights.bold};
+`;
+
+export const ExploreAlert = forwardRef(
+ (
+ {
+ title,
+ bodyText,
+ primaryButtonAction,
+ secondaryButtonAction,
+ primaryButtonText,
+ secondaryButtonText,
+ type = 'info',
+ className = '',
+ }: ControlPanelAlertProps,
+ ref: RefObject<HTMLDivElement>,
+ ) => (
+ <AlertContainer className={`alert-type-${type} ${className}`} ref={ref}>
+ <Title>{title}</Title>
+ <p>{bodyText}</p>
+ {primaryButtonText && primaryButtonAction && (
+ <ButtonContainer>
+ {secondaryButtonAction && secondaryButtonText && (
+ <Button
+ buttonStyle="link"
+ buttonSize="small"
+ onClick={secondaryButtonAction}
+ >
+ {secondaryButtonText}
+ </Button>
+ )}
+ <Button
+ buttonStyle={type === 'warning' ? 'warning' : 'primary'}
+ buttonSize="small"
+ onClick={primaryButtonAction}
+ >
+ {primaryButtonText}
+ </Button>
+ </ButtonContainer>
+ )}
+ </AlertContainer>
+ ),
+);
diff --git a/superset-frontend/src/explore/components/ExploreChartPanel.jsx b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
index 37523713a8..5cd818f52e 100644
--- a/superset-frontend/src/explore/components/ExploreChartPanel.jsx
+++ b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
@@ -19,7 +19,14 @@
import React, { useState, useEffect, useCallback, useMemo } from 'react';
import PropTypes from 'prop-types';
import Split from 'react-split';
-import { css, styled, SupersetClient, useTheme } from '@superset-ui/core';
+import {
+ css,
+ ensureIsArray,
+ styled,
+ SupersetClient,
+ t,
+ useTheme,
+} from '@superset-ui/core';
import { useResizeDetector } from 'react-resize-detector';
import { chartPropShape } from 'src/dashboard/util/propShapes';
import ChartContainer from 'src/components/Chart/ChartContainer';
@@ -31,6 +38,8 @@ import {
import { DataTablesPane } from './DataTablesPane';
import { buildV1ChartDataPayload } from '../exploreUtils';
import { ChartPills } from './ChartPills';
+import { ExploreAlert } from './ExploreAlert';
+import { getChartRequiredFieldsMissingMessage } from '../../utils/getChartRequiredFieldsMissingMessage';
const propTypes = {
actions: PropTypes.object.isRequired,
@@ -51,7 +60,7 @@ const propTypes = {
standalone: PropTypes.number,
force: PropTypes.bool,
timeout: PropTypes.number,
- refreshOverlayVisible: PropTypes.bool,
+ chartIsStale: PropTypes.bool,
chart: chartPropShape,
errorMessage: PropTypes.node,
triggerRender: PropTypes.bool,
@@ -115,10 +124,11 @@ const ExploreChartPanel = ({
errorMessage,
form_data: formData,
onQuery,
- refreshOverlayVisible,
actions,
timeout,
standalone,
+ chartIsStale,
+ chartAlert,
}) => {
const theme = useTheme();
const gutterMargin = theme.gridUnit * GUTTER_SIZE_FACTOR;
@@ -134,6 +144,13 @@ const ExploreChartPanel = ({
const [splitSizes, setSplitSizes] = useState(
getItem(LocalStorageKeys.chart_split_sizes, INITIAL_SIZES),
);
+
+ const showAlertBanner =
+ !chartAlert &&
+ chartIsStale &&
+ chart.chartStatus !== 'failed' &&
+ ensureIsArray(chart.queriesResponse).length > 0;
+
const updateQueryContext = useCallback(
async function fetchChartData() {
if (slice && slice.query_context === null) {
@@ -167,11 +184,11 @@ const ExploreChartPanel = ({
setItem(LocalStorageKeys.chart_split_sizes, splitSizes);
}, [splitSizes]);
- const onDragEnd = sizes => {
+ const onDragEnd = useCallback(sizes => {
setSplitSizes(sizes);
- };
+ }, []);
- const refreshCachedQuery = () => {
+ const refreshCachedQuery = useCallback(() => {
actions.postChartFormData(
formData,
true,
@@ -180,7 +197,7 @@ const ExploreChartPanel = ({
undefined,
ownState,
);
- };
+ }, [actions, chart.id, formData, ownState, timeout]);
const onCollapseChange = useCallback(isOpen => {
let splitSizes;
@@ -219,9 +236,10 @@ const ExploreChartPanel = ({
datasource={datasource}
errorMessage={errorMessage}
formData={formData}
+ latestQueryFormData={chart.latestQueryFormData}
onQuery={onQuery}
queriesResponse={chart.queriesResponse}
- refreshOverlayVisible={refreshOverlayVisible}
+ chartIsStale={chartIsStale}
setControlValue={actions.setControlValue}
timeout={timeout}
triggerQuery={chart.triggerQuery}
@@ -237,8 +255,10 @@ const ExploreChartPanel = ({
chart.chartStackTrace,
chart.chartStatus,
chart.id,
+ chart.latestQueryFormData,
chart.queriesResponse,
chart.triggerQuery,
+ chartIsStale,
chartPanelHeight,
chartPanelRef,
chartPanelWidth,
@@ -248,7 +268,6 @@ const ExploreChartPanel = ({
formData,
onQuery,
ownState,
- refreshOverlayVisible,
timeout,
triggerRender,
vizType,
@@ -264,6 +283,34 @@ const ExploreChartPanel = ({
flex-direction: column;
`}
>
+ {showAlertBanner && (
+ <ExploreAlert
+ title={
+ errorMessage
+ ? t('Required control values have been removed')
+ : t('Your chart is not up to date')
+ }
+ bodyText={
+ errorMessage ? (
+ getChartRequiredFieldsMissingMessage(false)
+ ) : (
+ <span>
+ {t(
+ 'You updated the values in the control panel, but the chart was not updated automatically. Run the query by clicking on the "Update chart" button or',
+ )}{' '}
+ <span role="button" tabIndex={0} onClick={onQuery}>
+ {t('click here')}
+ </span>
+ .
+ </span>
+ )
+ }
+ type="warning"
+ css={theme => css`
+ margin: 0 0 ${theme.gridUnit * 4}px 0;
+ `}
+ />
+ )}
<ChartPills
queriesResponse={chart.queriesResponse}
chartStatus={chart.chartStatus}
@@ -275,7 +322,18 @@ const ExploreChartPanel = ({
{renderChart()}
</div>
),
- [chartPanelRef, renderChart],
+ [
+ showAlertBanner,
+ errorMessage,
+ onQuery,
+ chart.queriesResponse,
+ chart.chartStatus,
+ chart.chartUpdateStartTime,
+ chart.chartUpdateEndTime,
+ refreshCachedQuery,
+ formData?.row_limit,
+ renderChart,
+ ],
);
const standaloneChartBody = useMemo(() => renderChart(), [renderChart]);
@@ -294,6 +352,13 @@ const ExploreChartPanel = ({
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [chart.latestQueryFormData]);
+ const elementStyle = useCallback(
+ (dimension, elementSize, gutterSize) => ({
+ [dimension]: `calc(${elementSize}% - ${gutterSize + gutterMargin}px)`,
+ }),
+ [gutterMargin],
+ );
+
if (standalone) {
// dom manipulation hack to get rid of the boostrap theme's body background
const standaloneClass = 'background-transparent';
@@ -304,10 +369,6 @@ const ExploreChartPanel = ({
return standaloneChartBody;
}
- const elementStyle = (dimension, elementSize, gutterSize) => ({
- [dimension]: `calc(${elementSize}% - ${gutterSize + gutterMargin}px)`,
- });
-
return (
<Styles className="panel panel-default chart-container">
{vizType === 'filter_box' ? (
diff --git a/superset-frontend/src/explore/components/ExploreChartPanel.test.jsx b/superset-frontend/src/explore/components/ExploreChartPanel.test.jsx
index c50a605a40..a779773052 100644
--- a/superset-frontend/src/explore/components/ExploreChartPanel.test.jsx
+++ b/superset-frontend/src/explore/components/ExploreChartPanel.test.jsx
@@ -17,23 +17,70 @@
* under the License.
*/
import React from 'react';
-
+import { render, screen } from 'spec/helpers/testing-library';
import ChartContainer from 'src/explore/components/ExploreChartPanel';
-describe('ChartContainer', () => {
- const mockProps = {
- sliceName: 'Trend Line',
- vizType: 'line',
- height: '500px',
- actions: {},
- can_overwrite: false,
- can_download: false,
- containerId: 'foo',
- width: '50px',
- isStarred: false,
- };
+const createProps = (overrides = {}) => ({
+ sliceName: 'Trend Line',
+ vizType: 'line',
+ height: '500px',
+ actions: {},
+ can_overwrite: false,
+ can_download: false,
+ containerId: 'foo',
+ width: '500px',
+ isStarred: false,
+ chartIsStale: false,
+ chart: {},
+ form_data: {},
+ ...overrides,
+});
+describe('ChartContainer', () => {
it('renders when vizType is line', () => {
- expect(React.isValidElement(<ChartContainer {...mockProps} />)).toBe(true);
+ const props = createProps();
+ expect(React.isValidElement(<ChartContainer {...props} />)).toBe(true);
+ });
+
+ it('renders with alert banner', () => {
+ const props = createProps({
+ chartIsStale: true,
+ chart: { chartStatus: 'rendered', queriesResponse: [{}] },
+ });
+ render(<ChartContainer {...props} />, { useRedux: true });
+ expect(screen.getByText('Your chart is not up to date')).toBeVisible();
+ });
+
+ it('doesnt render alert banner when no changes in control panel were made (chart is not stale)', () => {
+ const props = createProps({
+ chartIsStale: false,
+ });
+ render(<ChartContainer {...props} />, { useRedux: true });
+ expect(
+ screen.queryByText('Your chart is not up to date'),
+ ).not.toBeInTheDocument();
+ });
+
+ it('doesnt render alert banner when chart not created yet (no queries response)', () => {
+ const props = createProps({
+ chartIsStale: true,
+ chart: { queriesResponse: [] },
+ });
+ render(<ChartContainer {...props} />, { useRedux: true });
+ expect(
+ screen.queryByText('Your chart is not up to date'),
+ ).not.toBeInTheDocument();
+ });
+
+ it('renders prompt to fill required controls when required control removed', () => {
+ const props = createProps({
+ chartIsStale: true,
+ chart: { chartStatus: 'rendered', queriesResponse: [{}] },
+ errorMessage: 'error',
+ });
+ render(<ChartContainer {...props} />, { useRedux: true });
+ expect(
+ screen.getByText('Required control values have been removed'),
+ ).toBeVisible();
});
});
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
index a240578c49..7743997a35 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
@@ -27,7 +27,10 @@ import ExploreViewContainer from '.';
const reduxState = {
explore: {
common: { conf: { SUPERSET_WEBSERVER_TIMEOUT: 60 } },
- controls: { datasource: { value: '1__table' } },
+ controls: {
+ datasource: { value: '1__table' },
+ viz_type: { value: 'table' },
+ },
datasource: {
id: 1,
type: 'table',
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
index 7299adf251..da18dcc4ff 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
@@ -22,7 +22,7 @@ import PropTypes from 'prop-types';
import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import { styled, t, css, useTheme, logging } from '@superset-ui/core';
-import { debounce } from 'lodash';
+import { debounce, pick } from 'lodash';
import { Resizable } from 're-resizable';
import { useChangeEffect } from 'src/hooks/useChangeEffect';
import { usePluginContext } from 'src/components/DynamicPlugins';
@@ -381,18 +381,33 @@ function ExploreViewContainer(props) {
}
}, []);
- const reRenderChart = () => {
- props.actions.updateQueryFormData(
- getFormDataFromControls(props.controls),
+ const reRenderChart = useCallback(
+ controlsChanged => {
+ const newQueryFormData = controlsChanged
+ ? {
+ ...props.chart.latestQueryFormData,
+ ...getFormDataFromControls(pick(props.controls, controlsChanged)),
+ }
+ : getFormDataFromControls(props.controls);
+ props.actions.updateQueryFormData(newQueryFormData, props.chart.id);
+ props.actions.renderTriggered(new Date().getTime(), props.chart.id);
+ addHistory();
+ },
+ [
+ addHistory,
+ props.actions,
props.chart.id,
- );
- props.actions.renderTriggered(new Date().getTime(), props.chart.id);
- addHistory();
- };
+ props.chart.latestQueryFormData,
+ props.controls,
+ ],
+ );
// effect to run when controls change
useEffect(() => {
- if (previousControls) {
+ if (
+ previousControls &&
+ props.chart.latestQueryFormData.viz_type === props.controls.viz_type.value
+ ) {
if (
props.controls.datasource &&
(previousControls.datasource == null ||
@@ -412,11 +427,11 @@ function ExploreViewContainer(props) {
);
// this should also be handled by the actions that are actually changing the controls
- const hasDisplayControlChanged = changedControlKeys.some(
+ const displayControlsChanged = changedControlKeys.filter(
key => props.controls[key].renderTrigger,
);
- if (hasDisplayControlChanged) {
- reRenderChart();
+ if (displayControlsChanged.length > 0) {
+ reRenderChart(displayControlsChanged);
}
}
}, [props.controls, props.ownState]);
@@ -493,7 +508,7 @@ function ExploreViewContainer(props) {
<ExploreChartPanel
{...props}
errorMessage={errorMessage}
- refreshOverlayVisible={chartIsStale}
+ chartIsStale={chartIsStale}
onQuery={onQuery}
/>
);
diff --git a/superset-frontend/src/explore/controlUtils/getFormDataFromControls.ts b/superset-frontend/src/explore/controlUtils/getFormDataFromControls.ts
index f5ffa523c3..ba9419da18 100644
--- a/superset-frontend/src/explore/controlUtils/getFormDataFromControls.ts
+++ b/superset-frontend/src/explore/controlUtils/getFormDataFromControls.ts
@@ -22,13 +22,10 @@ import { ControlStateMapping } from '@superset-ui/chart-controls';
export function getFormDataFromControls(
controlsState: ControlStateMapping,
): QueryFormData {
- const formData: QueryFormData = {
- viz_type: 'table',
- datasource: '',
- };
+ const formData = {};
Object.keys(controlsState).forEach(controlName => {
const control = controlsState[controlName];
formData[controlName] = control.value;
});
- return formData;
+ return formData as QueryFormData;
}
diff --git a/superset-frontend/src/explore/controlUtils/getFormDataFromControls.ts b/superset-frontend/src/utils/getChartRequiredFieldsMissingMessage.ts
similarity index 63%
copy from superset-frontend/src/explore/controlUtils/getFormDataFromControls.ts
copy to superset-frontend/src/utils/getChartRequiredFieldsMissingMessage.ts
index f5ffa523c3..ac11e8503d 100644
--- a/superset-frontend/src/explore/controlUtils/getFormDataFromControls.ts
+++ b/superset-frontend/src/utils/getChartRequiredFieldsMissingMessage.ts
@@ -16,19 +16,11 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { QueryFormData } from '@superset-ui/core';
-import { ControlStateMapping } from '@superset-ui/chart-controls';
-export function getFormDataFromControls(
- controlsState: ControlStateMapping,
-): QueryFormData {
- const formData: QueryFormData = {
- viz_type: 'table',
- datasource: '',
- };
- Object.keys(controlsState).forEach(controlName => {
- const control = controlsState[controlName];
- formData[controlName] = control.value;
- });
- return formData;
-}
+import { t } from '@superset-ui/core';
+
+export const getChartRequiredFieldsMissingMessage = (isCreating: boolean) =>
+ t(
+ 'Select values in highlighted field(s) in the control panel. Then run the query by clicking on the %s button.',
+ isCreating ? '"Create chart"' : '"Update chart"',
+ );