From 1490f3074d3416eb7b68b1fd0b012994b440fac1 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 5 Nov 2020 22:30:03 +0100 Subject: [PATCH] refactor: Replace react-bootstrap MenuItems with Antd Menu (#11554) * Refactor SliceHeaderControls * Refactor DisplayQueryButton * Fix duplicate keys * Refactor SliceAdder * Move css from styles to Emotion * Fix e2e test --- .../integration/dashboard/controls.test.js | 3 +- .../components/DisplayQueryButton_spec.jsx | 6 +- .../src/components/Menu/Menu.tsx | 2 +- .../src/components/ModalTrigger.jsx | 11 -- .../src/components/URLShortLinkModal.jsx | 2 - .../src/dashboard/components/SliceAdder.jsx | 38 ++-- .../components/SliceHeaderControls.jsx | 145 ++++++++------ .../stylesheets/components/chart.less | 12 -- .../src/dashboard/stylesheets/dashboard.less | 12 +- .../explore/components/DisplayQueryButton.jsx | 142 +++++++------ .../components/controls/DateFilterControl.jsx | 186 +++++++++--------- 11 files changed, 289 insertions(+), 270 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 6f9215ce24..8330eaba64 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/controls.test.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/controls.test.js @@ -81,8 +81,7 @@ describe('Dashboard top-level controls', () => { .next() .find('[data-test="dashboard-slice-refresh-tooltip"]') .parent() - .parent() - .should('have.class', 'disabled'); + .should('have.class', 'ant-menu-item-disabled'); cy.wait(`@postJson_${mapId}_force`); cy.get('[data-test="refresh-dashboard-menu-item"]').should( diff --git a/superset-frontend/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx b/superset-frontend/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx index e49487ebd8..4d1bb4556c 100644 --- a/superset-frontend/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx @@ -18,10 +18,10 @@ */ import React from 'react'; import { mount } from 'enzyme'; +import { supersetTheme, ThemeProvider } from '@superset-ui/core'; +import { Menu } from 'src/common/components'; import ModalTrigger from 'src/components/ModalTrigger'; import { DisplayQueryButton } from 'src/explore/components/DisplayQueryButton'; -import { MenuItem } from 'react-bootstrap'; -import { supersetTheme, ThemeProvider } from '@superset-ui/core'; describe('DisplayQueryButton', () => { const defaultProps = { @@ -51,6 +51,6 @@ describe('DisplayQueryButton', () => { }, }); expect(wrapper.find(ModalTrigger)).toHaveLength(3); - expect(wrapper.find(MenuItem)).toHaveLength(5); + expect(wrapper.find(Menu.Item)).toHaveLength(5); }); }); diff --git a/superset-frontend/src/components/Menu/Menu.tsx b/superset-frontend/src/components/Menu/Menu.tsx index 90b2e577f3..a193b08515 100644 --- a/superset-frontend/src/components/Menu/Menu.tsx +++ b/superset-frontend/src/components/Menu/Menu.tsx @@ -191,7 +191,7 @@ export function Menu({ , ]} {(navbarRight.version_string || navbarRight.version_sha) && [ - , + , {}, onExit: () => {}, isButton: false, - isMenuItem: false, className: '', modalTitle: '', }; @@ -102,14 +99,6 @@ export default class ModalTrigger extends React.Component { ); } - if (this.props.isMenuItem) { - return ( - <> - {this.props.triggerNode} - {this.renderModal()} - - ); - } /* eslint-disable jsx-a11y/interactive-supports-focus */ return ( <> diff --git a/superset-frontend/src/components/URLShortLinkModal.jsx b/superset-frontend/src/components/URLShortLinkModal.jsx index b999712eed..2b5bbfca8b 100644 --- a/superset-frontend/src/components/URLShortLinkModal.jsx +++ b/superset-frontend/src/components/URLShortLinkModal.jsx @@ -29,7 +29,6 @@ const propTypes = { emailSubject: PropTypes.string, emailContent: PropTypes.string, addDangerToast: PropTypes.func.isRequired, - isMenuItem: PropTypes.bool, title: PropTypes.string, triggerNode: PropTypes.node.isRequired, }; @@ -65,7 +64,6 @@ class URLShortLinkModal extends React.Component { return ( item.key === 'changed_on'), + sortBy: DEFAULT_SORT_KEY, selectedSliceIdsSet: new Set(props.selectedSliceIds), }; this.rowRenderer = this.rowRenderer.bind(this); @@ -102,7 +104,7 @@ class SliceAdder extends React.Component { if (nextProps.lastUpdated !== this.props.lastUpdated) { nextState.filteredSlices = Object.values(nextProps.slices) .filter(createFilter(this.state.searchTerm, KEYS_TO_FILTERS)) - .sort(SliceAdder.sortByComparator(KEYS_TO_SORT[this.state.sortBy].key)); + .sort(SliceAdder.sortByComparator(this.state.sortBy)); } if (nextProps.selectedSliceIds !== this.props.selectedSliceIds) { @@ -123,7 +125,7 @@ class SliceAdder extends React.Component { getFilteredSortedSlices(searchTerm, sortBy) { return Object.values(this.props.slices) .filter(createFilter(searchTerm, KEYS_TO_FILTERS)) - .sort(SliceAdder.sortByComparator(KEYS_TO_SORT[sortBy].key)); + .sort(SliceAdder.sortByComparator(sortBy)); } handleKeyPress(ev) { @@ -216,17 +218,17 @@ class SliceAdder extends React.Component { onKeyPress={this.handleKeyPress} data-test="dashboard-charts-filter-search-input" /> - - {KEYS_TO_SORT.map((item, index) => ( - - Sort by {item.label} - + {Object.entries(KEYS_TO_SORT).map(([key, label]) => ( + + Sort by {label} + ))} - + {this.props.isLoading && } {!this.props.isLoading && this.state.filteredSlices.length > 0 && ( diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls.jsx b/superset-frontend/src/dashboard/components/SliceHeaderControls.jsx index 1778ef5b21..a453daa004 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls.jsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls.jsx @@ -19,8 +19,9 @@ import React from 'react'; import PropTypes from 'prop-types'; import moment from 'moment'; -import { Dropdown, MenuItem } from 'react-bootstrap'; -import { t } from '@superset-ui/core'; +import { DropdownButton } from 'react-bootstrap'; +import { styled, t } from '@superset-ui/core'; +import { Menu } from 'src/common/components'; import URLShortLinkModal from '../../components/URLShortLinkModal'; import downloadAsImage from '../../utils/downloadAsImage'; import getDashboardUrl from '../util/getDashboardUrl'; @@ -58,41 +59,49 @@ const defaultProps = { sliceCanEdit: false, }; +const MENU_KEYS = { + FORCE_REFRESH: 'force_refresh', + TOGGLE_CHART_DESCRIPTION: 'toggle_chart_description', + EXPLORE_CHART: 'explore_chart', + EXPORT_CSV: 'export_csv', + RESIZE_LABEL: 'resize_label', + SHARE_CHART: 'share_chart', + DOWNLOAD_AS_IMAGE: 'download_as_image', +}; + +const VerticalDotsContainer = styled.div` + padding: ${({ theme }) => theme.gridUnit / 4}px + ${({ theme }) => theme.gridUnit * 1.5}px; + + .dot { + display: block; + } + + &:hover { + cursor: pointer; + } +`; + const VerticalDotsTrigger = () => ( -
+ -
+ ); class SliceHeaderControls extends React.PureComponent { constructor(props) { super(props); - this.exportCSV = this.exportCSV.bind(this); - this.exploreChart = this.exploreChart.bind(this); this.toggleControls = this.toggleControls.bind(this); this.refreshChart = this.refreshChart.bind(this); - this.toggleExpandSlice = this.props.toggleExpandSlice.bind( - this, - this.props.slice.slice_id, - ); - - this.handleToggleFullSize = this.handleToggleFullSize.bind(this); + this.handleMenuClick = this.handleMenuClick.bind(this); this.state = { showControls: false, }; } - exportCSV() { - this.props.exportCSV(this.props.slice.slice_id); - } - - exploreChart() { - this.props.exploreChart(this.props.slice.slice_id); - } - refreshChart() { if (this.props.updatedDttm) { this.props.forceRefresh( @@ -108,8 +117,32 @@ class SliceHeaderControls extends React.PureComponent { })); } - handleToggleFullSize() { - this.props.handleToggleFullSize(); + handleMenuClick({ key, domEvent }) { + switch (key) { + case MENU_KEYS.FORCE_REFRESH: + this.refreshChart(); + break; + case MENU_KEYS.TOGGLE_CHART_DESCRIPTION: + this.props.toggleExpandSlice(this.props.slice.slice_id); + break; + case MENU_KEYS.EXPLORE_CHART: + this.props.exploreChart(this.props.slice.slice_id); + break; + case MENU_KEYS.EXPORT_CSV: + this.props.exportCSV(this.props.slice.slice_id); + break; + case MENU_KEYS.RESIZE_LABEL: + this.props.handleToggleFullSize(); + break; + case MENU_KEYS.DOWNLOAD_AS_IMAGE: + downloadAsImage( + '.dashboard-component-chart-holder', + this.props.slice.slice_name, + )(domEvent); + break; + default: + break; + } } render() { @@ -129,21 +162,21 @@ class SliceHeaderControls extends React.PureComponent { : (updatedWhen && t('Fetched %s', updatedWhen)) || ''; const resizeLabel = isFullSize ? t('Minimize') : t('Maximize'); return ( - } + style={{ padding: 0 }} // react-bootstrap handles visibility, but call toggle to force a re-render // and update the fetched/cached timestamps onToggle={this.toggleControls} > - - - - - - + {t('Force refresh')}
{refreshTooltip}
- + - + {slice.description && ( - + {t('Toggle chart description')} - + )} {this.props.supersetCanExplore && ( - + {t('Explore chart')} - + )} {this.props.supersetCanCSV && ( - {t('Export CSV')} + {t('Export CSV')} )} - {resizeLabel} + {resizeLabel} - {t('Share chart')}} - /> + + {t('Share chart')}} + /> + - + {t('Download as image')} - - - + + + ); } } diff --git a/superset-frontend/src/dashboard/stylesheets/components/chart.less b/superset-frontend/src/dashboard/stylesheets/components/chart.less index b23ace530f..ebf2e93559 100644 --- a/superset-frontend/src/dashboard/stylesheets/components/chart.less +++ b/superset-frontend/src/dashboard/stylesheets/components/chart.less @@ -112,14 +112,6 @@ } } -.slice-header-controls-trigger { - padding: 2px 6px; - - &:hover { - cursor: pointer; - } -} - .dot { @dot-diameter: 4px; @@ -131,10 +123,6 @@ background-color: @gray; display: inline-block; - .vertical-dots-container & { - display: block; - } - a[role='menuitem'] & { width: 8px; height: 8px; diff --git a/superset-frontend/src/dashboard/stylesheets/dashboard.less b/superset-frontend/src/dashboard/stylesheets/dashboard.less index c86a7d983a..02f718cdfb 100644 --- a/superset-frontend/src/dashboard/stylesheets/dashboard.less +++ b/superset-frontend/src/dashboard/stylesheets/dashboard.less @@ -138,16 +138,8 @@ body { padding: 9px 0; } - .dropdown-menu li a { - padding: 3px 16px; - color: @almost-black; - font-size: @font-size-m; - - &:hover, - &:focus { - background: @menu-hover; - color: @almost-black; - } + .dropdown-menu li { + font-weight: @font-weight-normal; } } diff --git a/superset-frontend/src/explore/components/DisplayQueryButton.jsx b/superset-frontend/src/explore/components/DisplayQueryButton.jsx index d97c8e6ff3..0b4d993cc5 100644 --- a/superset-frontend/src/explore/components/DisplayQueryButton.jsx +++ b/superset-frontend/src/explore/components/DisplayQueryButton.jsx @@ -26,16 +26,11 @@ import markdownSyntax from 'react-syntax-highlighter/dist/cjs/languages/hljs/mar import sqlSyntax from 'react-syntax-highlighter/dist/cjs/languages/hljs/sql'; import jsonSyntax from 'react-syntax-highlighter/dist/cjs/languages/hljs/json'; import github from 'react-syntax-highlighter/dist/cjs/styles/hljs/github'; -import { - DropdownButton, - MenuItem, - Row, - Col, - FormControl, -} from 'react-bootstrap'; +import { DropdownButton, Row, Col, FormControl } from 'react-bootstrap'; import { Table } from 'reactable-arc'; import { t } from '@superset-ui/core'; +import { Menu } from 'src/common/components'; import Button from 'src/components/Button'; import getClientErrorObject from '../../utils/getClientErrorObject'; import CopyToClipboard from '../../components/CopyToClipboard'; @@ -65,6 +60,12 @@ const propTypes = { slice: PropTypes.object, }; +const MENU_KEYS = { + EDIT_PROPERTIES: 'edit_properties', + RUN_IN_SQL_LAB: 'run_in_sql_lab', + DOWNLOAD_AS_IMAGE: 'download_as_image', +}; + export class DisplayQueryButton extends React.PureComponent { constructor(props) { super(props); @@ -81,8 +82,8 @@ export class DisplayQueryButton extends React.PureComponent { }; this.beforeOpen = this.beforeOpen.bind(this); this.changeFilterText = this.changeFilterText.bind(this); - this.openPropertiesModal = this.openPropertiesModal.bind(this); this.closePropertiesModal = this.closePropertiesModal.bind(this); + this.handleMenuClick = this.handleMenuClick.bind(this); } beforeOpen(resultType) { @@ -118,10 +119,6 @@ export class DisplayQueryButton extends React.PureComponent { this.setState({ filterText: event.target.value }); } - redirectSQLLab() { - this.props.onOpenInEditor(this.props.latestQueryFormData); - } - openPropertiesModal() { this.setState({ isPropertiesModalOpen: true }); } @@ -130,6 +127,35 @@ export class DisplayQueryButton extends React.PureComponent { this.setState({ isPropertiesModalOpen: false }); } + handleMenuClick({ key, domEvent }) { + const { + chartHeight, + slice, + onOpenInEditor, + latestQueryFormData, + } = this.props; + switch (key) { + case MENU_KEYS.EDIT_PROPERTIES: + this.openPropertiesModal(); + break; + case MENU_KEYS.RUN_IN_SQL_LAB: + onOpenInEditor(latestQueryFormData); + break; + case MENU_KEYS.DOWNLOAD_AS_IMAGE: + downloadAsImage( + '.chart-container', + // eslint-disable-next-line camelcase + slice?.slice_name ?? t('New chart'), + { + height: parseInt(chartHeight, 10), + }, + )(domEvent); + break; + default: + break; + } + } + renderQueryModalBody() { if (this.state.isLoading) { return ; @@ -230,7 +256,7 @@ export class DisplayQueryButton extends React.PureComponent { } render() { - const { chartHeight, slice } = this.props; + const { slice } = this.props; return ( - {slice && ( - <> - + + {slice && [ + {t('Edit properties')} - + , , + ]} + + {t('View query')} + } + modalTitle={t('View query')} + beforeOpen={() => this.beforeOpen('query')} + modalBody={this.renderQueryModalBody()} + responsive /> - - )} - {t('View query')} - } - modalTitle={t('View query')} - beforeOpen={() => this.beforeOpen('query')} - modalBody={this.renderQueryModalBody()} - responsive - /> - {t('View results')}} - modalTitle={t('View results')} - beforeOpen={() => this.beforeOpen('results')} - modalBody={this.renderResultsModalBody()} - responsive - /> - {t('View samples')}} - modalTitle={t('View samples')} - beforeOpen={() => this.beforeOpen('samples')} - modalBody={this.renderSamplesModalBody()} - responsive - /> - {this.state.sqlSupported && ( - - {t('Run in SQL Lab')} - - )} - + + {t('View results')}} + modalTitle={t('View results')} + beforeOpen={() => this.beforeOpen('results')} + modalBody={this.renderResultsModalBody()} + responsive + /> + + + {t('View samples')}} + modalTitle={t('View samples')} + beforeOpen={() => this.beforeOpen('samples')} + modalBody={this.renderSamplesModalBody()} + responsive + /> + + {this.state.sqlSupported && ( + + {t('Run in SQL Lab')} + )} - > - {t('Download as image')} - + + {t('Download as image')} + + ); } diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl.jsx b/superset-frontend/src/explore/components/controls/DateFilterControl.jsx index 335f963b16..d59649075e 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl.jsx +++ b/superset-frontend/src/explore/components/controls/DateFilterControl.jsx @@ -88,8 +88,6 @@ const FREEFORM_TOOLTIP = t( '`2 weeks from now` can be used.', ); -const DATE_FILTER_POPOVER_STYLE = { width: '250px' }; - const propTypes = { animation: PropTypes.bool, name: PropTypes.string.isRequired, @@ -173,12 +171,34 @@ function getStateFromCustomRange(value) { }; } -const Styles = styled.div` +const TimeFramesStyles = styled.div` .radio { margin: ${({ theme }) => theme.gridUnit}px 0; } `; +const PopoverContentStyles = styled.div` + width: ${({ theme }) => theme.gridUnit * 60}px; + + .timeframes-container { + margin-left: ${({ theme }) => theme.gridUnit * 2}px; + } + + .relative-timerange-container { + display: flex; + margin-top: ${({ theme }) => theme.gridUnit * 2}px; + } + + .timerange-input { + width: ${({ theme }) => theme.gridUnit * 15}px; + margin: 0 ${({ theme }) => theme.gridUnit}px; + } + + .datetime { + margin: ${({ theme }) => theme.gridUnit}px 0; + } +`; + class DateFilterControl extends React.Component { constructor(props) { super(props); @@ -399,7 +419,7 @@ class DateFilterControl extends React.Component { const timeRange = buildTimeRangeString(nextState.since, nextState.until); return ( - + - + ); }); return ( -
{ this.popoverContainer = ref; }} @@ -438,7 +457,7 @@ class DateFilterControl extends React.Component { onSelect={this.changeTab} > -
+
{timeFrames}
@@ -449,48 +468,37 @@ class DateFilterControl extends React.Component { isSelected={this.state.type === TYPES.CUSTOM_RANGE} onSelect={this.setTypeCustomRange} > -
-
- -
-
+ - this.setCustomRange('num', event.target.value) - } - onFocus={this.setTypeCustomRange} - onPressEnter={this.close} - value={this.state.num} - /> -
-
- -
+ + Last + + + Next + + + + this.setCustomRange('num', event.target.value) + } + onFocus={this.setTypeCustomRange} + onPressEnter={this.close} + value={this.state.num} + /> +
-
- - this.setCustomStartEnd('since', value) - } - isValidDate={this.isValidSince} - onClick={this.setTypeCustomStartEnd} - renderInput={props => - this.renderInput(props, 'showSinceCalendar') - } - open={this.state.showSinceCalendar} - viewMode={this.state.sinceViewMode} - onViewModeChange={sinceViewMode => - this.setState({ sinceViewMode }) - } - /> -
-
- - this.setCustomStartEnd('until', value) - } - isValidDate={this.isValidUntil} - onClick={this.setTypeCustomStartEnd} - renderInput={props => - this.renderInput(props, 'showUntilCalendar') - } - open={this.state.showUntilCalendar} - viewMode={this.state.untilViewMode} - onViewModeChange={untilViewMode => - this.setState({ untilViewMode }) - } - /> -
+ this.setCustomStartEnd('since', value)} + isValidDate={this.isValidSince} + onClick={this.setTypeCustomStartEnd} + renderInput={props => + this.renderInput(props, 'showSinceCalendar') + } + open={this.state.showSinceCalendar} + viewMode={this.state.sinceViewMode} + onViewModeChange={sinceViewMode => + this.setState({ sinceViewMode }) + } + /> + this.setCustomStartEnd('until', value)} + isValidDate={this.isValidUntil} + onClick={this.setTypeCustomStartEnd} + renderInput={props => + this.renderInput(props, 'showUntilCalendar') + } + open={this.state.showUntilCalendar} + viewMode={this.state.untilViewMode} + onViewModeChange={untilViewMode => + this.setState({ untilViewMode }) + } + />
@@ -564,7 +566,7 @@ class DateFilterControl extends React.Component { Ok
- + ); }