Skip to content

Commit

Permalink
feat(dashboard): Rearrange items in chart header controls dropdown (#…
Browse files Browse the repository at this point in the history
…20049)

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

* Fix download as image and tests
  • Loading branch information
kgabryje authored May 13, 2022
1 parent 21c5b26 commit 3043a54
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ const SliceHeader: FC<SliceHeaderProps> = ({
addDangerToast={addDangerToast}
handleToggleFullSize={handleToggleFullSize}
isFullSize={isFullSize}
isDescriptionExpanded={isExpanded}
chartStatus={chartStatus}
formData={formData}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand All @@ -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);
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import React from 'react';
import moment from 'moment';
import {
Behavior,
css,
getChartMetadataRegistry,
QueryFormData,
styled,
Expand All @@ -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',
};
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -99,6 +101,7 @@ export interface SliceHeaderControlsProps {
isExpanded?: boolean;
updatedDttm: number | null;
isFullSize?: boolean;
isDescriptionExpanded?: boolean;
formData: Pick<QueryFormData, 'slice_id' | 'datasource'>;
onExploreChart: () => void;

Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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';
Expand Down Expand Up @@ -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}
Expand All @@ -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>
)}

Expand All @@ -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>
)}

Expand All @@ -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>
);
Expand All @@ -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`}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down

0 comments on commit 3043a54

Please sign in to comment.