From 19f2deb27f2a869a542e4d70d7f2c036b219b196 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Mon, 2 Nov 2020 08:04:53 +0100 Subject: [PATCH] refactor: Replace react-bootstrap Modals with Antd in Explore (#11389) * VizTypeControl * SaveModal * explore/PropertiesModal * Fix e2e tests * Remove console logs * Fix tests * Fix test * Fix e2e test * Remove unnecessary fragment * Fix e2e tests * Fix e2e test --- .../cypress/integration/explore/link.test.js | 4 +- .../explore/components/SaveModal_spec.jsx | 7 +- .../components/VizTypeControl_spec.jsx | 8 +- .../src/common/components/Modal/Modal.tsx | 4 +- .../explore/components/PropertiesModal.tsx | 95 ++++---- .../src/explore/components/SaveModal.jsx | 79 +++---- .../components/controls/VizTypeControl.jsx | 219 +++++++++--------- 7 files changed, 205 insertions(+), 211 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/explore/link.test.js b/superset-frontend/cypress-base/cypress/integration/explore/link.test.js index 029527ffcb..142e6187d9 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/link.test.js +++ b/superset-frontend/cypress-base/cypress/integration/explore/link.test.js @@ -45,8 +45,8 @@ describe('Test explore links', () => { cy.wait('@postJson').then(() => { cy.get('code'); }); - cy.get('.modal-header').within(() => { - cy.get('button.close').first().click({ force: true }); + cy.get('.ant-modal-content').within(() => { + cy.get('button.ant-modal-close').first().click({ force: true }); }); }); diff --git a/superset-frontend/spec/javascripts/explore/components/SaveModal_spec.jsx b/superset-frontend/spec/javascripts/explore/components/SaveModal_spec.jsx index 28e9aee8f7..a35223213c 100644 --- a/superset-frontend/spec/javascripts/explore/components/SaveModal_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/SaveModal_spec.jsx @@ -23,11 +23,12 @@ import { bindActionCreators } from 'redux'; import { shallow } from 'enzyme'; import { styledMount as mount } from 'spec/helpers/theming'; -import { FormControl, Modal, Radio } from 'react-bootstrap'; +import { FormControl, Radio } from 'react-bootstrap'; import Button from 'src/components/Button'; import sinon from 'sinon'; import fetchMock from 'fetch-mock'; +import Modal from 'src/common/components/Modal'; import * as exploreUtils from 'src/explore/exploreUtils'; import * as saveModalActions from 'src/explore/actions/saveModalActions'; import SaveModal from 'src/explore/components/SaveModal'; @@ -79,8 +80,10 @@ describe('SaveModal', () => { const wrapper = getWrapper(); expect(wrapper.find(Modal)).toExist(); expect(wrapper.find(FormControl)).toExist(); - expect(wrapper.find(Button)).toHaveLength(3); expect(wrapper.find(Radio)).toHaveLength(2); + + const footerWrapper = shallow(wrapper.find('Modal').props().footer); + expect(footerWrapper.find(Button)).toHaveLength(3); }); it('overwrite radio button is disabled for new slice', () => { diff --git a/superset-frontend/spec/javascripts/explore/components/VizTypeControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/VizTypeControl_spec.jsx index cce63b0c57..496f689712 100644 --- a/superset-frontend/spec/javascripts/explore/components/VizTypeControl_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/VizTypeControl_spec.jsx @@ -19,9 +19,9 @@ import React from 'react'; import sinon from 'sinon'; import { shallow } from 'enzyme'; -import { Modal } from 'react-bootstrap'; import { getChartMetadataRegistry, ChartMetadata } from '@superset-ui/core'; import VizTypeControl from 'src/explore/components/controls/VizTypeControl'; +import Modal from 'src/common/components/Modal'; const defaultProps = { name: 'viz_type', @@ -65,7 +65,11 @@ describe('VizTypeControl', () => { }); it('filters images based on text input', () => { expect(wrapper.find('img')).toHaveLength(2); - wrapper.setState({ filter: 'vis2' }); + wrapper.find('FormControl').simulate('change', { + target: { + value: 'vis2', + }, + }); expect(wrapper.find('img')).toExist(); }); }); diff --git a/superset-frontend/src/common/components/Modal/Modal.tsx b/superset-frontend/src/common/components/Modal/Modal.tsx index 35e0d41962..9b3b73f5e2 100644 --- a/superset-frontend/src/common/components/Modal/Modal.tsx +++ b/superset-frontend/src/common/components/Modal/Modal.tsx @@ -39,6 +39,7 @@ interface ModalProps { hideFooter?: boolean; centered?: boolean; footer?: React.ReactNode; + wrapProps?: object; } interface StyledModalProps extends SupersetThemeProps { @@ -120,6 +121,7 @@ export default function Modal({ centered, footer, hideFooter, + wrapProps, ...rest }: ModalProps) { const modalFooter = isNil(footer) @@ -157,7 +159,7 @@ export default function Modal({ } footer={!hideFooter ? modalFooter : null} - wrapProps={{ 'data-test': `${title}-modal` }} + wrapProps={{ 'data-test': `${title}-modal`, ...wrapProps }} {...rest} > {children} diff --git a/superset-frontend/src/explore/components/PropertiesModal.tsx b/superset-frontend/src/explore/components/PropertiesModal.tsx index eccc6bab73..93a6daebdc 100644 --- a/superset-frontend/src/explore/components/PropertiesModal.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal.tsx @@ -18,13 +18,13 @@ */ import React, { useState, useEffect, useRef, useCallback } from 'react'; import { - Modal, Row, Col, FormControl, FormGroup, FormControlProps, } from 'react-bootstrap'; +import Modal from 'src/common/components/Modal'; import Button from 'src/components/Button'; import Dialog from 'react-bootstrap-dialog'; import { OptionsType } from 'react-select/src/types'; @@ -35,10 +35,11 @@ import Chart, { Slice } from 'src/types/Chart'; import FormLabel from 'src/components/FormLabel'; import getClientErrorObject from '../../utils/getClientErrorObject'; -type InternalProps = { +type PropertiesModalProps = { slice: Slice; onHide: () => void; onSave: (chart: Chart) => void; + show: boolean; }; type OwnerOption = { @@ -46,12 +47,12 @@ type OwnerOption = { value: number; }; -export type WrapperProps = InternalProps & { - show: boolean; - animation?: boolean; // for the modal -}; - -function PropertiesModal({ slice, onHide, onSave }: InternalProps) { +export default function PropertiesModal({ + slice, + onHide, + onSave, + show, +}: PropertiesModalProps) { const [submitting, setSubmitting] = useState(false); const errorDialog = useRef(null); @@ -157,11 +158,40 @@ function PropertiesModal({ slice, onHide, onSave }: InternalProps) { }; return ( -
- - Edit Chart Properties - - + + + + + + } + responsive + wrapProps={{ 'data-test': 'properties-edit-modal' }} + > +

{t('Basic Information')}

@@ -247,44 +277,7 @@ function PropertiesModal({ slice, onHide, onSave }: InternalProps) { - - - - - - -
- ); -} - -export default function PropertiesModalWrapper({ - show, - onHide, - animation, - slice, - onSave, -}: WrapperProps) { - // The wrapper is a separate component so that hooks only run when the modal opens - return ( - - + ); } diff --git a/superset-frontend/src/explore/components/SaveModal.jsx b/superset-frontend/src/explore/components/SaveModal.jsx index fb32a8e5a4..df6e2bea50 100644 --- a/superset-frontend/src/explore/components/SaveModal.jsx +++ b/superset-frontend/src/explore/components/SaveModal.jsx @@ -20,12 +20,13 @@ import React from 'react'; import PropTypes from 'prop-types'; import { connect } from 'react-redux'; -import { Alert, FormControl, FormGroup, Modal, Radio } from 'react-bootstrap'; +import { Alert, FormControl, FormGroup, Radio } from 'react-bootstrap'; +import { t } from '@superset-ui/core'; +import ReactMarkdown from 'react-markdown'; +import Modal from 'src/common/components/Modal'; import Button from 'src/components/Button'; import FormLabel from 'src/components/FormLabel'; import { CreatableSelect } from 'src/components/Select/SupersetStyledSelect'; -import { t } from '@superset-ui/core'; -import ReactMarkdown from 'react-markdown'; const propTypes = { can_overwrite: PropTypes.bool, @@ -132,11 +133,41 @@ class SaveModal extends React.Component { render() { return ( - - - {t('Save Chart')} - - + + + + + + } + > +
{(this.state.alert || this.props.alert) && ( {this.state.alert ? this.state.alert : this.props.alert} @@ -207,37 +238,7 @@ class SaveModal extends React.Component { } /> - - - -
- - - -
- +
); } diff --git a/superset-frontend/src/explore/components/controls/VizTypeControl.jsx b/superset-frontend/src/explore/components/controls/VizTypeControl.jsx index 4f196a7259..d659b46578 100644 --- a/superset-frontend/src/explore/components/controls/VizTypeControl.jsx +++ b/superset-frontend/src/explore/components/controls/VizTypeControl.jsx @@ -16,18 +16,17 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; +import React, { useEffect, useRef, useState } from 'react'; import PropTypes from 'prop-types'; import { Row, Col, FormControl, - Modal, OverlayTrigger, Tooltip, } from 'react-bootstrap'; import { t, getChartMetadataRegistry } from '@superset-ui/core'; - +import Modal from 'src/common/components/Modal'; import Label from 'src/components/Label'; import ControlHeader from '../ControlHeader'; import './VizTypeControl.less'; @@ -98,44 +97,38 @@ const DEFAULT_ORDER = [ const typesWithDefaultOrder = new Set(DEFAULT_ORDER); -export default class VizTypeControl extends React.PureComponent { - constructor(props) { - super(props); - this.state = { - showModal: false, - filter: '', - }; - this.toggleModal = this.toggleModal.bind(this); - this.changeSearch = this.changeSearch.bind(this); - this.setSearchRef = this.setSearchRef.bind(this); - this.focusSearch = this.focusSearch.bind(this); - } +const VizTypeControl = props => { + const [showModal, setShowModal] = useState(false); + const [filter, setFilter] = useState(''); + const searchRef = useRef(null); - onChange(vizType) { - this.props.onChange(vizType); - this.setState({ showModal: false }); - } - - setSearchRef(searchRef) { - this.searchRef = searchRef; - } - - toggleModal() { - this.setState(prevState => ({ showModal: !prevState.showModal })); - } - - changeSearch(event) { - this.setState({ filter: event.target.value }); - } - - focusSearch() { - if (this.searchRef) { - this.searchRef.focus(); + useEffect(() => { + if (showModal) { + searchRef?.current?.focus(); } - } + }, [showModal]); - renderItem(entry) { - const { value } = this.props; + const onChange = vizType => { + props.onChange(vizType); + setShowModal(false); + }; + + const toggleModal = () => { + setShowModal(prevState => !prevState); + }; + + const changeSearch = event => { + setFilter(event.target.value); + }; + + const focusSearch = () => { + if (searchRef) { + searchRef.focus(); + } + }; + + const renderItem = entry => { + const { value } = props; const { key, value: type } = entry; const isSelected = key === value; @@ -144,7 +137,7 @@ export default class VizTypeControl extends React.PureComponent { role="button" tabIndex={0} className={`viztype-selector-container ${isSelected ? 'selected' : ''}`} - onClick={this.onChange.bind(this, key)} + onClick={() => onChange(key)} > {type.name} ); - } + }; - render() { - const { filter, showModal } = this.state; - const { value, labelBsStyle } = this.props; + const { value, labelBsStyle } = props; + const filterString = filter.toLowerCase(); - const filterString = filter.toLowerCase(); - const filteredTypes = DEFAULT_ORDER.filter(type => registry.has(type)) - .map(type => ({ - key: type, - value: registry.get(type), - })) - .concat( - registry.entries().filter(({ key }) => !typesWithDefaultOrder.has(key)), - ) - .filter(entry => entry.value.name.toLowerCase().includes(filterString)); + const filteredTypes = DEFAULT_ORDER.filter(type => registry.has(type)) + .map(type => ({ + key: type, + value: registry.get(type), + })) + .concat( + registry.entries().filter(({ key }) => !typesWithDefaultOrder.has(key)), + ) + .filter(entry => entry.value.name.toLowerCase().includes(filterString)); - const rows = []; - for (let i = 0; i <= filteredTypes.length; i += IMAGE_PER_ROW) { - rows.push( - - {filteredTypes.slice(i, i + IMAGE_PER_ROW).map(entry => ( - - {this.renderItem(entry)} - - ))} - , - ); - } - - return ( -
- - - {t('Click to change visualization type')} - - } - > - <> - - {!registry.has(value) && ( -
- {' '} - {t('This visualization type is not supported.')} -
- )} - - - - - {t('Select a visualization type')} - - -
- -
- {rows} - - -
+ const rows = []; + for (let i = 0; i <= filteredTypes.length; i += IMAGE_PER_ROW) { + rows.push( + + {filteredTypes.slice(i, i + IMAGE_PER_ROW).map(entry => ( + + {renderItem(entry)} + + ))} + , ); } -} + + return ( +
+ + + {t('Click to change visualization type')} + + } + > + <> + + {!registry.has(value) && ( +
+ {' '} + {t('This visualization type is not supported.')} +
+ )} + + + +
+ { + searchRef.current = ref; + }} + type="text" + value={filter} + placeholder={t('Search')} + onChange={changeSearch} + /> +
+ {rows} + +
+ ); +}; VizTypeControl.propTypes = propTypes; VizTypeControl.defaultProps = defaultProps; + +export default VizTypeControl;