Skip to content

Commit

Permalink
feat(explore): Move chart actions into dropdown (apache#19446)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
kgabryje authored and philipher29 committed Jun 9, 2022
1 parent 97c9584 commit 7857ecf
Show file tree
Hide file tree
Showing 16 changed files with 905 additions and 851 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
});
Expand Down
2 changes: 2 additions & 0 deletions superset-frontend/src/components/ModalTrigger/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const propTypes = {
resizableConfig: PropTypes.object,
draggable: PropTypes.bool,
draggableConfig: PropTypes.object,
destroyOnClose: PropTypes.bool,
};

const defaultProps = {
Expand Down Expand Up @@ -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}
</Modal>
Expand Down
25 changes: 10 additions & 15 deletions superset-frontend/src/components/ReportModal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -85,14 +84,14 @@ interface ChartObject {
chartUpdateEndTime: number;
chartUpdateStartTime: number;
latestQueryFormData: object;
sliceFormData: Record<string, any>;
queryController: { abort: () => {} };
queriesResponse: object;
triggerQuery: boolean;
lastRendered: number;
}

interface ReportProps {
addReport: (report?: ReportObject) => {};
onHide: () => {};
onReportAdd: (report?: ReportObject) => {};
addDangerToast: (msg: string) => void;
Expand All @@ -102,7 +101,6 @@ interface ReportProps {
dashboardId?: number;
chart?: ChartObject;
creationMethod: string;
props: any;
}

interface ReportPayloadType {
Expand Down Expand Up @@ -189,8 +187,8 @@ const ReportModal: FunctionComponent<ReportProps> = ({
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
Expand Down Expand Up @@ -226,19 +224,19 @@ const ReportModal: FunctionComponent<ReportProps> = ({
// Create new Report
const newReportValues: Partial<ReportObject> = {
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,
Expand Down Expand Up @@ -416,7 +414,4 @@ const ReportModal: FunctionComponent<ReportProps> = ({
);
};

const mapDispatchToProps = (dispatch: any) =>
bindActionCreators({ addReport, editReport }, dispatch);

export default connect(null, mapDispatchToProps)(withToasts(ReportModal));
export default withToasts(ReportModal);
10 changes: 4 additions & 6 deletions superset-frontend/src/dashboard/components/Header/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -662,12 +662,10 @@ class Header extends React.PureComponent {
<ReportModal
show={this.state.showingReportModal}
onHide={this.hideReportModal}
props={{
userId: user.userId,
userEmail: user.email,
dashboardId: dashboardInfo.id,
creationMethod: 'dashboards',
}}
userId={user.userId}
userEmail={user.email}
dashboardId={dashboardInfo.id}
creationMethod="dashboards"
/>
)}

Expand Down
168 changes: 0 additions & 168 deletions superset-frontend/src/explore/components/EmbedCodeButton.jsx

This file was deleted.

62 changes: 0 additions & 62 deletions superset-frontend/src/explore/components/EmbedCodeButton.test.jsx

This file was deleted.

0 comments on commit 7857ecf

Please sign in to comment.