feat(explore): Redesign of Run/Save buttons (apache#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

@@ -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';
@@ -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}

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;
   }
 

superset-frontend/src/explore/components/ControlPanelsContainer.tsx

@@ -18,6 +18,7 @@
  */
 /* eslint camelcase: 0 */
 import React, {
+  ReactNode,
   useCallback,
   useContext,
   useEffect,
@@ -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,
@@ -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>
   );
 };

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>

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();
+});