feat(dashboard): Rearrange items in chart header controls dropdown

superset-frontend/src/dashboard/components/SliceHeader/index.tsx

@@ -205,6 +205,7 @@ const SliceHeader: FC<SliceHeaderProps> = ({
  addDangerToast={addDangerToast}
  handleToggleFullSize={handleToggleFullSize}
  isFullSize={isFullSize}
+ isDescriptionExpanded={isExpanded}
  chartStatus={chartStatus}
  formData={formData}
  />

superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx

@@ -125,99 +125,90 @@ test('Should render default props', () => {
  delete props.sliceCanEdit;
 
  render(<SliceHeaderControls {...props} />, { useRedux: true });
- userEvent.click(screen.getByRole('menuitem', { name: 'Maximize chart' }));
- userEvent.click(screen.getByRole('menuitem', { name: /Force refresh/ }));
- userEvent.click(
- screen.getByRole('menuitem', { name: 'Toggle chart description' }),
- );
- userEvent.click(
- screen.getByRole('menuitem', { name: 'View chart in Explore' }),
- );
- userEvent.click(screen.getByRole('menuitem', { name: 'Export CSV' }));
- userEvent.click(screen.getByRole('menuitem', { name: /Force refresh/ }));
+ expect(
+ screen.getByRole('menuitem', { name: 'Enter fullscreen' }),
+ ).toBeInTheDocument();
+ expect(
+ screen.getByRole('menuitem', { name: /Force refresh/ }),
+ ).toBeInTheDocument();
+ expect(
+ screen.getByRole('menuitem', { name: 'Show chart description' }),
+ ).toBeInTheDocument();
+ expect(
+ screen.getByRole('menuitem', { name: 'Edit chart' }),
+ ).toBeInTheDocument();
+ expect(
+ screen.getByRole('menuitem', { name: 'Download' }),
+ ).toBeInTheDocument();
+ expect(screen.getByRole('menuitem', { name: 'Share' })).toBeInTheDocument();
 
  expect(
  screen.getByRole('button', { name: 'More Options' }),
  ).toBeInTheDocument();
  expect(screen.getByTestId('NoAnimationDropdown')).toBeInTheDocument();
 });
 
-test('Should "export to CSV"', () => {
+test('Should "export to CSV"', async () => {
  const props = createProps();
  render(<SliceHeaderControls {...props} />, { useRedux: true });
 
  expect(props.exportCSV).toBeCalledTimes(0);
- userEvent.click(screen.getByRole('menuitem', { name: 'Export CSV' }));
+ userEvent.hover(screen.getByText('Download'));
+ userEvent.click(await screen.findByText('Export to .CSV'));
  expect(props.exportCSV).toBeCalledTimes(1);
  expect(props.exportCSV).toBeCalledWith(371);
 });
 
-test('Should not show "Export to CSV" if slice is filter box', () => {
+test('Should not show "Download" if slice is filter box', () => {
  const props = createProps('filter_box');
  render(<SliceHeaderControls {...props} />, { useRedux: true });
- expect(screen.queryByRole('menuitem', { name: 'Export CSV' })).toBe(null);
+ expect(screen.queryByText('Download')).not.toBeInTheDocument();
 });
 
-test('Export full CSV is under featureflag', () => {
+test('Export full CSV is under featureflag', async () => {
  // @ts-ignore
  global.featureFlags = {
  [FeatureFlag.ALLOW_FULL_CSV_EXPORT]: false,
  };
  const props = createProps('table');
  render(<SliceHeaderControls {...props} />, { useRedux: true });
- expect(screen.queryByRole('menuitem', { name: 'Export full CSV' })).toBe(
-  null,
- );
+ userEvent.hover(screen.getByText('Download'));
+ expect(await screen.findByText('Export to .CSV')).toBeInTheDocument();
+ expect(screen.queryByText('Export to full .CSV')).not.toBeInTheDocument();
 });
 
-test('Should "export full CSV"', () => {
+test('Should "export full CSV"', async () => {
  // @ts-ignore
  global.featureFlags = {
  [FeatureFlag.ALLOW_FULL_CSV_EXPORT]: true,
  };
  const props = createProps('table');
  render(<SliceHeaderControls {...props} />, { useRedux: true });
- expect(screen.queryByRole('menuitem', { name: 'Export full CSV' })).not.toBe(
- null,
- );
  expect(props.exportFullCSV).toBeCalledTimes(0);
- userEvent.click(screen.getByRole('menuitem', { name: 'Export full CSV' }));
+ userEvent.hover(screen.getByText('Download'));
+ userEvent.click(await screen.findByText('Export to full .CSV'));
  expect(props.exportFullCSV).toBeCalledTimes(1);
  expect(props.exportFullCSV).toBeCalledWith(371);
 });
 
-test('Should not show export full CSV if report is not table', () => {
+test('Should not show export full CSV if report is not table', async () => {
  // @ts-ignore
  global.featureFlags = {
  [FeatureFlag.ALLOW_FULL_CSV_EXPORT]: true,
  };
  const props = createProps();
  render(<SliceHeaderControls {...props} />, { useRedux: true });
- expect(screen.queryByRole('menuitem', { name: 'Export full CSV' })).toBe(
- null,
- );
-});
-
-test('Should not show export full CSV if slice is filter box', () => {
- // @ts-ignore
- global.featureFlags = {
- [FeatureFlag.ALLOW_FULL_CSV_EXPORT]: true,
- };
- const props = createProps('filter_box');
- render(<SliceHeaderControls {...props} />, { useRedux: true });
- expect(screen.queryByRole('menuitem', { name: 'Export full CSV' })).toBe(
- null,
- );
+ userEvent.hover(screen.getByText('Download'));
+ expect(await screen.findByText('Export to .CSV')).toBeInTheDocument();
+ expect(screen.queryByText('Export to full .CSV')).not.toBeInTheDocument();
 });
 
-test('Should "Toggle chart description"', () => {
+test('Should "Show chart description"', () => {
  const props = createProps();
  render(<SliceHeaderControls {...props} />, { useRedux: true });
 
  expect(props.toggleExpandSlice).toBeCalledTimes(0);
- userEvent.click(
- screen.getByRole('menuitem', { name: 'Toggle chart description' }),
- );
+ userEvent.click(screen.getByText('Show chart description'));
  expect(props.toggleExpandSlice).toBeCalledTimes(1);
  expect(props.toggleExpandSlice).toBeCalledWith(371);
 });
@@ -227,17 +218,17 @@ test('Should "Force refresh"', () => {
  render(<SliceHeaderControls {...props} />, { useRedux: true });
 
  expect(props.forceRefresh).toBeCalledTimes(0);
- userEvent.click(screen.getByRole('menuitem', { name: /Force refresh/ }));
+ userEvent.click(screen.getByText('Force refresh'));
  expect(props.forceRefresh).toBeCalledTimes(1);
  expect(props.forceRefresh).toBeCalledWith(371, 26);
  expect(props.addSuccessToast).toBeCalledTimes(1);
 });
 
-test('Should "Maximize chart"', () => {
+test('Should "Enter fullscreen"', () => {
  const props = createProps();
  render(<SliceHeaderControls {...props} />, { useRedux: true });
 
  expect(props.handleToggleFullSize).toBeCalledTimes(0);
- userEvent.click(screen.getByRole('menuitem', { name: 'Maximize chart' }));
+ userEvent.click(screen.getByText('Enter fullscreen'));
  expect(props.handleToggleFullSize).toBeCalledTimes(1);
 });

superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx

@@ -20,6 +20,7 @@ import React from 'react';
 import moment from 'moment';
 import {
  Behavior,
+ css,
  getChartMetadataRegistry,
  QueryFormData,
  styled,
@@ -42,7 +43,7 @@ const MENU_KEYS = {
  EXPORT_CSV: 'export_csv',
  EXPORT_FULL_CSV: 'export_full_csv',
  FORCE_REFRESH: 'force_refresh',
- RESIZE_LABEL: 'resize_label',
+ FULLSCREEN: 'fullscreen',
  TOGGLE_CHART_DESCRIPTION: 'toggle_chart_description',
  VIEW_QUERY: 'view_query',
 };
@@ -71,7 +72,8 @@ const RefreshTooltip = styled.div`
  justify-content: flex-start;
 `;
 
-const SCREENSHOT_NODE_SELECTOR = '.dashboard-component-chart-holder';
+const getScreenshotNodeSelector = (chartId: string | number) =>
+ `.dashboard-chart-id-${chartId}`;
 
 const VerticalDotsTrigger = () => (
  <VerticalDotsContainer>
@@ -99,6 +101,7 @@ export interface SliceHeaderControlsProps {
  isExpanded?: boolean;
  updatedDttm: number | null;
  isFullSize?: boolean;
+ isDescriptionExpanded?: boolean;
  formData: Pick<QueryFormData, 'slice_id' | 'datasource'>;
  onExploreChart: () => void;
 
@@ -122,6 +125,13 @@ interface State {
  showCrossFilterScopingModal: boolean;
 }
 
+const dropdownIconsStyles = css`
+ &&.anticon > .anticon:first-child {
+ margin-right: 0;
+ vertical-align: 0;
+ }
+`;
+
 class SliceHeaderControls extends React.PureComponent<
  SliceHeaderControlsProps,
  State
@@ -182,7 +192,7 @@ class SliceHeaderControls extends React.PureComponent<
  // eslint-disable-next-line no-unused-expressions
  this.props.exportCSV && this.props.exportCSV(this.props.slice.slice_id);
  break;
- case MENU_KEYS.RESIZE_LABEL:
+ case MENU_KEYS.FULLSCREEN:
  this.props.handleToggleFullSize();
  break;
  case MENU_KEYS.EXPORT_FULL_CSV:
@@ -198,8 +208,10 @@ class SliceHeaderControls extends React.PureComponent<
  ) as HTMLElement;
  menu.style.visibility = 'hidden';
  downloadAsImage(
- SCREENSHOT_NODE_SELECTOR,
+ getScreenshotNodeSelector(this.props.slice.slice_id),
  this.props.slice.slice_name,
+ {},
+ true,
  // @ts-ignore
  )(domEvent).then(() => {
  menu.style.visibility = 'visible';
@@ -256,7 +268,9 @@ class SliceHeaderControls extends React.PureComponent<
  : item}
  </div>
  ));
- const resizeLabel = isFullSize ? t('Minimize chart') : t('Maximize chart');
+ const fullscreenLabel = isFullSize
+ ? t('Exit fullscreen')
+ : t('Enter fullscreen');
  const menu = (
  <Menu
  onClick={this.handleMenuClick}
@@ -275,11 +289,15 @@ class SliceHeaderControls extends React.PureComponent<
  </RefreshTooltip>
  </Menu.Item>
 
+ <Menu.Item key={MENU_KEYS.FULLSCREEN}>{fullscreenLabel}</Menu.Item>
+
  <Menu.Divider />
 
  {slice.description && (
  <Menu.Item key={MENU_KEYS.TOGGLE_CHART_DESCRIPTION}>
- {t('Toggle chart description')}
+ {this.props.isDescriptionExpanded
+ ? t('Hide chart description')
+ : t('Show chart description')}
  </Menu.Item>
  )}
 
@@ -288,7 +306,7 @@ class SliceHeaderControls extends React.PureComponent<
  key={MENU_KEYS.EXPLORE_CHART}
  onClick={this.props.onExploreChart}
  >
- {t('View chart in Explore')}
+ {t('Edit chart')}
  </Menu.Item>
  )}
 
@@ -309,45 +327,65 @@ class SliceHeaderControls extends React.PureComponent<
  </Menu.Item>
  )}
 
- {supersetCanShare && (
- <ShareMenuItems
- dashboardId={dashboardId}
- dashboardComponentId={componentId}
- copyMenuItemTitle={t('Copy permalink to clipboard')}
- emailMenuItemTitle={t('Share permalink by email')}
- emailSubject={t('Superset chart')}
- emailBody={t('Check out this chart: ')}
- addSuccessToast={addSuccessToast}
- addDangerToast={addDangerToast}
- />
+ {(slice.description || this.props.supersetCanExplore) && (
+ <Menu.Divider />
  )}
 
- <Menu.Item key={MENU_KEYS.RESIZE_LABEL}>{resizeLabel}</Menu.Item>
+ {isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) &&
+ isCrossFilter &&
+ canEmitCrossFilter && (
+ <>
+ <Menu.Item key={MENU_KEYS.CROSS_FILTER_SCOPING}>
+ {t('Cross-filter scoping')}
+ </Menu.Item>
+ <Menu.Divider />
+ </>
+ )}
 
- <Menu.Item key={MENU_KEYS.DOWNLOAD_AS_IMAGE}>
- {t('Download as image')}
- </Menu.Item>
+ {supersetCanShare && (
+ <Menu.SubMenu title={t('Share')}>
+ <ShareMenuItems
+ dashboardId={dashboardId}
+ dashboardComponentId={componentId}
+ copyMenuItemTitle={t('Copy permalink to clipboard')}
+ emailMenuItemTitle={t('Share chart by email')}
+ emailSubject={t('Superset chart')}
+ emailBody={t('Check out this chart: ')}
+ addSuccessToast={addSuccessToast}
+ addDangerToast={addDangerToast}
+ />
+ </Menu.SubMenu>
+ )}
 
  {this.props.slice.viz_type !== 'filter_box' &&
  this.props.supersetCanCSV && (
- <Menu.Item key={MENU_KEYS.EXPORT_CSV}>{t('Export CSV')}</Menu.Item>
- )}
+ <Menu.SubMenu title={t('Download')}>
+ <Menu.Item
+ key={MENU_KEYS.EXPORT_CSV}
+ icon={<Icons.FileOutlined css={dropdownIconsStyles} />}
+ >
+ {t('Export to .CSV')}
+ </Menu.Item>
 
- {this.props.slice.viz_type !== 'filter_box' &&
- isFeatureEnabled(FeatureFlag.ALLOW_FULL_CSV_EXPORT) &&
- this.props.supersetCanCSV &&
- isTable && (
- <Menu.Item key={MENU_KEYS.EXPORT_FULL_CSV}>
- {t('Export full CSV')}
- </Menu.Item>
- )}
+ {this.props.slice.viz_type !== 'filter_box' &&
+ isFeatureEnabled(FeatureFlag.ALLOW_FULL_CSV_EXPORT) &&
+ this.props.supersetCanCSV &&
+ isTable && (
+ <Menu.Item
+ key={MENU_KEYS.EXPORT_FULL_CSV}
+ icon={<Icons.FileOutlined css={dropdownIconsStyles} />}
+ >
+ {t('Export to full .CSV')}
+ </Menu.Item>
+ )}
 
- {isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) &&
- isCrossFilter &&
- canEmitCrossFilter && (
- <Menu.Item key={MENU_KEYS.CROSS_FILTER_SCOPING}>
- {t('Cross-filter scoping')}
- </Menu.Item>
+ <Menu.Item
+ key={MENU_KEYS.DOWNLOAD_AS_IMAGE}
+ icon={<Icons.FileImageOutlined css={dropdownIconsStyles} />}
+ >
+ {t('Download as image')}
+ </Menu.Item>
+ </Menu.SubMenu>
  )}
  </Menu>
  );
@@ -371,9 +409,6 @@ class SliceHeaderControls extends React.PureComponent<
  overlay={menu}
  trigger={['click']}
  placement="bottomRight"
- getPopupContainer={triggerNode =>
- triggerNode.closest(SCREENSHOT_NODE_SELECTOR) as HTMLElement
- }
  >
  <span
  id={`slice_${slice.slice_id}-controls`}

superset-frontend/src/dashboard/components/gridComponents/Chart.jsx

@@ -389,7 +389,7 @@ export default class Chart extends React.Component {
  <SliceHeader
  innerRef={this.setHeaderRef}
  slice={slice}
- isExpanded={!!isExpanded}
+ isExpanded={isExpanded}
  isCached={isCached}
  cachedDttm={cachedDttm}
  updatedDttm={chartUpdateEndTime}