From ad98981d9d3d95bd2e7f19a0481e8ff58aa1d4c4 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Wed, 4 Nov 2020 22:47:25 +0100 Subject: [PATCH] refactor: Replace react-bootstrap MenuItems with Antd Menu (#11487) * Remove MenuItem from CopyToClipboard * Refactor DateFilterControl * fixup! Remove MenuItem from CopyToClipboard * Remove console log * Refactor LanguagePicker * Refactor HeaderActionsDropdown * Remove dir with Menu component * Add imports to common/components/index * Fix after rebase --- .../integration/dashboard/controls.test.js | 35 +-- .../components/HeaderActionsDropdown_spec.jsx | 15 +- .../src/common/components/index.tsx | 2 + .../src/components/CopyToClipboard.jsx | 29 +-- .../src/components/Menu/LanguagePicker.tsx | 30 ++- .../src/dashboard/components/CssEditor.jsx | 1 - .../components/HeaderActionsDropdown.jsx | 231 ++++++++++-------- .../components/RefreshIntervalModal.jsx | 1 - .../src/dashboard/components/SaveModal.jsx | 3 - .../components/controls/DateFilterControl.jsx | 71 ++---- 10 files changed, 210 insertions(+), 208 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/controls.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/controls.test.js index 58849de90b..6f9215ce24 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/controls.test.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/controls.test.js @@ -72,9 +72,10 @@ describe('Dashboard top-level controls', () => { .trigger('click', { force: true }); // not allow dashboard level force refresh when any chart is loading - cy.get('[data-test="refresh-dashboard-menu-item"]') - .parent() - .should('have.class', 'disabled'); + cy.get('[data-test="refresh-dashboard-menu-item"]').should( + 'have.class', + 'ant-menu-item-disabled', + ); // not allow chart level force refresh when it is loading cy.get(`#slice_${mapId}-controls`) .next() @@ -84,25 +85,28 @@ describe('Dashboard top-level controls', () => { .should('have.class', 'disabled'); cy.wait(`@postJson_${mapId}_force`); - cy.get('[data-test="refresh-dashboard-menu-item"]') - .parent() - .not('have.class', 'disabled'); + cy.get('[data-test="refresh-dashboard-menu-item"]').should( + 'not.have.class', + 'ant-menu-item-disabled', + ); }); it('should allow dashboard level force refresh', () => { // when charts are not start loading, for example, under a secondary tab, // should allow force refresh cy.get('[data-test="more-horiz"]').click(); - cy.get('[data-test="refresh-dashboard-menu-item"]') - .parent() - .not('have.class', 'disabled'); + cy.get('[data-test="refresh-dashboard-menu-item"]').should( + 'not.have.class', + 'ant-menu-item-disabled', + ); // wait the all dash finish loading. cy.wait(sliceRequests); cy.get('[data-test="refresh-dashboard-menu-item"]').click(); - cy.get('[data-test="refresh-dashboard-menu-item"]') - .parent() - .should('have.class', 'disabled'); + cy.get('[data-test="refresh-dashboard-menu-item"]').should( + 'have.class', + 'ant-menu-item-disabled', + ); // wait all charts force refreshed cy.wait(forceRefreshRequests, { responseTimeout: 15000 }).then(xhrs => { @@ -115,8 +119,9 @@ describe('Dashboard top-level controls', () => { }); cy.get('[data-test="more-horiz"]').click(); - cy.get('[data-test="refresh-dashboard-menu-item"]') - .parent() - .not('have.class', 'disabled'); + cy.get('[data-test="refresh-dashboard-menu-item"]').should( + 'not.have.class', + 'ant-menu-item-disabled', + ); }); }); diff --git a/superset-frontend/spec/javascripts/dashboard/components/HeaderActionsDropdown_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/HeaderActionsDropdown_spec.jsx index 033e801bc5..deb9450452 100644 --- a/superset-frontend/spec/javascripts/dashboard/components/HeaderActionsDropdown_spec.jsx +++ b/superset-frontend/spec/javascripts/dashboard/components/HeaderActionsDropdown_spec.jsx @@ -18,7 +18,8 @@ */ import React from 'react'; import { shallow } from 'enzyme'; -import { DropdownButton, MenuItem } from 'react-bootstrap'; +import { DropdownButton } from 'react-bootstrap'; +import { Menu } from 'src/common/components'; import RefreshIntervalModal from 'src/dashboard/components/RefreshIntervalModal'; import URLShortLinkModal from 'src/components/URLShortLinkModal'; import HeaderActionsDropdown from 'src/dashboard/components/HeaderActionsDropdown'; @@ -72,9 +73,9 @@ describe('HeaderActionsDropdown', () => { expect(wrapper.find(SaveModal)).not.toExist(); }); - it('should render four MenuItems', () => { + it('should render five Menu items', () => { const wrapper = setup(overrideProps); - expect(wrapper.find(MenuItem)).toHaveLength(4); + expect(wrapper.find(Menu.Item)).toHaveLength(5); }); it('should render the RefreshIntervalModal', () => { @@ -106,9 +107,9 @@ describe('HeaderActionsDropdown', () => { expect(wrapper.find(SaveModal)).toExist(); }); - it('should render four MenuItems', () => { + it('should render six Menu items', () => { const wrapper = setup(overrideProps); - expect(wrapper.find(MenuItem)).toHaveLength(4); + expect(wrapper.find(Menu.Item)).toHaveLength(6); }); it('should render the RefreshIntervalModal', () => { @@ -140,9 +141,9 @@ describe('HeaderActionsDropdown', () => { expect(wrapper.find(SaveModal)).toExist(); }); - it('should render three MenuItems', () => { + it('should render seven MenuItems', () => { const wrapper = setup(overrideProps); - expect(wrapper.find(MenuItem)).toHaveLength(3); + expect(wrapper.find(Menu.Item)).toHaveLength(7); }); it('should render the RefreshIntervalModal', () => { diff --git a/superset-frontend/src/common/components/index.tsx b/superset-frontend/src/common/components/index.tsx index 2e6b90d9e3..99fafb3dd3 100644 --- a/superset-frontend/src/common/components/index.tsx +++ b/superset-frontend/src/common/components/index.tsx @@ -33,8 +33,10 @@ export { DatePicker, Dropdown, Empty, + Input, Modal, Popover, + Select, Skeleton, Tabs, Tooltip, diff --git a/superset-frontend/src/components/CopyToClipboard.jsx b/superset-frontend/src/components/CopyToClipboard.jsx index 918e3608c2..abc7df60c8 100644 --- a/superset-frontend/src/components/CopyToClipboard.jsx +++ b/superset-frontend/src/components/CopyToClipboard.jsx @@ -18,7 +18,7 @@ */ import React from 'react'; import PropTypes from 'prop-types'; -import { Tooltip, OverlayTrigger, MenuItem } from 'react-bootstrap'; +import { Tooltip, OverlayTrigger } from 'react-bootstrap'; import { t } from '@superset-ui/core'; import withToasts from 'src/messageToasts/enhancers/withToasts'; @@ -28,7 +28,6 @@ const propTypes = { onCopyEnd: PropTypes.func, shouldShowText: PropTypes.bool, text: PropTypes.string, - inMenu: PropTypes.bool, wrapped: PropTypes.bool, tooltipText: PropTypes.string, addDangerToast: PropTypes.func.isRequired, @@ -38,7 +37,6 @@ const defaultProps = { copyNode: Copy, onCopyEnd: () => {}, shouldShowText: true, - inMenu: false, wrapped: true, tooltipText: t('Copy to clipboard'), }; @@ -160,27 +158,6 @@ class CopyToClipboard extends React.Component { ); } - renderInMenu() { - return ( - - - - {this.props.copyNode} - - - - ); - } - renderTooltip() { return ( {this.tooltipText()} @@ -188,11 +165,11 @@ class CopyToClipboard extends React.Component { } render() { - const { wrapped, inMenu } = this.props; + const { wrapped } = this.props; if (!wrapped) { return this.renderNotWrapped(); } - return inMenu ? this.renderInMenu() : this.renderLink(); + return this.renderLink(); } } diff --git a/superset-frontend/src/components/Menu/LanguagePicker.tsx b/superset-frontend/src/components/Menu/LanguagePicker.tsx index 69e2846313..51b88be89f 100644 --- a/superset-frontend/src/components/Menu/LanguagePicker.tsx +++ b/superset-frontend/src/components/Menu/LanguagePicker.tsx @@ -17,7 +17,7 @@ * under the License. */ import React from 'react'; -import { MenuItem } from 'react-bootstrap'; +import { Menu } from 'src/common/components'; import NavDropdown from 'src/components/NavDropdown'; export interface Languages { @@ -46,17 +46,23 @@ export default function LanguagePicker({ } > - {Object.keys(languages).map(langKey => - langKey === locale ? null : ( - - {' '} -
- -{' '} - {languages[langKey].name} -
- - ), - )} + { + window.location.href = languages[key].url; + }} + > + {Object.keys(languages).map(langKey => + langKey === locale ? null : ( + + {' '} +
+ -{' '} + {languages[langKey].name} +
+ + ), + )} + ); } diff --git a/superset-frontend/src/dashboard/components/CssEditor.jsx b/superset-frontend/src/dashboard/components/CssEditor.jsx index 69f2500233..396276ff1c 100644 --- a/superset-frontend/src/dashboard/components/CssEditor.jsx +++ b/superset-frontend/src/dashboard/components/CssEditor.jsx @@ -81,7 +81,6 @@ class CssEditor extends React.PureComponent { {this.renderTemplateSelector()} diff --git a/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx b/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx index 35f9a3085c..ede085f4d9 100644 --- a/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx +++ b/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx @@ -20,8 +20,9 @@ import React from 'react'; import PropTypes from 'prop-types'; import { SupersetClient, t } from '@superset-ui/core'; -import { DropdownButton, MenuItem } from 'react-bootstrap'; +import { DropdownButton } from 'react-bootstrap'; +import { Menu } from 'src/common/components'; import Icon from 'src/components/Icon'; import CssEditor from './CssEditor'; @@ -71,6 +72,18 @@ const defaultProps = { refreshWarning: null, }; +const MENU_KEYS = { + SAVE_MODAL: 'save-modal', + SHARE_DASHBOARD: 'share-dashboard', + REFRESH_DASHBOARD: 'refresh-dashboard', + AUTOREFRESH_MODAL: 'autorefresh-modal', + SET_FILTER_MAPPING: 'set-filter-mapping', + EDIT_PROPERTIES: 'edit-properties', + EDIT_CSS: 'edit-css', + DOWNLOAD_AS_IMAGE: 'download-as-image', + TOGGLE_FULLSCREEN: 'toggle-fullscreen', +}; + class HeaderActionsDropdown extends React.PureComponent { static discardChanges() { window.location.reload(); @@ -85,6 +98,7 @@ class HeaderActionsDropdown extends React.PureComponent { this.changeCss = this.changeCss.bind(this); this.changeRefreshInterval = this.changeRefreshInterval.bind(this); + this.handleMenuClick = this.handleMenuClick.bind(this); } UNSAFE_componentWillMount() { @@ -119,12 +133,40 @@ class HeaderActionsDropdown extends React.PureComponent { this.props.startPeriodicRender(refreshInterval * 1000); } + handleMenuClick({ key, domEvent }) { + switch (key) { + case MENU_KEYS.REFRESH_DASHBOARD: + this.props.forceRefreshAllCharts(); + break; + case MENU_KEYS.EDIT_PROPERTIES: + this.props.showPropertiesModal(); + break; + case MENU_KEYS.DOWNLOAD_AS_IMAGE: + downloadAsImage('.dashboard', this.props.dashboardTitle)(domEvent); + break; + case MENU_KEYS.TOGGLE_FULLSCREEN: { + const hasStandalone = window.location.search.includes( + 'standalone=true', + ); + const url = getDashboardUrl( + window.location.pathname, + getActiveFilters(), + window.location.hash, + !hasStandalone, + ); + window.location.replace(url); + break; + } + default: + break; + } + } + render() { const { dashboardTitle, dashboardId, dashboardInfo, - forceRefreshAllCharts, refreshFrequency, shouldPersistRefreshFrequency, editMode, @@ -155,105 +197,102 @@ class HeaderActionsDropdown extends React.PureComponent { style={{ border: 'none', padding: 0, marginLeft: '4px' }} pullRight > - {userCanSave && ( - <> - {t('Save as')} - } - canOverwrite={userCanEdit} - /> - - )} - + {userCanSave && ( + + {t('Save as')} + } + canOverwrite={userCanEdit} + /> + )} - emailSubject={emailSubject} - emailContent={emailBody} - addDangerToast={this.props.addDangerToast} - isMenuItem - triggerNode={{t('Share dashboard')}} - /> - - {t('Refresh dashboard')} - - - {t('Set auto-refresh interval')}} - /> - - {editMode && ( - <> - {t('Set filter mapping')} - } - /> - - {t('Edit dashboard properties')} - - {t('Edit CSS')}} - initialCss={this.state.css} - templates={this.state.cssTemplates} - onChange={this.changeCss} - /> - - )} - - {!editMode && ( - - {t('Download as image')} - - )} - - {!editMode && ( - { - const hasStandalone = window.location.search.includes( - 'standalone=true', - ); - const url = getDashboardUrl( + + {t('Share dashboard')}} + /> + + - {t('Toggle FullScreen')} - - )} + {t('Refresh dashboard')} + + + + {t('Set auto-refresh interval')}} + /> + + + {editMode && ( + + + + )} + + {editMode && ( + + {t('Edit dashboard properties')} + + )} + + {editMode && ( + + {t('Edit CSS')}} + initialCss={this.state.css} + templates={this.state.cssTemplates} + onChange={this.changeCss} + /> + + )} + + {!editMode && ( + + {t('Download as image')} + + )} + + {!editMode && ( + + {t('Toggle FullScreen')} + + )} + ); } diff --git a/superset-frontend/src/dashboard/components/RefreshIntervalModal.jsx b/superset-frontend/src/dashboard/components/RefreshIntervalModal.jsx index 2710e66aa3..492444502f 100644 --- a/superset-frontend/src/dashboard/components/RefreshIntervalModal.jsx +++ b/superset-frontend/src/dashboard/components/RefreshIntervalModal.jsx @@ -94,7 +94,6 @@ class RefreshIntervalModal extends React.PureComponent { diff --git a/superset-frontend/src/dashboard/components/SaveModal.jsx b/superset-frontend/src/dashboard/components/SaveModal.jsx index e3a85b5bbe..46c4c55e2c 100644 --- a/superset-frontend/src/dashboard/components/SaveModal.jsx +++ b/superset-frontend/src/dashboard/components/SaveModal.jsx @@ -41,14 +41,12 @@ const propTypes = { colorNamespace: PropTypes.string, colorScheme: PropTypes.string, onSave: PropTypes.func.isRequired, - isMenuItem: PropTypes.bool, canOverwrite: PropTypes.bool.isRequired, refreshFrequency: PropTypes.number.isRequired, lastModifiedTime: PropTypes.number.isRequired, }; const defaultProps = { - isMenuItem: false, saveType: SAVE_TYPE_OVERWRITE, colorNamespace: undefined, colorScheme: undefined, @@ -157,7 +155,6 @@ class SaveModal extends React.PureComponent { return ( ( - this.setCustomRange('grain', value)} + {grain} - + )); const timeFrames = COMMON_TIME_FRAMES.map(timeFrame => { const nextState = getStateFromCommonTimeFrame(timeFrame); @@ -425,6 +422,7 @@ class DateFilterControl extends React.Component { ); }); + return (
-
- +
- this.setCustomRange('num', event.target.value) } onFocus={this.setTypeCustomRange} - onKeyPress={this.onEnter} + onPressEnter={this.close} value={this.state.num} - style={{ height: '30px' }} />
-
- +