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/13 14:58:47 UTC
[superset] branch master updated: feat(explore): Redesign of Run/Save buttons (#19558)
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 c8304a2821 feat(explore): Redesign of Run/Save buttons (#19558)
c8304a2821 is described below
commit c8304a2821cc86d01e3e3c01ee597c94b1fb64e9
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Wed Apr 13 16:58:39 2022 +0200
feat(explore): Redesign of Run/Save buttons (#19558)
* feat(explore): Move save button to header, run button to bottom of control panel
* Make the tabs sticky
* Add error icon to Data tab
* Show message when creating chart and all controls are filled correctly
* Add tests and storybook
* Fix tests
* Disable save button when control have errors
* Fix types
* Apply code review comments
* Replace styled with css
* Remove unused import
---
superset-frontend/src/components/Chart/Chart.jsx | 19 +++-
.../components/ControlPanelsContainer.test.tsx | 6 +-
.../explore/components/ControlPanelsContainer.tsx | 100 ++++++++++++++---
.../ExploreAdditionalActionsMenu/index.jsx | 6 +-
.../ExploreChartHeader/ExploreChartHeader.test.tsx | 15 +++
.../components/ExploreChartHeader/index.jsx | 117 +++++++++++--------
.../ExploreViewContainer.test.tsx | 7 +-
.../components/ExploreViewContainer/index.jsx | 25 ++---
.../explore/components/QueryAndSaveBtns.test.jsx | 60 ----------
.../src/explore/components/QueryAndSaveBtns.tsx | 124 ---------------------
.../RunQueryButton.stories.tsx} | 24 ++--
.../RunQueryButton/RunQueryButton.test.tsx | 76 +++++++++++++
.../explore/components/RunQueryButton/index.tsx | 56 ++++++++++
13 files changed, 359 insertions(+), 276 deletions(-)
diff --git a/superset-frontend/src/components/Chart/Chart.jsx b/superset-frontend/src/components/Chart/Chart.jsx
index 0d2914522d..35209bb94a 100644
--- a/superset-frontend/src/components/Chart/Chart.jsx
+++ b/superset-frontend/src/components/Chart/Chart.jsx
@@ -18,7 +18,7 @@
*/
import PropTypes from 'prop-types';
import React from 'react';
-import { styled, logging, t } from '@superset-ui/core';
+import { styled, logging, t, ensureIsArray } from '@superset-ui/core';
import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants';
@@ -288,6 +288,23 @@ class Chart extends React.PureComponent {
);
}
+ if (
+ !isLoading &&
+ !chartAlert &&
+ isFaded &&
+ 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',
+ )}
+ image="chart.svg"
+ />
+ );
+ }
+
return (
<ErrorBoundary
onError={this.handleRenderContainerFailure}
diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx
index 24101d9c17..6af860470c 100644
--- a/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx
+++ b/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx
@@ -30,7 +30,7 @@ import {
ControlPanelsContainerProps,
} from 'src/explore/components/ControlPanelsContainer';
-describe('ControlPanelsContainer2', () => {
+describe('ControlPanelsContainer', () => {
beforeAll(() => {
getChartControlPanelRegistry().registerValue('table', {
controlPanelSections: [
@@ -90,6 +90,10 @@ describe('ControlPanelsContainer2', () => {
form_data: getFormDataFromControls(controls),
isDatasourceMetaLoading: false,
exploreState: {},
+ chart: {
+ queriesResponse: null,
+ chartStatus: 'success',
+ },
} as ControlPanelsContainerProps;
}
diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
index 832d3552f6..e20e8574e3 100644
--- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
+++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
@@ -18,6 +18,7 @@
*/
/* eslint camelcase: 0 */
import React, {
+ ReactNode,
useCallback,
useContext,
useEffect,
@@ -33,6 +34,7 @@ import {
QueryFormData,
DatasourceType,
css,
+ SupersetTheme,
} from '@superset-ui/core';
import {
ControlPanelSectionConfig,
@@ -54,10 +56,12 @@ import { getSectionsToRender } from 'src/explore/controlUtils';
import { ExploreActions } from 'src/explore/actions/exploreActions';
import { ExplorePageState } from 'src/explore/reducers/getInitialState';
import { ChartState } from 'src/explore/types';
+import { Tooltip } from 'src/components/Tooltip';
import ControlRow from './ControlRow';
import Control from './Control';
import { ControlPanelAlert } from './ControlPanelAlert';
+import { RunQueryButton } from './RunQueryButton';
export type ControlPanelsContainerProps = {
exploreState: ExplorePageState['explore'];
@@ -67,6 +71,11 @@ export type ControlPanelsContainerProps = {
controls: Record<string, ControlState>;
form_data: QueryFormData;
isDatasourceMetaLoading: boolean;
+ errorMessage: ReactNode;
+ onQuery: () => void;
+ onStop: () => void;
+ canStopQuery: boolean;
+ chartIsStale: boolean;
};
export type ExpandedControlPanelSectionConfig = Omit<
@@ -76,13 +85,33 @@ export type ExpandedControlPanelSectionConfig = Omit<
controlSetRows: ExpandedControlItem[][];
};
+const actionButtonsContainerStyles = (theme: SupersetTheme) => css`
+ display: flex;
+ position: sticky;
+ bottom: 0;
+ flex-direction: column;
+ align-items: center;
+ padding: ${theme.gridUnit * 4}px;
+ background: linear-gradient(
+ transparent,
+ ${theme.colors.grayscale.light5} ${theme.opacity.mediumLight}
+ );
+
+ & > button {
+ min-width: 156px;
+ }
+`;
+
const Styles = styled.div`
+ position: relative;
height: 100%;
width: 100%;
- overflow: auto;
- overflow-x: visible;
+
+ // Resizable add overflow-y: auto as a style to this div
+ // To override it, we need to use !important
+ overflow: visible !important;
#controlSections {
- min-height: 100%;
+ height: 100%;
overflow: visible;
}
.nav-tabs {
@@ -105,15 +134,22 @@ const Styles = styled.div`
`;
const ControlPanelsTabs = styled(Tabs)`
- .ant-tabs-nav-list {
- width: ${({ fullWidth }) => (fullWidth ? '100%' : '50%')};
- }
- .ant-tabs-content-holder {
- overflow: visible;
- }
- .ant-tabs-tabpane {
+ ${({ theme, fullWidth }) => css`
height: 100%;
- }
+ overflow: visible;
+ .ant-tabs-nav {
+ margin-bottom: 0;
+ }
+ .ant-tabs-nav-list {
+ width: ${fullWidth ? '100%' : '50%'};
+ }
+ .ant-tabs-tabpane {
+ height: 100%;
+ }
+ .ant-tabs-content-holder {
+ padding-top: ${theme.gridUnit * 4}px;
+ }
+ `}
`;
const isTimeSection = (section: ControlPanelSectionConfig): boolean =>
@@ -350,7 +386,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
box-shadow: none;
&:last-child {
- padding-bottom: ${theme.gridUnit * 10}px;
+ padding-bottom: ${theme.gridUnit * 16}px;
}
.panel-body {
@@ -432,6 +468,32 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
[handleClearFormClick, handleContinueClick, hasControlsTransferred],
);
+ const dataTabTitle = useMemo(
+ () => (
+ <>
+ <span>{t('Data')}</span>
+ {props.errorMessage && (
+ <span
+ css={(theme: SupersetTheme) => css`
+ font-size: ${theme.typography.sizes.xs}px;
+ margin-left: ${theme.gridUnit * 2}px;
+ `}
+ >
+ {' '}
+ <Tooltip
+ id="query-error-tooltip"
+ placement="right"
+ title={props.errorMessage}
+ >
+ <i className="fa fa-exclamation-circle text-danger fa-lg" />
+ </Tooltip>
+ </span>
+ )}
+ </>
+ ),
+ [props.errorMessage],
+ );
+
const controlPanelRegistry = getChartControlPanelRegistry();
if (
!controlPanelRegistry.has(props.form_data.viz_type) &&
@@ -448,8 +510,9 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
id="controlSections"
data-test="control-tabs"
fullWidth={showCustomizeTab}
+ allowOverflow={false}
>
- <Tabs.TabPane key="query" tab={t('Data')}>
+ <Tabs.TabPane key="query" tab={dataTabTitle}>
<Collapse
bordered
defaultActiveKey={expandedQuerySections}
@@ -475,6 +538,17 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
</Tabs.TabPane>
)}
</ControlPanelsTabs>
+ <div css={actionButtonsContainerStyles}>
+ <RunQueryButton
+ onQuery={props.onQuery}
+ onStop={props.onStop}
+ errorMessage={props.errorMessage}
+ loading={props.chart.chartStatus === 'loading'}
+ isNewChart={!props.chart.queriesResponse}
+ canStopQuery={props.canStopQuery}
+ chartIsStale={props.chartIsStale}
+ />
+ </div>
</Styles>
);
};
diff --git a/superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/index.jsx b/superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/index.jsx
index 640912d694..f02ab01622 100644
--- a/superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/index.jsx
+++ b/superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/index.jsx
@@ -86,8 +86,8 @@ const MenuItemWithCheckboxContainer = styled.div`
const MenuTrigger = styled(Button)`
${({ theme }) => css`
- width: ${theme.gridUnit * 6}px;
- height: ${theme.gridUnit * 6}px;
+ width: ${theme.gridUnit * 8}px;
+ height: ${theme.gridUnit * 8}px;
padding: 0;
border: 1px solid ${theme.colors.primary.dark2};
@@ -425,7 +425,7 @@ const ExploreAdditionalActionsMenu = ({
>
<Icons.MoreHoriz
iconColor={theme.colors.primary.dark2}
- iconSize={theme.typography.sizes.m}
+ iconSize="l"
/>
</MenuTrigger>
</AntdDropdown>
diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx
index 3c90d4650d..8f298dce76 100644
--- a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx
+++ b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx
@@ -90,6 +90,7 @@ const createProps = () => ({
user: {
userId: 1,
},
+ onSaveChart: jest.fn(),
});
test('Cancelling changes to the properties should reset previous properties', () => {
@@ -115,3 +116,17 @@ test('Cancelling changes to the properties should reset previous properties', ()
expect(screen.getByDisplayValue(prevChartName)).toBeInTheDocument();
});
+
+test('Save chart', () => {
+ const props = createProps();
+ render(<ExploreHeader {...props} />, { useRedux: true });
+ userEvent.click(screen.getByText('Save'));
+ expect(props.onSaveChart).toHaveBeenCalled();
+});
+
+test('Save disabled', () => {
+ const props = createProps();
+ render(<ExploreHeader {...props} saveDisabled />, { useRedux: true });
+ userEvent.click(screen.getByText('Save'));
+ expect(props.onSaveChart).not.toHaveBeenCalled();
+});
diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx b/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
index bbaa34648b..1be9dd77a2 100644
--- a/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
+++ b/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
@@ -24,7 +24,6 @@ import {
CategoricalColorNamespace,
css,
SupersetClient,
- styled,
t,
} from '@superset-ui/core';
import {
@@ -36,9 +35,12 @@ import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import { chartPropShape } from 'src/dashboard/util/propShapes';
import AlteredSliceTag from 'src/components/AlteredSliceTag';
import FaveStar from 'src/components/FaveStar';
+import Button from 'src/components/Button';
+import Icons from 'src/components/Icons';
import PropertiesModal from 'src/explore/components/PropertiesModal';
import { sliceUpdated } from 'src/explore/actions/exploreActions';
import CertifiedBadge from 'src/components/CertifiedBadge';
+import { Tooltip } from 'src/components/Tooltip';
import ExploreAdditionalActionsMenu from '../ExploreAdditionalActionsMenu';
import { ChartEditableTitle } from './ChartEditableTitle';
@@ -55,60 +57,58 @@ const propTypes = {
ownState: PropTypes.object,
timeout: PropTypes.number,
chart: chartPropShape,
+ saveDisabled: PropTypes.bool,
};
-const StyledHeader = styled.div`
- ${({ theme }) => css`
+const saveButtonStyles = theme => css`
+ color: ${theme.colors.primary.dark2};
+ & > span[role='img'] {
+ margin-right: 0;
+ }
+`;
+
+const headerStyles = theme => css`
+ display: flex;
+ flex-direction: row;
+ align-items: center;
+ flex-wrap: nowrap;
+ justify-content: space-between;
+ height: 100%;
+
+ span[role='button'] {
display: flex;
- flex-direction: row;
- align-items: center;
- flex-wrap: nowrap;
- justify-content: space-between;
height: 100%;
+ }
- span[role='button'] {
- display: flex;
- height: 100%;
- }
+ .title-panel {
+ display: flex;
+ align-items: center;
+ min-width: 0;
+ margin-right: ${theme.gridUnit * 12}px;
+ }
- .title-panel {
- display: flex;
- align-items: center;
- min-width: 0;
- margin-right: ${theme.gridUnit * 12}px;
- }
+ .right-button-panel {
+ display: flex;
+ align-items: center;
+ }
+`;
- .right-button-panel {
- display: flex;
- align-items: center;
+const buttonsStyles = theme => css`
+ display: flex;
+ align-items: center;
+ padding-left: ${theme.gridUnit * 2}px;
- > .btn-group {
- flex: 0 0 auto;
- margin-left: ${theme.gridUnit}px;
- }
- }
+ & .fave-unfave-icon {
+ padding: 0 ${theme.gridUnit}px;
- .action-button {
- color: ${theme.colors.grayscale.base};
- margin: 0 ${theme.gridUnit * 1.5}px 0 ${theme.gridUnit}px;
+ &:first-child {
+ padding-left: 0;
}
- `}
+ }
`;
-const StyledButtons = styled.span`
- ${({ theme }) => css`
- display: flex;
- align-items: center;
- padding-left: ${theme.gridUnit * 2}px;
-
- & .fave-unfave-icon {
- padding: 0 ${theme.gridUnit}px;
-
- &:first-child {
- padding-left: 0;
- }
- }
- `}
+const saveButtonContainerStyles = theme => css`
+ margin-right: ${theme.gridUnit * 2}px;
`;
export class ExploreChartHeader extends React.PureComponent {
@@ -231,11 +231,13 @@ export class ExploreChartHeader extends React.PureComponent {
isStarred,
sliceUpdated,
sliceName,
+ onSaveChart,
+ saveDisabled,
} = this.props;
const { latestQueryFormData, sliceFormData } = chart;
const oldSliceName = slice?.slice_name;
return (
- <StyledHeader id="slice-header">
+ <div id="slice-header" css={headerStyles}>
<div className="title-panel">
<ChartEditableTitle
title={sliceName}
@@ -248,7 +250,7 @@ export class ExploreChartHeader extends React.PureComponent {
placeholder={t('Add the name of the chart')}
/>
{slice && (
- <StyledButtons>
+ <span css={buttonsStyles}>
{slice.certified_by && (
<CertifiedBadge
certifiedBy={slice.certified_by}
@@ -279,10 +281,31 @@ export class ExploreChartHeader extends React.PureComponent {
currentFormData={{ ...formData, chartTitle: sliceName }}
/>
)}
- </StyledButtons>
+ </span>
)}
</div>
<div className="right-button-panel">
+ <Tooltip
+ title={
+ saveDisabled
+ ? t('Add required control values to save chart')
+ : null
+ }
+ >
+ {/* needed to wrap button in a div - antd tooltip doesn't work with disabled button */}
+ <div css={saveButtonContainerStyles}>
+ <Button
+ buttonStyle="secondary"
+ onClick={onSaveChart}
+ disabled={saveDisabled}
+ data-test="query-save-button"
+ css={saveButtonStyles}
+ >
+ <Icons.SaveOutlined iconSize="l" />
+ {t('Save')}
+ </Button>
+ </div>
+ </Tooltip>
<ExploreAdditionalActionsMenu
onOpenInEditor={actions.redirectSQLLab}
onOpenPropertiesModal={this.openPropertiesModal}
@@ -292,7 +315,7 @@ export class ExploreChartHeader extends React.PureComponent {
canAddReports={this.canAddReports()}
/>
</div>
- </StyledHeader>
+ </div>
);
}
}
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
index da815ca7dc..a240578c49 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
@@ -18,6 +18,7 @@
*/
import React from 'react';
import fetchMock from 'fetch-mock';
+import { getChartControlPanelRegistry } from '@superset-ui/core';
import { MemoryRouter, Route } from 'react-router-dom';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
@@ -111,13 +112,17 @@ test('generates a different form_data param when one is provided and is mounting
});
test('reuses the same form_data param when updating', async () => {
+ getChartControlPanelRegistry().registerValue('table', {
+ controlPanelSections: [],
+ });
const replaceState = jest.spyOn(window.history, 'replaceState');
const pushState = jest.spyOn(window.history, 'pushState');
await waitFor(() => renderWithRouter());
expect(replaceState.mock.calls.length).toBe(1);
- userEvent.click(screen.getByText('Run'));
+ userEvent.click(screen.getByText('Update chart'));
await waitFor(() => expect(pushState.mock.calls.length).toBe(1));
expect(replaceState.mock.calls[0]).toEqual(pushState.mock.calls[0]);
replaceState.mockRestore();
pushState.mockRestore();
+ getChartControlPanelRegistry().remove('table');
});
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
index d789b350aa..b856ba706c 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
@@ -48,7 +48,6 @@ import { useTabId } from 'src/hooks/useTabId';
import ExploreChartPanel from '../ExploreChartPanel';
import ConnectedControlPanelsContainer from '../ControlPanelsContainer';
import SaveModal from '../SaveModal';
-import QueryAndSaveBtns from '../QueryAndSaveBtns';
import DataSourcePanel from '../DatasourcePanel';
import { mountExploreUrl } from '../../exploreUtils';
import { areObjectsEqual } from '../../../reduxUtils';
@@ -477,8 +476,7 @@ function ExploreViewContainer(props) {
props.actions.logEvent(LOG_ACTIONS_CHANGE_EXPLORE_CONTROLS);
}
- function renderErrorMessage() {
- // Returns an error message as a node if any errors are in the store
+ const errorMessage = useMemo(() => {
const controlsWithErrors = Object.values(props.controls).filter(
control =>
control.validationErrors && control.validationErrors.length > 0,
@@ -512,7 +510,7 @@ function ExploreViewContainer(props) {
errorMessage = <div style={{ textAlign: 'left' }}>{errors}</div>;
}
return errorMessage;
- }
+ }, [props.controls]);
function renderChartContainer() {
return (
@@ -520,7 +518,7 @@ function ExploreViewContainer(props) {
width={width}
height={height}
{...props}
- errorMessage={renderErrorMessage()}
+ errorMessage={errorMessage}
refreshOverlayVisible={chartIsStale}
onQuery={onQuery}
/>
@@ -558,6 +556,8 @@ function ExploreViewContainer(props) {
chart={props.chart}
user={props.user}
reports={props.reports}
+ onSaveChart={toggleModal}
+ saveDisabled={errorMessage || props.chart.chartStatus === 'loading'}
/>
</ExploreHeaderContainer>
<ExplorePanelContainer id="explore-container">
@@ -669,16 +669,6 @@ function ExploreViewContainer(props) {
enable={{ right: true }}
className="col-sm-3 explore-column controls-column"
>
- <QueryAndSaveBtns
- canAdd={!!(props.can_add || props.can_overwrite)}
- onQuery={onQuery}
- onSave={toggleModal}
- onStop={onStop}
- loading={props.chart.chartStatus === 'loading'}
- chartIsStale={chartIsStale}
- errorMessage={renderErrorMessage()}
- datasourceType={props.datasource_type}
- />
<ConnectedControlPanelsContainer
exploreState={props.exploreState}
actions={props.actions}
@@ -687,6 +677,11 @@ function ExploreViewContainer(props) {
chart={props.chart}
datasource_type={props.datasource_type}
isDatasourceMetaLoading={props.isDatasourceMetaLoading}
+ onQuery={onQuery}
+ onStop={onStop}
+ canStopQuery={props.can_add || props.can_overwrite}
+ errorMessage={errorMessage}
+ chartIsStale={chartIsStale}
/>
</Resizable>
<div
diff --git a/superset-frontend/src/explore/components/QueryAndSaveBtns.test.jsx b/superset-frontend/src/explore/components/QueryAndSaveBtns.test.jsx
deleted file mode 100644
index 12e1a95e8e..0000000000
--- a/superset-frontend/src/explore/components/QueryAndSaveBtns.test.jsx
+++ /dev/null
@@ -1,60 +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 { styledMount as mount } from 'spec/helpers/theming';
-import sinon from 'sinon';
-
-import QueryAndSaveButtons from 'src/explore/components/QueryAndSaveBtns';
-import Button from 'src/components/Button';
-
-describe('QueryAndSaveButtons', () => {
- const defaultProps = {
- canAdd: true,
- onQuery: sinon.spy(),
- };
-
- // It must render
- it('renders', () => {
- expect(
- React.isValidElement(<QueryAndSaveButtons {...defaultProps} />),
- ).toBe(true);
- });
-
- // Test the output
- describe('output', () => {
- const wrapper = mount(<QueryAndSaveButtons {...defaultProps} />);
-
- it('renders 2 buttons', () => {
- expect(wrapper.find(Button)).toHaveLength(2);
- });
-
- it('renders buttons with correct text', () => {
- expect(wrapper.find(Button).at(0).text().trim()).toBe('Run');
- expect(wrapper.find(Button).at(1).text().trim()).toBe('Save');
- });
-
- it('calls onQuery when query button is clicked', () => {
- const queryButton = wrapper
- .find('[data-test="run-query-button"]')
- .hostNodes();
- queryButton.simulate('click');
- expect(defaultProps.onQuery.called).toBe(true);
- });
- });
-});
diff --git a/superset-frontend/src/explore/components/QueryAndSaveBtns.tsx b/superset-frontend/src/explore/components/QueryAndSaveBtns.tsx
deleted file mode 100644
index 91366580b3..0000000000
--- a/superset-frontend/src/explore/components/QueryAndSaveBtns.tsx
+++ /dev/null
@@ -1,124 +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 ButtonGroup from 'src/components/ButtonGroup';
-import { t, useTheme } from '@superset-ui/core';
-
-import { Tooltip } from 'src/components/Tooltip';
-import Button, { ButtonStyle, OnClickHandler } from 'src/components/Button';
-
-export type QueryAndSaveBtnsProps = {
- canAdd: boolean;
- onQuery: OnClickHandler;
- onSave: OnClickHandler;
- onStop: OnClickHandler;
- loading?: boolean;
- chartIsStale?: boolean;
- errorMessage: React.ReactElement | undefined;
-};
-
-export default function QueryAndSaveBtns(props: QueryAndSaveBtnsProps) {
- const {
- canAdd,
- onQuery = () => {},
- onSave = () => {},
- onStop = () => {},
- loading,
- chartIsStale,
- errorMessage,
- } = props;
- let qryButtonStyle: ButtonStyle = 'tertiary';
- if (errorMessage) {
- qryButtonStyle = 'danger';
- } else if (chartIsStale) {
- qryButtonStyle = 'primary';
- }
-
- const saveButtonDisabled = errorMessage ? true : loading;
- const qryOrStopButton = loading ? (
- <Button
- onClick={onStop}
- buttonStyle="warning"
- buttonSize="small"
- disabled={!canAdd}
- >
- <i className="fa fa-stop-circle-o" /> {t('Stop')}
- </Button>
- ) : (
- <Button
- buttonSize="small"
- onClick={onQuery}
- buttonStyle={qryButtonStyle}
- disabled={!!errorMessage}
- data-test="run-query-button"
- >
- <i className="fa fa-bolt" /> {t('Run')}
- </Button>
- );
-
- const theme = useTheme();
-
- return (
- <div
- css={{
- display: 'flex',
- flexShrink: 0,
- flexDirection: 'row',
- alignItems: 'center',
- paddingTop: theme.gridUnit * 2,
- paddingRight: theme.gridUnit * 2,
- paddingBottom: 0,
- paddingLeft: theme.gridUnit * 4,
- '& button': {
- width: 100,
- },
- '.errMsg': {
- marginLeft: theme.gridUnit * 4,
- },
- }}
- >
- <ButtonGroup className="query-and-save">
- {qryOrStopButton}
- <Button
- buttonStyle="tertiary"
- buttonSize="small"
- data-target="#save_modal"
- data-toggle="modal"
- disabled={saveButtonDisabled}
- onClick={onSave}
- data-test="query-save-button"
- >
- <i className="fa fa-plus-circle" /> {t('Save')}
- </Button>
- </ButtonGroup>
- {errorMessage && (
- <span className="errMsg">
- {' '}
- <Tooltip
- id="query-error-tooltip"
- placement="right"
- title={errorMessage}
- >
- <i className="fa fa-exclamation-circle text-danger fa-lg" />
- </Tooltip>
- </span>
- )}
- </div>
- );
-}
diff --git a/superset-frontend/src/explore/components/QueryAndSaveBtns.stories.tsx b/superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.stories.tsx
similarity index 69%
rename from superset-frontend/src/explore/components/QueryAndSaveBtns.stories.tsx
rename to superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.stories.tsx
index 1f8d999107..98c36bed22 100644
--- a/superset-frontend/src/explore/components/QueryAndSaveBtns.stories.tsx
+++ b/superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.stories.tsx
@@ -17,29 +17,31 @@
* under the License.
*/
import React from 'react';
-import QueryAndSaveBtns, { QueryAndSaveBtnsProps } from './QueryAndSaveBtns';
+import { RunQueryButton, RunQueryButtonProps } from '.';
export default {
- title: 'QueryAndSaveBtns',
- component: QueryAndSaveBtns,
+ title: 'RunQueryButton',
+ component: RunQueryButton,
};
-export const InteractiveQueryAndSaveBtnsProps = (
- args: QueryAndSaveBtnsProps,
-) => <QueryAndSaveBtns {...args} />;
+export const InteractiveRunQueryButtonProps = (args: RunQueryButtonProps) => (
+ <RunQueryButton {...args} />
+);
-InteractiveQueryAndSaveBtnsProps.args = {
- canAdd: true,
+InteractiveRunQueryButtonProps.args = {
+ canStopQuery: true,
loading: false,
+ errorMessage: null,
+ isNewChart: false,
+ chartIsStale: true,
};
-InteractiveQueryAndSaveBtnsProps.argTypes = {
+InteractiveRunQueryButtonProps.argTypes = {
onQuery: { action: 'onQuery' },
- onSave: { action: 'onSave' },
onStop: { action: 'onStop' },
};
-InteractiveQueryAndSaveBtnsProps.story = {
+InteractiveRunQueryButtonProps.story = {
parameters: {
knobs: {
disable: true,
diff --git a/superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.test.tsx b/superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.test.tsx
new file mode 100644
index 0000000000..2e298f5c02
--- /dev/null
+++ b/superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.test.tsx
@@ -0,0 +1,76 @@
+/**
+ * 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 userEvent from '@testing-library/user-event';
+import { render, screen } from 'spec/helpers/testing-library';
+import { RunQueryButton } from './index';
+
+const createProps = (overrides: Record<string, any> = {}) => ({
+ loading: false,
+ onQuery: jest.fn(),
+ onStop: jest.fn(),
+ errorMessage: null,
+ isNewChart: false,
+ canStopQuery: true,
+ chartIsStale: true,
+ ...overrides,
+});
+
+test('renders update chart button', () => {
+ const props = createProps();
+ render(<RunQueryButton {...props} />);
+ expect(screen.getByText('Update chart')).toBeVisible();
+ userEvent.click(screen.getByRole('button'));
+ expect(props.onQuery).toHaveBeenCalled();
+});
+
+test('renders create chart button', () => {
+ const props = createProps({ isNewChart: true });
+ render(<RunQueryButton {...props} />);
+ expect(screen.getByText('Create chart')).toBeVisible();
+ userEvent.click(screen.getByRole('button'));
+ expect(props.onQuery).toHaveBeenCalled();
+});
+
+test('renders disabled button', () => {
+ const props = createProps({ errorMessage: 'error' });
+ render(<RunQueryButton {...props} />);
+ expect(screen.getByText('Update chart')).toBeVisible();
+ expect(screen.getByRole('button')).toBeDisabled();
+ userEvent.click(screen.getByRole('button'));
+ expect(props.onQuery).not.toHaveBeenCalled();
+});
+
+test('renders query running button', () => {
+ const props = createProps({ loading: true });
+ render(<RunQueryButton {...props} />);
+ expect(screen.getByText('Stop')).toBeVisible();
+ userEvent.click(screen.getByRole('button'));
+ expect(props.onStop).toHaveBeenCalled();
+});
+
+test('renders query running button disabled', () => {
+ const props = createProps({ loading: true, canStopQuery: false });
+ render(<RunQueryButton {...props} />);
+ expect(screen.getByText('Stop')).toBeVisible();
+ expect(screen.getByRole('button')).toBeDisabled();
+ userEvent.click(screen.getByRole('button'));
+ expect(props.onStop).not.toHaveBeenCalled();
+});
diff --git a/superset-frontend/src/explore/components/RunQueryButton/index.tsx b/superset-frontend/src/explore/components/RunQueryButton/index.tsx
new file mode 100644
index 0000000000..622cb516f0
--- /dev/null
+++ b/superset-frontend/src/explore/components/RunQueryButton/index.tsx
@@ -0,0 +1,56 @@
+/**
+ * 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, { ReactNode } from 'react';
+import { t } from '@superset-ui/core';
+import Button from 'src/components/Button';
+
+export type RunQueryButtonProps = {
+ loading: boolean;
+ onQuery: () => void;
+ onStop: () => void;
+ errorMessage: ReactNode;
+ isNewChart: boolean;
+ canStopQuery: boolean;
+ chartIsStale: boolean;
+};
+
+export const RunQueryButton = ({
+ loading,
+ onQuery,
+ onStop,
+ errorMessage,
+ isNewChart,
+ canStopQuery,
+ chartIsStale,
+}: RunQueryButtonProps) =>
+ loading ? (
+ <Button onClick={onStop} buttonStyle="warning" disabled={!canStopQuery}>
+ <i className="fa fa-stop-circle-o" /> {t('Stop')}
+ </Button>
+ ) : (
+ <Button
+ onClick={onQuery}
+ buttonStyle={chartIsStale ? 'primary' : 'secondary'}
+ disabled={!!errorMessage}
+ data-test="run-query-button"
+ >
+ {isNewChart ? t('Create chart') : t('Update chart')}
+ </Button>
+ );