From 1cd002e801caf88f89906fa745f6fb28a79eae7a Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 13 May 2022 15:36:18 +0200 Subject: [PATCH] chore: Create a generic header component for Explore and Dashboard (#20044) * chore: Create a generic header component for Explore and Dashboard * Add tests * Fix undefined error * Remove duplicate code * Fix cypress test --- .../visualizations/download_chart.test.js | 2 +- .../DynamicEditableTitle.test.tsx} | 11 +- .../DynamicEditableTitle}/index.tsx | 84 ++--- .../src/components/FaveStar/index.tsx | 2 +- .../PageHeaderWithActions.test.tsx | 57 ++++ .../PageHeaderWithActions/index.tsx | 152 +++++++++ .../HeaderReportDropdown/index.tsx | 2 +- superset-frontend/src/components/index.ts | 1 + .../ExploreAdditionalActionsMenu.test.tsx | 294 ----------------- .../ExploreChartHeader.test.tsx | 233 ++++++++++++- .../components/ExploreChartHeader/index.jsx | 270 ++++++--------- .../components/ExploreViewContainer/index.jsx | 2 - .../index.jsx | 311 ++++++++---------- 13 files changed, 733 insertions(+), 688 deletions(-) rename superset-frontend/src/{explore/components/ExploreChartHeader/ChartEditableTitle/ChartEditableTitle.test.tsx => components/DynamicEditableTitle/DynamicEditableTitle.test.tsx} (89%) rename superset-frontend/src/{explore/components/ExploreChartHeader/ChartEditableTitle => components/DynamicEditableTitle}/index.tsx (81%) create mode 100644 superset-frontend/src/components/PageHeaderWithActions/PageHeaderWithActions.test.tsx create mode 100644 superset-frontend/src/components/PageHeaderWithActions/index.tsx delete mode 100644 superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/ExploreAdditionalActionsMenu.test.tsx rename superset-frontend/src/explore/components/{ExploreAdditionalActionsMenu => useExploreAdditionalActionsMenu}/index.jsx (56%) diff --git a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/download_chart.test.js b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/download_chart.test.js index e8cc38c08863..42f9c13123bd 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/download_chart.test.js +++ b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/download_chart.test.js @@ -34,7 +34,7 @@ describe('Download Chart > Distribution bar chart', () => { }; cy.visitChartByParams(JSON.stringify(formData)); - cy.get('.right-button-panel > .ant-dropdown-trigger').click(); + cy.get('.right-button-panel .ant-dropdown-trigger').click(); cy.get(':nth-child(1) > .ant-dropdown-menu-submenu-title').click(); cy.get( '.ant-dropdown-menu-submenu > .ant-dropdown-menu li:nth-child(3)', diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/ChartEditableTitle/ChartEditableTitle.test.tsx b/superset-frontend/src/components/DynamicEditableTitle/DynamicEditableTitle.test.tsx similarity index 89% rename from superset-frontend/src/explore/components/ExploreChartHeader/ChartEditableTitle/ChartEditableTitle.test.tsx rename to superset-frontend/src/components/DynamicEditableTitle/DynamicEditableTitle.test.tsx index dd98518c8c41..95567d2948df 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader/ChartEditableTitle/ChartEditableTitle.test.tsx +++ b/superset-frontend/src/components/DynamicEditableTitle/DynamicEditableTitle.test.tsx @@ -19,20 +19,21 @@ import React from 'react'; import userEvent from '@testing-library/user-event'; import { render, screen } from 'spec/helpers/testing-library'; -import { ChartEditableTitle } from './index'; +import { DynamicEditableTitle } from '.'; const createProps = (overrides: Record = {}) => ({ title: 'Chart title', placeholder: 'Add the name of the chart', canEdit: true, onSave: jest.fn(), + label: 'Chart title', ...overrides, }); describe('Chart editable title', () => { it('renders chart title', () => { const props = createProps(); - render(); + render(); expect(screen.getByText('Chart title')).toBeVisible(); }); @@ -40,13 +41,13 @@ describe('Chart editable title', () => { const props = createProps({ title: '', }); - render(); + render(); expect(screen.getByText('Add the name of the chart')).toBeVisible(); }); it('click, edit and save title', () => { const props = createProps(); - render(); + render(); const textboxElement = screen.getByRole('textbox'); userEvent.click(textboxElement); userEvent.type(textboxElement, ' edited'); @@ -57,7 +58,7 @@ describe('Chart editable title', () => { it('renders in non-editable mode', () => { const props = createProps({ canEdit: false }); - render(); + render(); const titleElement = screen.getByLabelText('Chart title'); expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); expect(titleElement).toBeVisible(); diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/ChartEditableTitle/index.tsx b/superset-frontend/src/components/DynamicEditableTitle/index.tsx similarity index 81% rename from superset-frontend/src/explore/components/ExploreChartHeader/ChartEditableTitle/index.tsx rename to superset-frontend/src/components/DynamicEditableTitle/index.tsx index 0e2761b6a9de..969aea19b730 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader/ChartEditableTitle/index.tsx +++ b/superset-frontend/src/components/DynamicEditableTitle/index.tsx @@ -26,62 +26,62 @@ import React, { useRef, useState, } from 'react'; -import { css, styled, t } from '@superset-ui/core'; +import { css, SupersetTheme, t } from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; import { useResizeDetector } from 'react-resize-detector'; -export type ChartEditableTitleProps = { +export type DynamicEditableTitleProps = { title: string; placeholder: string; onSave: (title: string) => void; canEdit: boolean; + label: string | undefined; }; -const Styles = styled.div` - ${({ theme }) => css` - display: flex; - font-size: ${theme.typography.sizes.xl}px; - font-weight: ${theme.typography.weights.bold}; +const titleStyles = (theme: SupersetTheme) => css` + display: flex; + font-size: ${theme.typography.sizes.xl}px; + font-weight: ${theme.typography.weights.bold}; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + + & .dynamic-title, + & .dynamic-title-input { + display: inline-block; + max-width: 100%; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; - - & .chart-title, - & .chart-title-input { - display: inline-block; - max-width: 100%; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - } - - & .chart-title { - cursor: default; + } + + & .dynamic-title { + cursor: default; + } + & .dynamic-title-input { + border: none; + padding: 0; + outline: none; + + &::placeholder { + color: ${theme.colors.grayscale.light1}; } - & .chart-title-input { - border: none; - padding: 0; - outline: none; + } - &::placeholder { - color: ${theme.colors.grayscale.light1}; - } - } - - & .input-sizer { - position: absolute; - left: -9999px; - display: inline-block; - } - `} + & .input-sizer { + position: absolute; + left: -9999px; + display: inline-block; + } `; -export const ChartEditableTitle = ({ +export const DynamicEditableTitle = ({ title, placeholder, onSave, canEdit, -}: ChartEditableTitleProps) => { + label, +}: DynamicEditableTitleProps) => { const [isEditing, setIsEditing] = useState(false); const [currentTitle, setCurrentTitle] = useState(title || ''); const contentRef = useRef(null); @@ -170,7 +170,7 @@ export const ChartEditableTitle = ({ ); return ( - +
) : ( {currentTitle} @@ -208,6 +208,6 @@ export const ChartEditableTitle = ({ )} - +
); }; diff --git a/superset-frontend/src/components/FaveStar/index.tsx b/superset-frontend/src/components/FaveStar/index.tsx index 0473e3d9b238..8a6f4eca1f1e 100644 --- a/superset-frontend/src/components/FaveStar/index.tsx +++ b/superset-frontend/src/components/FaveStar/index.tsx @@ -23,7 +23,7 @@ import { Tooltip } from 'src/components/Tooltip'; import { useComponentDidMount } from 'src/hooks/useComponentDidMount'; import Icons from 'src/components/Icons'; -interface FaveStarProps { +export interface FaveStarProps { itemId: number; isStarred?: boolean; showTooltip?: boolean; diff --git a/superset-frontend/src/components/PageHeaderWithActions/PageHeaderWithActions.test.tsx b/superset-frontend/src/components/PageHeaderWithActions/PageHeaderWithActions.test.tsx new file mode 100644 index 000000000000..65158f3ac811 --- /dev/null +++ b/superset-frontend/src/components/PageHeaderWithActions/PageHeaderWithActions.test.tsx @@ -0,0 +1,57 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +import { render, screen } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import { PageHeaderWithActions, PageHeaderWithActionsProps } from './index'; +import { Menu } from '../Menu'; + +const defaultProps: PageHeaderWithActionsProps = { + editableTitleProps: { + title: 'Test title', + placeholder: 'Test placeholder', + onSave: jest.fn(), + canEdit: true, + label: 'Title', + }, + showTitlePanelItems: true, + certificatiedBadgeProps: {}, + showFaveStar: true, + faveStarProps: { itemId: 1, saveFaveStar: jest.fn() }, + titlePanelAdditionalItems: , + rightPanelAdditionalItems: , + additionalActionsMenu: ( + + Test menu item + + ), + menuDropdownProps: { onVisibleChange: jest.fn(), visible: true }, +}; + +test('Renders', async () => { + render(); + expect(screen.getByText('Test title')).toBeVisible(); + expect(screen.getByTestId('fave-unfave-icon')).toBeVisible(); + expect(screen.getByText('Title panel button')).toBeVisible(); + expect(screen.getByText('Save')).toBeVisible(); + + userEvent.click(screen.getByLabelText('Menu actions trigger')); + expect(defaultProps.menuDropdownProps.onVisibleChange).toHaveBeenCalled(); +}); diff --git a/superset-frontend/src/components/PageHeaderWithActions/index.tsx b/superset-frontend/src/components/PageHeaderWithActions/index.tsx new file mode 100644 index 000000000000..204a82b235d1 --- /dev/null +++ b/superset-frontend/src/components/PageHeaderWithActions/index.tsx @@ -0,0 +1,152 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React, { ReactNode, ReactElement } from 'react'; +import { css, SupersetTheme, t, useTheme } from '@superset-ui/core'; +import { AntdDropdown, AntdDropdownProps } from 'src/components'; +import { + DynamicEditableTitle, + DynamicEditableTitleProps, +} from '../DynamicEditableTitle'; +import CertifiedBadge, { CertifiedBadgeProps } from '../CertifiedBadge'; +import FaveStar, { FaveStarProps } from '../FaveStar'; +import Icons from '../Icons'; +import Button from '../Button'; + +export const menuTriggerStyles = (theme: SupersetTheme) => css` + width: ${theme.gridUnit * 8}px; + height: ${theme.gridUnit * 8}px; + padding: 0; + border: 1px solid ${theme.colors.primary.dark2}; + + &.ant-btn > span.anticon { + line-height: 0; + transition: inherit; + } + + &:hover:not(:focus) > span.anticon { + color: ${theme.colors.primary.light1}; + } +`; + +const headerStyles = (theme: SupersetTheme) => css` + display: flex; + flex-direction: row; + align-items: center; + flex-wrap: nowrap; + justify-content: space-between; + height: 100%; + + span[role='button'] { + display: flex; + height: 100%; + } + + .title-panel { + display: flex; + align-items: center; + min-width: 0; + margin-right: ${theme.gridUnit * 12}px; + } + + .right-button-panel { + display: flex; + align-items: center; + } +`; + +const buttonsStyles = (theme: SupersetTheme) => css` + display: flex; + align-items: center; + padding-left: ${theme.gridUnit * 2}px; + + & .fave-unfave-icon { + padding: 0 ${theme.gridUnit}px; + + &:first-child { + padding-left: 0; + } + } +`; + +const additionalActionsContainerStyles = (theme: SupersetTheme) => css` + margin-left: ${theme.gridUnit * 2}px; +`; + +export type PageHeaderWithActionsProps = { + editableTitleProps: DynamicEditableTitleProps; + showTitlePanelItems: boolean; + certificatiedBadgeProps?: CertifiedBadgeProps; + showFaveStar: boolean; + faveStarProps: FaveStarProps; + titlePanelAdditionalItems: ReactNode; + rightPanelAdditionalItems: ReactNode; + additionalActionsMenu: ReactElement; + menuDropdownProps: Omit; +}; + +export const PageHeaderWithActions = ({ + editableTitleProps, + showTitlePanelItems, + certificatiedBadgeProps, + showFaveStar, + faveStarProps, + titlePanelAdditionalItems, + rightPanelAdditionalItems, + additionalActionsMenu, + menuDropdownProps, +}: PageHeaderWithActionsProps) => { + const theme = useTheme(); + return ( +
+
+ + {showTitlePanelItems && ( +
+ {certificatiedBadgeProps?.certifiedBy && ( + + )} + {showFaveStar && } + {titlePanelAdditionalItems} +
+ )} +
+
+ {rightPanelAdditionalItems} +
+ + + +
+
+
+ ); +}; diff --git a/superset-frontend/src/components/ReportModal/HeaderReportDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportDropdown/index.tsx index 9e75b7fbaf71..cd741e5c338b 100644 --- a/superset-frontend/src/components/ReportModal/HeaderReportDropdown/index.tsx +++ b/superset-frontend/src/components/ReportModal/HeaderReportDropdown/index.tsx @@ -44,7 +44,7 @@ import { deleteActiveReport, } from 'src/reports/actions/reports'; import { reportSelector } from 'src/views/CRUD/hooks'; -import { MenuItemWithCheckboxContainer } from 'src/explore/components/ExploreAdditionalActionsMenu/index'; +import { MenuItemWithCheckboxContainer } from 'src/explore/components/useExploreAdditionalActionsMenu/index'; const deleteColor = (theme: SupersetTheme) => css` color: ${theme.colors.error.base}; diff --git a/superset-frontend/src/components/index.ts b/superset-frontend/src/components/index.ts index 8f1fccdbc63e..a370836fcfb8 100644 --- a/superset-frontend/src/components/index.ts +++ b/superset-frontend/src/components/index.ts @@ -73,4 +73,5 @@ export { export type { FormInstance } from 'antd/lib/form'; export type { ListItemProps } from 'antd/lib/list'; export type { ModalProps as AntdModalProps } from 'antd/lib/modal'; +export type { DropDownProps as AntdDropdownProps } from 'antd/lib/dropdown'; export type { RadioChangeEvent } from 'antd/lib/radio'; diff --git a/superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/ExploreAdditionalActionsMenu.test.tsx b/superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/ExploreAdditionalActionsMenu.test.tsx deleted file mode 100644 index 2b323db4bcad..000000000000 --- a/superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/ExploreAdditionalActionsMenu.test.tsx +++ /dev/null @@ -1,294 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import React from 'react'; -import fetchMock from 'fetch-mock'; -import sinon from 'sinon'; -import userEvent from '@testing-library/user-event'; -import { render, screen, waitFor } from 'spec/helpers/testing-library'; -import * as chartAction from 'src/components/Chart/chartAction'; -import * as downloadAsImage from 'src/utils/downloadAsImage'; -import * as exploreUtils from 'src/explore/exploreUtils'; -import ExploreAdditionalActionsMenu from '.'; - -const createProps = () => ({ - latestQueryFormData: { - viz_type: 'histogram', - datasource: '49__table', - slice_id: 318, - url_params: {}, - granularity_sqla: 'time_start', - time_range: 'No filter', - all_columns_x: ['age'], - adhoc_filters: [], - row_limit: 10000, - groupby: null, - color_scheme: 'supersetColors', - label_colors: {}, - link_length: '25', - x_axis_label: 'age', - y_axis_label: 'count', - }, - slice: { - cache_timeout: null, - changed_on: '2021-03-19T16:30:56.750230', - changed_on_humanized: '3 days ago', - datasource: 'FCC 2018 Survey', - description: null, - description_markeddown: '', - edit_url: '/chart/edit/318', - form_data: { - adhoc_filters: [], - all_columns_x: ['age'], - color_scheme: 'supersetColors', - datasource: '49__table', - granularity_sqla: 'time_start', - groupby: null, - label_colors: {}, - link_length: '25', - queryFields: { groupby: 'groupby' }, - row_limit: 10000, - slice_id: 318, - time_range: 'No filter', - url_params: {}, - viz_type: 'histogram', - x_axis_label: 'age', - y_axis_label: 'count', - }, - modified: '3 days ago', - owners: [], - slice_id: 318, - slice_name: 'Age distribution of respondents', - slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20318%7D', - }, - chartStatus: 'rendered', - onOpenPropertiesModal: jest.fn(), - onOpenInEditor: jest.fn(), - canDownloadCSV: false, - showReportSubMenu: false, -}); - -fetchMock.post( - 'http://api/v1/chart/data?form_data=%7B%22slice_id%22%3A318%7D', - { body: {} }, - { - sendAsJson: false, - }, -); - -test('Should render a button', () => { - const props = createProps(); - render(, { useRedux: true }); - expect(screen.getByRole('button')).toBeInTheDocument(); -}); - -test('Should open a menu', () => { - const props = createProps(); - render(, { - useRedux: true, - }); - - expect(props.onOpenInEditor).toBeCalledTimes(0); - expect(props.onOpenPropertiesModal).toBeCalledTimes(0); - userEvent.click(screen.getByRole('button')); - expect(props.onOpenInEditor).toBeCalledTimes(0); - expect(props.onOpenPropertiesModal).toBeCalledTimes(0); - - expect(screen.getByText('Edit chart properties')).toBeInTheDocument(); - expect(screen.getByText('Download')).toBeInTheDocument(); - expect(screen.getByText('Share')).toBeInTheDocument(); - expect(screen.getByText('View query')).toBeInTheDocument(); - expect(screen.getByText('Run in SQL Lab')).toBeInTheDocument(); - - expect(screen.queryByText('Set up an email report')).not.toBeInTheDocument(); - expect(screen.queryByText('Manage email report')).not.toBeInTheDocument(); -}); - -test('Should open download submenu', async () => { - const props = createProps(); - render(, { - useRedux: true, - }); - - userEvent.click(screen.getByRole('button')); - - expect(screen.queryByText('Export to .CSV')).not.toBeInTheDocument(); - expect(screen.queryByText('Export to .JSON')).not.toBeInTheDocument(); - expect(screen.queryByText('Download as image')).not.toBeInTheDocument(); - - expect(screen.getByText('Download')).toBeInTheDocument(); - userEvent.hover(screen.getByText('Download')); - expect(await screen.findByText('Export to .CSV')).toBeInTheDocument(); - expect(await screen.findByText('Export to .JSON')).toBeInTheDocument(); - expect(await screen.findByText('Download as image')).toBeInTheDocument(); -}); - -test('Should open share submenu', async () => { - const props = createProps(); - render(, { - useRedux: true, - }); - - userEvent.click(screen.getByRole('button')); - - expect( - screen.queryByText('Copy permalink to clipboard'), - ).not.toBeInTheDocument(); - expect(screen.queryByText('Embed code')).not.toBeInTheDocument(); - expect(screen.queryByText('Share chart by email')).not.toBeInTheDocument(); - - expect(screen.getByText('Share')).toBeInTheDocument(); - userEvent.hover(screen.getByText('Share')); - expect( - await screen.findByText('Copy permalink to clipboard'), - ).toBeInTheDocument(); - expect(await screen.findByText('Embed code')).toBeInTheDocument(); - expect(await screen.findByText('Share chart by email')).toBeInTheDocument(); -}); - -test('Should call onOpenPropertiesModal when click on "Edit chart properties"', () => { - const props = createProps(); - render(, { - useRedux: true, - }); - expect(props.onOpenInEditor).toBeCalledTimes(0); - userEvent.click(screen.getByRole('button')); - userEvent.click( - screen.getByRole('menuitem', { name: 'Edit chart properties' }), - ); - expect(props.onOpenPropertiesModal).toBeCalledTimes(1); -}); - -test('Should call getChartDataRequest when click on "View query"', async () => { - const props = createProps(); - const getChartDataRequest = jest.spyOn(chartAction, 'getChartDataRequest'); - render(, { - useRedux: true, - }); - - expect(getChartDataRequest).toBeCalledTimes(0); - userEvent.click(screen.getByRole('button')); - expect(getChartDataRequest).toBeCalledTimes(0); - - const menuItem = screen.getByText('View query').parentElement!; - userEvent.click(menuItem); - - await waitFor(() => expect(getChartDataRequest).toBeCalledTimes(1)); -}); - -test('Should call onOpenInEditor when click on "Run in SQL Lab"', () => { - const props = createProps(); - render(, { - useRedux: true, - }); - - expect(props.onOpenInEditor).toBeCalledTimes(0); - userEvent.click(screen.getByRole('button')); - expect(props.onOpenInEditor).toBeCalledTimes(0); - - userEvent.click(screen.getByRole('menuitem', { name: 'Run in SQL Lab' })); - expect(props.onOpenInEditor).toBeCalledTimes(1); -}); - -describe('Download', () => { - let spyDownloadAsImage = sinon.spy(); - let spyExportChart = sinon.spy(); - - beforeEach(() => { - spyDownloadAsImage = sinon.spy(downloadAsImage, 'default'); - spyExportChart = sinon.spy(exploreUtils, 'exportChart'); - }); - afterEach(() => { - spyDownloadAsImage.restore(); - spyExportChart.restore(); - }); - test('Should call downloadAsImage when click on "Download as image"', async () => { - const props = createProps(); - const spy = jest.spyOn(downloadAsImage, 'default'); - render(, { - useRedux: true, - }); - - expect(spy).toBeCalledTimes(0); - userEvent.click(screen.getByRole('button')); - expect(spy).toBeCalledTimes(0); - - userEvent.hover(screen.getByText('Download')); - const downloadAsImageElement = await screen.findByText('Download as image'); - userEvent.click(downloadAsImageElement); - - expect(spy).toBeCalledTimes(1); - }); - - test('Should not export to CSV if canDownloadCSV=false', async () => { - const props = createProps(); - render(, { - useRedux: true, - }); - userEvent.click(screen.getByRole('button')); - userEvent.hover(screen.getByText('Download')); - const exportCSVElement = await screen.findByText('Export to .CSV'); - userEvent.click(exportCSVElement); - expect(spyExportChart.callCount).toBe(0); - spyExportChart.restore(); - }); - - test('Should export to CSV if canDownloadCSV=true', async () => { - const props = createProps(); - props.canDownloadCSV = true; - render(, { - useRedux: true, - }); - - userEvent.click(screen.getByRole('button')); - userEvent.hover(screen.getByText('Download')); - const exportCSVElement = await screen.findByText('Export to .CSV'); - userEvent.click(exportCSVElement); - expect(spyExportChart.callCount).toBe(1); - spyExportChart.restore(); - }); - - test('Should export to JSON', async () => { - const props = createProps(); - render(, { - useRedux: true, - }); - - userEvent.click(screen.getByRole('button')); - userEvent.hover(screen.getByText('Download')); - const exportJsonElement = await screen.findByText('Export to .JSON'); - userEvent.click(exportJsonElement); - expect(spyExportChart.callCount).toBe(1); - }); - - test('Should export to pivoted CSV if canDownloadCSV=true and viz_type=pivot_table_v2', async () => { - const props = createProps(); - props.canDownloadCSV = true; - props.latestQueryFormData.viz_type = 'pivot_table_v2'; - render(, { - useRedux: true, - }); - - userEvent.click(screen.getByRole('button')); - userEvent.hover(screen.getByText('Download')); - const exportCSVElement = await screen.findByText('Export to pivoted .CSV'); - userEvent.click(exportCSVElement); - expect(spyExportChart.callCount).toBe(1); - }); -}); diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx index 324a6965a453..540a444ab047 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx +++ b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx @@ -18,10 +18,13 @@ */ import React from 'react'; -import { render, screen } from 'spec/helpers/testing-library'; +import sinon from 'sinon'; +import { render, screen, waitFor } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; - import fetchMock from 'fetch-mock'; +import * as chartAction from 'src/components/Chart/chartAction'; +import * as downloadAsImage from 'src/utils/downloadAsImage'; +import * as exploreUtils from 'src/explore/exploreUtils'; import ExploreHeader from '.'; const chartEndpoint = 'glob:*api/v1/chart/*'; @@ -30,6 +33,7 @@ fetchMock.get(chartEndpoint, { json: 'foo' }); const createProps = () => ({ chart: { + id: 1, latestQueryFormData: { viz_type: 'histogram', datasource: '49__table', @@ -88,17 +92,29 @@ const createProps = () => ({ }, slice_name: 'Age distribution of respondents', actions: { - postChartFormData: () => null, - updateChartTitle: () => null, - fetchFaveStar: () => null, - saveFaveStar: () => null, + postChartFormData: jest.fn(), + updateChartTitle: jest.fn(), + fetchFaveStar: jest.fn(), + saveFaveStar: jest.fn(), + redirectSQLLab: jest.fn(), }, user: { userId: 1, }, onSaveChart: jest.fn(), + canOverwrite: false, + canDownload: false, + isStarred: false, }); +fetchMock.post( + 'http://api/v1/chart/data?form_data=%7B%22slice_id%22%3A318%7D', + { body: {} }, + { + sendAsJson: false, + }, +); + test('Cancelling changes to the properties should reset previous properties', () => { const props = createProps(); render(, { useRedux: true }); @@ -136,3 +152,208 @@ test('Save disabled', () => { userEvent.click(screen.getByText('Save')); expect(props.onSaveChart).not.toHaveBeenCalled(); }); + +describe('Additional actions tests', () => { + test('Should render a button', () => { + const props = createProps(); + render(, { useRedux: true }); + expect(screen.getByLabelText('Menu actions trigger')).toBeInTheDocument(); + }); + + test('Should open a menu', () => { + const props = createProps(); + render(, { + useRedux: true, + }); + + userEvent.click(screen.getByLabelText('Menu actions trigger')); + + expect(screen.getByText('Edit chart properties')).toBeInTheDocument(); + expect(screen.getByText('Download')).toBeInTheDocument(); + expect(screen.getByText('Share')).toBeInTheDocument(); + expect(screen.getByText('View query')).toBeInTheDocument(); + expect(screen.getByText('Run in SQL Lab')).toBeInTheDocument(); + + expect( + screen.queryByText('Set up an email report'), + ).not.toBeInTheDocument(); + expect(screen.queryByText('Manage email report')).not.toBeInTheDocument(); + }); + + test('Should open download submenu', async () => { + const props = createProps(); + render(, { + useRedux: true, + }); + + userEvent.click(screen.getByLabelText('Menu actions trigger')); + + expect(screen.queryByText('Export to .CSV')).not.toBeInTheDocument(); + expect(screen.queryByText('Export to .JSON')).not.toBeInTheDocument(); + expect(screen.queryByText('Download as image')).not.toBeInTheDocument(); + + expect(screen.getByText('Download')).toBeInTheDocument(); + userEvent.hover(screen.getByText('Download')); + expect(await screen.findByText('Export to .CSV')).toBeInTheDocument(); + expect(await screen.findByText('Export to .JSON')).toBeInTheDocument(); + expect(await screen.findByText('Download as image')).toBeInTheDocument(); + }); + + test('Should open share submenu', async () => { + const props = createProps(); + render(, { + useRedux: true, + }); + + userEvent.click(screen.getByLabelText('Menu actions trigger')); + + expect( + screen.queryByText('Copy permalink to clipboard'), + ).not.toBeInTheDocument(); + expect(screen.queryByText('Embed code')).not.toBeInTheDocument(); + expect(screen.queryByText('Share chart by email')).not.toBeInTheDocument(); + + expect(screen.getByText('Share')).toBeInTheDocument(); + userEvent.hover(screen.getByText('Share')); + expect( + await screen.findByText('Copy permalink to clipboard'), + ).toBeInTheDocument(); + expect(await screen.findByText('Embed code')).toBeInTheDocument(); + expect(await screen.findByText('Share chart by email')).toBeInTheDocument(); + }); + + test('Should call onOpenPropertiesModal when click on "Edit chart properties"', async () => { + const props = createProps(); + render(, { + useRedux: true, + }); + expect(props.actions.redirectSQLLab).toBeCalledTimes(0); + userEvent.click(screen.getByLabelText('Menu actions trigger')); + userEvent.click( + screen.getByRole('menuitem', { name: 'Edit chart properties' }), + ); + expect(await screen.findByText('Edit Chart Properties')).toBeVisible(); + }); + + test('Should call getChartDataRequest when click on "View query"', async () => { + const props = createProps(); + const getChartDataRequest = jest.spyOn(chartAction, 'getChartDataRequest'); + render(, { + useRedux: true, + }); + + expect(getChartDataRequest).toBeCalledTimes(0); + userEvent.click(screen.getByLabelText('Menu actions trigger')); + expect(getChartDataRequest).toBeCalledTimes(0); + + const menuItem = screen.getByText('View query').parentElement!; + userEvent.click(menuItem); + + await waitFor(() => expect(getChartDataRequest).toBeCalledTimes(1)); + }); + + test('Should call onOpenInEditor when click on "Run in SQL Lab"', () => { + const props = createProps(); + render(, { + useRedux: true, + }); + + expect(props.actions.redirectSQLLab).toBeCalledTimes(0); + userEvent.click(screen.getByLabelText('Menu actions trigger')); + expect(props.actions.redirectSQLLab).toBeCalledTimes(0); + + userEvent.click(screen.getByRole('menuitem', { name: 'Run in SQL Lab' })); + expect(props.actions.redirectSQLLab).toBeCalledTimes(1); + }); + + describe('Download', () => { + let spyDownloadAsImage = sinon.spy(); + let spyExportChart = sinon.spy(); + + beforeEach(() => { + spyDownloadAsImage = sinon.spy(downloadAsImage, 'default'); + spyExportChart = sinon.spy(exploreUtils, 'exportChart'); + }); + afterEach(() => { + spyDownloadAsImage.restore(); + spyExportChart.restore(); + }); + test('Should call downloadAsImage when click on "Download as image"', async () => { + const props = createProps(); + const spy = jest.spyOn(downloadAsImage, 'default'); + render(, { + useRedux: true, + }); + + expect(spy).toBeCalledTimes(0); + userEvent.click(screen.getByLabelText('Menu actions trigger')); + expect(spy).toBeCalledTimes(0); + + userEvent.hover(screen.getByText('Download')); + const downloadAsImageElement = await screen.findByText( + 'Download as image', + ); + userEvent.click(downloadAsImageElement); + + expect(spy).toBeCalledTimes(1); + }); + + test('Should not export to CSV if canDownload=false', async () => { + const props = createProps(); + render(, { + useRedux: true, + }); + userEvent.click(screen.getByLabelText('Menu actions trigger')); + userEvent.hover(screen.getByText('Download')); + const exportCSVElement = await screen.findByText('Export to .CSV'); + userEvent.click(exportCSVElement); + expect(spyExportChart.callCount).toBe(0); + spyExportChart.restore(); + }); + + test('Should export to CSV if canDownload=true', async () => { + const props = createProps(); + props.canDownload = true; + render(, { + useRedux: true, + }); + + userEvent.click(screen.getByLabelText('Menu actions trigger')); + userEvent.hover(screen.getByText('Download')); + const exportCSVElement = await screen.findByText('Export to .CSV'); + userEvent.click(exportCSVElement); + expect(spyExportChart.callCount).toBe(1); + spyExportChart.restore(); + }); + + test('Should export to JSON', async () => { + const props = createProps(); + render(, { + useRedux: true, + }); + + userEvent.click(screen.getByLabelText('Menu actions trigger')); + userEvent.hover(screen.getByText('Download')); + const exportJsonElement = await screen.findByText('Export to .JSON'); + userEvent.click(exportJsonElement); + expect(spyExportChart.callCount).toBe(1); + }); + + test('Should export to pivoted CSV if canDownloadCSV=true and viz_type=pivot_table_v2', async () => { + const props = createProps(); + props.canDownload = true; + props.chart.latestQueryFormData.viz_type = 'pivot_table_v2'; + render(, { + useRedux: true, + }); + + userEvent.click(screen.getByLabelText('Menu actions trigger')); + userEvent.hover(screen.getByText('Download')); + const exportCSVElement = await screen.findByText( + 'Export to pivoted .CSV', + ); + userEvent.click(exportCSVElement); + expect(spyExportChart.callCount).toBe(1); + }); + }); +}); diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx b/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx index 92b949d78548..fa3f5f93542a 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx +++ b/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; +import React, { useEffect, useState } from 'react'; import { connect } from 'react-redux'; import { bindActionCreators } from 'redux'; import PropTypes from 'prop-types'; @@ -30,14 +30,12 @@ import { import { toggleActive, deleteActiveReport } from 'src/reports/actions/reports'; import { chartPropShape } from 'src/dashboard/util/propShapes'; import AlteredSliceTag from 'src/components/AlteredSliceTag'; -import FaveStar from 'src/components/FaveStar'; import Button from 'src/components/Button'; import Icons from 'src/components/Icons'; import PropertiesModal from 'src/explore/components/PropertiesModal'; import { sliceUpdated } from 'src/explore/actions/exploreActions'; -import CertifiedBadge from 'src/components/CertifiedBadge'; -import ExploreAdditionalActionsMenu from '../ExploreAdditionalActionsMenu'; -import { ChartEditableTitle } from './ChartEditableTitle'; +import { PageHeaderWithActions } from 'src/components/PageHeaderWithActions'; +import { useExploreAdditionalActionsMenu } from '../useExploreAdditionalActionsMenu'; const propTypes = { actions: PropTypes.object.isRequired, @@ -48,7 +46,7 @@ const propTypes = { slice: PropTypes.object, sliceName: PropTypes.string, table_name: PropTypes.string, - form_data: PropTypes.object, + formData: PropTypes.object, ownState: PropTypes.object, timeout: PropTypes.number, chart: chartPropShape, @@ -62,70 +60,25 @@ const saveButtonStyles = theme => css` } `; -const headerStyles = theme => css` - display: flex; - flex-direction: row; - align-items: center; - flex-wrap: nowrap; - justify-content: space-between; - height: 100%; - - span[role='button'] { - display: flex; - height: 100%; - } - - .title-panel { - display: flex; - align-items: center; - min-width: 0; - margin-right: ${theme.gridUnit * 12}px; - } - - .right-button-panel { - display: flex; - align-items: center; - } -`; - -const buttonsStyles = theme => css` - display: flex; - align-items: center; - padding-left: ${theme.gridUnit * 2}px; - - & .fave-unfave-icon { - padding: 0 ${theme.gridUnit}px; - - &:first-child { - padding-left: 0; - } - } -`; - -const saveButtonContainerStyles = theme => css` - margin-right: ${theme.gridUnit * 2}px; -`; - -export class ExploreChartHeader extends React.PureComponent { - constructor(props) { - super(props); - this.state = { - isPropertiesModalOpen: false, - }; - this.openPropertiesModal = this.openPropertiesModal.bind(this); - this.closePropertiesModal = this.closePropertiesModal.bind(this); - this.fetchChartDashboardData = this.fetchChartDashboardData.bind(this); - } - - componentDidMount() { - const { dashboardId } = this.props; - if (dashboardId) { - this.fetchChartDashboardData(); - } - } - - async fetchChartDashboardData() { - const { dashboardId, slice } = this.props; +export const ExploreChartHeader = ({ + dashboardId, + slice, + actions, + formData, + chart, + user, + canOverwrite, + canDownload, + isStarred, + sliceUpdated, + sliceName, + onSaveChart, + saveDisabled, +}) => { + const { latestQueryFormData, sliceFormData } = chart; + const [isPropertiesModalOpen, setIsPropertiesModalOpen] = useState(false); + + const fetchChartDashboardData = async () => { await SupersetClient.get({ endpoint: `/api/v1/chart/${slice.slice_id}`, }) @@ -162,96 +115,71 @@ export class ExploreChartHeader extends React.PureComponent { } }) .catch(() => {}); - } + }; - postChartFormData() { - this.props.actions.postChartFormData( - this.props.form_data, - true, - this.props.timeout, - this.props.chart.id, - this.props.ownState, - ); - } + useEffect(() => { + if (dashboardId) { + fetchChartDashboardData(); + } + }, []); - openPropertiesModal() { - this.setState({ - isPropertiesModalOpen: true, - }); - } + const openPropertiesModal = () => { + setIsPropertiesModalOpen(true); + }; - closePropertiesModal() { - this.setState({ - isPropertiesModalOpen: false, - }); - } + const closePropertiesModal = () => { + setIsPropertiesModalOpen(false); + }; - render() { - const { - actions, - chart, - user, - formData, - slice, - canOverwrite, + const [menu, isDropdownVisible, setIsDropdownVisible] = + useExploreAdditionalActionsMenu( + latestQueryFormData, canDownload, - isStarred, - sliceUpdated, - sliceName, - onSaveChart, - saveDisabled, - } = this.props; - const { latestQueryFormData, sliceFormData } = chart; - const oldSliceName = slice?.slice_name; - return ( -
-
- - {slice && ( - - {slice.certified_by && ( - - )} - {user.userId && ( - - )} - {this.state.isPropertiesModalOpen && ( - - )} - {sliceFormData && ( - - )} - - )} -
-
+ slice, + actions.redirectSQLLab, + openPropertiesModal, + ); + + const oldSliceName = slice?.slice_name; + return ( + <> + + ) : null + } + rightPanelAdditionalItems={ {/* needed to wrap button in a div - antd tooltip doesn't work with disabled button */} -
+
- -
-
- ); - } -} + } + additionalActionsMenu={menu} + menuDropdownProps={{ + visible: isDropdownVisible, + onVisibleChange: setIsDropdownVisible, + }} + /> + {isPropertiesModalOpen && ( + + )} + + ); +}; ExploreChartHeader.propTypes = propTypes; diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx index 2715141a104c..354e95b5ab2d 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx @@ -571,7 +571,6 @@ function ExploreViewContainer(props) { { +) => { const theme = useTheme(); + const { addDangerToast, addSuccessToast } = useToasts(); const [showReportSubMenu, setShowReportSubMenu] = useState(null); const [isDropdownVisible, setIsDropdownVisible] = useState(false); const [openSubmenus, setOpenSubmenus] = useState([]); - const chart = useSelector(state => { - if (!state.charts) { + const charts = useSelector(state => state.charts); + const chart = useMemo(() => { + if (!charts) { return undefined; } - const charts = Object.values(state.charts); - if (charts.length > 0) { - return charts[0]; + const chartsValues = Object.values(charts); + if (chartsValues.length > 0) { + return chartsValues[0]; } return undefined; - }); + }, [charts]); const { datasource } = latestQueryFormData; const sqlSupported = datasource && datasource.split('__')[1] === 'table'; @@ -258,157 +246,144 @@ const ExploreAdditionalActionsMenu = ({ ], ); - return ( - <> - - {slice && ( - <> - - {t('Edit chart properties')} - - - - )} - - {VIZ_TYPES_PIVOTABLE.includes(latestQueryFormData.viz_type) ? ( - <> - } - disabled={!canDownloadCSV} - > - {t('Export to original .CSV')} - - } - disabled={!canDownloadCSV} - > - {t('Export to pivoted .CSV')} - - - ) : ( - } - disabled={!canDownloadCSV} - > - {t('Export to .CSV')} - - )} - }> - {t('Export to .JSON')} - + const menu = useMemo( + () => ( + + {slice && ( + <> + + {t('Edit chart properties')} + + + + )} + + {VIZ_TYPES_PIVOTABLE.includes(latestQueryFormData.viz_type) ? ( + <> } + key={MENU_KEYS.EXPORT_TO_CSV} + icon={} + disabled={!canDownloadCSV} > - {t('Download as image')} - - - - - {t('Copy permalink to clipboard')} + {t('Export to original .CSV')} - - {t('Share chart by email')} + } + disabled={!canDownloadCSV} + > + {t('Export to pivoted .CSV')} - - {t('Embed code')} - } - modalTitle={t('Embed code')} - modalBody={ - - } - maxWidth={`${theme.gridUnit * 100}px`} - destroyOnClose - responsive + + ) : ( + } + disabled={!canDownloadCSV} + > + {t('Export to .CSV')} + + )} + }> + {t('Export to .JSON')} + + } + > + {t('Download as image')} + + + + + {t('Copy permalink to clipboard')} + + + {t('Share chart by email')} + + + {t('Embed code')} + } + modalTitle={t('Embed code')} + modalBody={ + - + } + maxWidth={`${theme.gridUnit * 100}px`} + destroyOnClose + responsive + /> + + + + {showReportSubMenu ? ( + <> + + - {showReportSubMenu ? ( - <> - - - - - - ) : ( - - - - )} - - - {t('View query')} - - } - modalTitle={t('View query')} - modalBody={ - - } - draggable - resizable - responsive - /> - - {sqlSupported && ( - - {t('Run in SQL Lab')} - - )} + + ) : ( + + - } - > - - + {t('View query')} + } + modalTitle={t('View query')} + modalBody={ + + } + draggable + resizable + responsive /> - - - + + {sqlSupported && ( + + {t('Run in SQL Lab')} + + )} + + ), + [ + addDangerToast, + canDownloadCSV, + chart, + handleMenuClick, + isDropdownVisible, + latestQueryFormData, + openSubmenus, + showReportSubMenu, + slice, + sqlSupported, + theme.gridUnit, + ], ); + return [menu, isDropdownVisible, setIsDropdownVisible]; }; - -ExploreAdditionalActionsMenu.propTypes = propTypes; - -export default withToasts(ExploreAdditionalActionsMenu);