From 7857ecf8dad920860e2d2f83001a1263e61416c0 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 31 Mar 2022 20:41:15 +0200 Subject: [PATCH] feat(explore): Move chart actions into dropdown (#19446) * feat(explore): Move chart actions to a dropdown menu * Fix tests and add some new ones * Add background color to embed code button * Fix cypress tests * Move copy permalink to actions menu * Remove unused function * Move share by email above embed code * Fix test * Fix test * Fix test * Fix test * Fix test --- .../cypress/integration/explore/link.test.ts | 9 +- .../src/components/ModalTrigger/index.jsx | 2 + .../src/components/ReportModal/index.tsx | 25 +- .../src/dashboard/components/Header/index.jsx | 10 +- .../explore/components/EmbedCodeButton.jsx | 168 ------- .../components/EmbedCodeButton.test.jsx | 62 --- .../explore/components/EmbedCodeContent.jsx | 153 ++++++ .../components/EmbedCodeContent.test.jsx | 48 ++ .../components/ExploreActionButtons.test.jsx | 117 ----- .../components/ExploreActionButtons.tsx | 234 --------- .../ExploreAdditionalActionsMenu.test.jsx | 60 --- .../ExploreAdditionalActionsMenu.test.tsx | 198 +++++++- .../ExploreReport.tsx | 92 ++++ .../ExploreAdditionalActionsMenu/index.jsx | 476 +++++++++++++++--- .../ExploreChartHeader.test.tsx | 10 +- .../components/ExploreChartHeader/index.jsx | 92 +--- 16 files changed, 905 insertions(+), 851 deletions(-) delete mode 100644 superset-frontend/src/explore/components/EmbedCodeButton.jsx delete mode 100644 superset-frontend/src/explore/components/EmbedCodeButton.test.jsx create mode 100644 superset-frontend/src/explore/components/EmbedCodeContent.jsx create mode 100644 superset-frontend/src/explore/components/EmbedCodeContent.test.jsx delete mode 100644 superset-frontend/src/explore/components/ExploreActionButtons.test.jsx delete mode 100644 superset-frontend/src/explore/components/ExploreActionButtons.tsx delete mode 100644 superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/ExploreAdditionalActionsMenu.test.jsx create mode 100644 superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/ExploreReport.tsx diff --git a/superset-frontend/cypress-base/cypress/integration/explore/link.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/link.test.ts index 027326e021837..9f07e9c10b859 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/link.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/link.test.ts @@ -38,7 +38,7 @@ describe('Test explore links', () => { cy.visitChartByName('Growth Rate'); cy.verifySliceSuccess({ waitAlias: '@chartData' }); - cy.get('div#query').click(); + cy.get('[aria-label="Menu actions trigger"]').click(); cy.get('span').contains('View query').parent().click(); cy.wait('@chartData').then(() => { cy.get('code'); @@ -52,7 +52,12 @@ describe('Test explore links', () => { cy.visitChartByName('Growth Rate'); cy.verifySliceSuccess({ waitAlias: '@chartData' }); - cy.get('[data-test=embed-code-button]').click(); + cy.get('[aria-label="Menu actions trigger"]').click(); + cy.get('div[title="Share"]').trigger('mouseover'); + // need to use [id= syntax, otherwise error gets triggered because of special character in id + cy.get('[id="share_submenu$Menu"]').within(() => { + cy.contains('Embed code').parent().click(); + }); cy.get('#embed-code-popover').within(() => { cy.get('textarea[name=embedCode]').contains('iframe'); }); diff --git a/superset-frontend/src/components/ModalTrigger/index.jsx b/superset-frontend/src/components/ModalTrigger/index.jsx index e01e6591adb07..b15000d851fb3 100644 --- a/superset-frontend/src/components/ModalTrigger/index.jsx +++ b/superset-frontend/src/components/ModalTrigger/index.jsx @@ -39,6 +39,7 @@ const propTypes = { resizableConfig: PropTypes.object, draggable: PropTypes.bool, draggableConfig: PropTypes.object, + destroyOnClose: PropTypes.bool, }; const defaultProps = { @@ -89,6 +90,7 @@ export default class ModalTrigger extends React.Component { resizableConfig={this.props.resizableConfig} draggable={this.props.draggable} draggableConfig={this.props.draggableConfig} + destroyOnClose={this.props.destroyOnClose} > {this.props.modalBody} diff --git a/superset-frontend/src/components/ReportModal/index.tsx b/superset-frontend/src/components/ReportModal/index.tsx index c8273bd985801..5cdc216a0e67f 100644 --- a/superset-frontend/src/components/ReportModal/index.tsx +++ b/superset-frontend/src/components/ReportModal/index.tsx @@ -26,8 +26,7 @@ import React, { } from 'react'; import { t, SupersetTheme } from '@superset-ui/core'; import { getClientErrorObject } from 'src/utils/getClientErrorObject'; -import { bindActionCreators } from 'redux'; -import { connect, useDispatch, useSelector } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; import { addReport, editReport } from 'src/reports/actions/reports'; import { AlertObject } from 'src/views/CRUD/alert/types'; @@ -85,6 +84,7 @@ interface ChartObject { chartUpdateEndTime: number; chartUpdateStartTime: number; latestQueryFormData: object; + sliceFormData: Record; queryController: { abort: () => {} }; queriesResponse: object; triggerQuery: boolean; @@ -92,7 +92,6 @@ interface ChartObject { } interface ReportProps { - addReport: (report?: ReportObject) => {}; onHide: () => {}; onReportAdd: (report?: ReportObject) => {}; addDangerToast: (msg: string) => void; @@ -102,7 +101,6 @@ interface ReportProps { dashboardId?: number; chart?: ChartObject; creationMethod: string; - props: any; } interface ReportPayloadType { @@ -189,8 +187,8 @@ const ReportModal: FunctionComponent = ({ show = false, ...props }) => { - const vizType = props.props.chart?.sliceFormData?.viz_type; - const isChart = !!props.props.chart; + const vizType = props.chart?.sliceFormData?.viz_type; + const isChart = !!props.chart; const defaultNotificationFormat = isChart && TEXT_BASED_VISUALIZATION_TYPES.includes(vizType) ? NOTIFICATION_FORMATS.TEXT @@ -226,19 +224,19 @@ const ReportModal: FunctionComponent = ({ // Create new Report const newReportValues: Partial = { crontab: currentReport?.crontab, - dashboard: props.props.dashboardId, - chart: props.props.chart?.id, + dashboard: props.dashboardId, + chart: props.chart?.id, description: currentReport?.description, name: currentReport?.name, - owners: [props.props.userId], + owners: [props.userId], recipients: [ { - recipient_config_json: { target: props.props.userEmail }, + recipient_config_json: { target: props.userEmail }, type: 'Email', }, ], type: 'Report', - creation_method: props.props.creationMethod, + creation_method: props.creationMethod, active: true, report_format: currentReport?.report_format || defaultNotificationFormat, timezone: currentReport?.timezone, @@ -416,7 +414,4 @@ const ReportModal: FunctionComponent = ({ ); }; -const mapDispatchToProps = (dispatch: any) => - bindActionCreators({ addReport, editReport }, dispatch); - -export default connect(null, mapDispatchToProps)(withToasts(ReportModal)); +export default withToasts(ReportModal); diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 524d84d677064..28b4f6d9f7ad2 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -662,12 +662,10 @@ class Header extends React.PureComponent { )} diff --git a/superset-frontend/src/explore/components/EmbedCodeButton.jsx b/superset-frontend/src/explore/components/EmbedCodeButton.jsx deleted file mode 100644 index 71f77a4621fa2..0000000000000 --- a/superset-frontend/src/explore/components/EmbedCodeButton.jsx +++ /dev/null @@ -1,168 +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 { t } from '@superset-ui/core'; - -import Popover from 'src/components/Popover'; -import { FormLabel } from 'src/components/Form'; -import Icons from 'src/components/Icons'; -import { Tooltip } from 'src/components/Tooltip'; -import CopyToClipboard from 'src/components/CopyToClipboard'; -import { URL_PARAMS } from 'src/constants'; -import { getChartPermalink } from 'src/utils/urlUtils'; - -export default class EmbedCodeButton extends React.Component { - constructor(props) { - super(props); - this.state = { - height: '400', - width: '600', - url: '', - errorMessage: '', - }; - this.handleInputChange = this.handleInputChange.bind(this); - this.updateUrl = this.updateUrl.bind(this); - } - - handleInputChange(e) { - const { value, name } = e.currentTarget; - const data = {}; - data[name] = value; - this.setState(data); - } - - updateUrl() { - this.setState({ url: '' }); - getChartPermalink(this.props.formData) - .then(url => this.setState({ errorMessage: '', url })) - .catch(() => { - this.setState({ errorMessage: t('Error') }); - this.props.addDangerToast( - t('Sorry, something went wrong. Try again later.'), - ); - }); - } - - generateEmbedHTML() { - if (!this.state.url) return ''; - const srcLink = `${this.state.url}?${URL_PARAMS.standalone.name}=1&height=${this.state.height}`; - return ( - '\n' + - '' - ); - } - - renderPopoverContent() { - const html = this.generateEmbedHTML(); - const text = - this.state.errorMessage || html || t('Generating link, please wait..'); - return ( -
-
-
-