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/05/13 17:07:25 UTC
[superset] branch master updated: feat(dashboard): Rearrange items in chart header controls dropdown (#20049)
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 3043a54bfc feat(dashboard): Rearrange items in chart header controls dropdown (#20049)
3043a54bfc is described below
commit 3043a54bfc1198bdef8128ec88c8e571d9640cff
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Fri May 13 19:07:19 2022 +0200
feat(dashboard): Rearrange items in chart header controls dropdown (#20049)
* feat(dashboard): Rearrange items in chart header controls dropdown
* Fix download as image and tests
---
.../src/dashboard/components/SliceHeader/index.tsx | 1 +
.../SliceHeaderControls.test.tsx | 83 +++++++--------
.../components/SliceHeaderControls/index.tsx | 117 +++++++++++++--------
.../dashboard/components/gridComponents/Chart.jsx | 2 +-
4 files changed, 115 insertions(+), 88 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx
index 44ddf7b5ad..3aba641fcb 100644
--- a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx
+++ b/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}
/>
diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx
index 945cb00252..7806a91e9b 100644
--- a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx
+++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx
@@ -125,16 +125,22 @@ 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' }),
@@ -142,82 +148,67 @@ test('Should render default props', () => {
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);
});
diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
index 56b1801874..e9a940e5f4 100644
--- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
+++ b/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`}
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
index b212af44e5..31c8246b19 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
+++ b/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}