Skip to content

Commit

Permalink
feat(explore): Redesign of Run/Save buttons (apache#19558)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
kgabryje authored and philipher29 committed Jun 9, 2022
1 parent fe026b3 commit 029757d
Show file tree
Hide file tree
Showing 13 changed files with 365 additions and 282 deletions.
23 changes: 20 additions & 3 deletions superset-frontend/src/components/Chart/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@
*/
import PropTypes from 'prop-types';
import React from 'react';
import { logging, styled, t } from '@superset-ui/core';
import { styled, logging, t, ensureIsArray } from '@superset-ui/core';

import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
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';
import { LOG_ACTIONS_RENDER_CHART, Logger } from 'src/logger/LogUtils';
import { Logger, LOG_ACTIONS_RENDER_CHART } from 'src/logger/LogUtils';
import { URL_PARAMS } from 'src/constants';
import { getUrlParam } from 'src/utils/urlUtils';
import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
Expand Down Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
ControlPanelsContainerProps,
} from 'src/explore/components/ControlPanelsContainer';

describe('ControlPanelsContainer2', () => {
describe('ControlPanelsContainer', () => {
beforeAll(() => {
getChartControlPanelRegistry().registerValue('table', {
controlPanelSections: [
Expand Down Expand Up @@ -90,6 +90,10 @@ describe('ControlPanelsContainer2', () => {
form_data: getFormDataFromControls(controls),
isDatasourceMetaLoading: false,
exploreState: {},
chart: {
queriesResponse: null,
chartStatus: 'success',
},
} as ControlPanelsContainerProps;
}

Expand Down
108 changes: 91 additions & 17 deletions superset-frontend/src/explore/components/ControlPanelsContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
/* eslint camelcase: 0 */
import React, {
ReactNode,
useCallback,
useContext,
useEffect,
Expand All @@ -26,13 +27,14 @@ import React, {
useState,
} from 'react';
import {
css,
DatasourceType,
ensureIsArray,
t,
styled,
getChartControlPanelRegistry,
QueryFormData,
styled,
t,
DatasourceType,
css,
SupersetTheme,
} from '@superset-ui/core';
import {
ControlPanelSectionConfig,
Expand All @@ -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'];
Expand All @@ -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<
Expand All @@ -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 {
Expand All @@ -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 =>
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) &&
Expand All @@ -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}
Expand All @@ -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>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -425,7 +425,7 @@ const ExploreAdditionalActionsMenu = ({
>
<Icons.MoreHoriz
iconColor={theme.colors.primary.dark2}
iconSize={theme.typography.sizes.m}
iconSize="l"
/>
</MenuTrigger>
</AntdDropdown>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ const createProps = () => ({
user: {
userId: 1,
},
onSaveChart: jest.fn(),
});

test('Cancelling changes to the properties should reset previous properties', () => {
Expand All @@ -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();
});
Loading

0 comments on commit 029757d

Please sign in to comment.