This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 19f2deb refactor: Replace react-bootstrap Modals with Antd in Explore
(#11389)
19f2deb is described below
commit 19f2deb27f2a869a542e4d70d7f2c036b219b196
Author: Kamil Gabryjelski <[email protected]>
AuthorDate: Mon Nov 2 08:04:53 2020 +0100
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 +-
.../explore/components/VizTypeControl_spec.jsx | 8 +-
.../src/common/components/Modal/Modal.tsx | 4 +-
.../src/explore/components/PropertiesModal.tsx | 95 +++++----
.../src/explore/components/SaveModal.jsx | 79 ++++----
.../explore/components/controls/VizTypeControl.jsx | 215 ++++++++++-----------
7 files changed, 203 insertions(+), 209 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 029527f..142e618 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 28e9aee..a352232 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 cce63b0..496f689 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 35e0d41..9b3b73f 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({
</span>
}
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 eccc6ba..93a6dae 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<any>(null);
@@ -157,11 +158,40 @@ function PropertiesModal({ slice, onHide, onSave }:
InternalProps) {
};
return (
- <form onSubmit={onSubmit}>
- <Modal.Header data-test="properties-edit-modal" closeButton>
- <Modal.Title>Edit Chart Properties</Modal.Title>
- </Modal.Header>
- <Modal.Body>
+ <Modal
+ show={show}
+ onHide={onHide}
+ title="Edit Chart Properties"
+ footer={
+ <>
+ <Button
+ data-test="properties-modal-cancel-button"
+ type="button"
+ buttonSize="sm"
+ onClick={onHide}
+ cta
+ >
+ {t('Cancel')}
+ </Button>
+ <Button
+ data-test="properties-modal-save-button"
+ type="button"
+ buttonSize="sm"
+ buttonStyle="primary"
+ // @ts-ignore
+ onClick={onSubmit}
+ disabled={!owners || submitting || !name}
+ cta
+ >
+ {t('Save')}
+ </Button>
+ <Dialog ref={errorDialog} />
+ </>
+ }
+ responsive
+ wrapProps={{ 'data-test': 'properties-edit-modal' }}
+ >
+ <form onSubmit={onSubmit}>
<Row>
<Col md={6}>
<h3>{t('Basic Information')}</h3>
@@ -247,44 +277,7 @@ function PropertiesModal({ slice, onHide, onSave }:
InternalProps) {
</FormGroup>
</Col>
</Row>
- </Modal.Body>
- <Modal.Footer>
- <Button
- data-test="properties-modal-cancel-button"
- type="button"
- buttonSize="sm"
- onClick={onHide}
- cta
- >
- {t('Cancel')}
- </Button>
- <Button
- data-test="properties-modal-save-button"
- type="submit"
- buttonSize="sm"
- buttonStyle="primary"
- disabled={!owners || submitting || !name}
- cta
- >
- {t('Save')}
- </Button>
- <Dialog ref={errorDialog} />
- </Modal.Footer>
- </form>
- );
-}
-
-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 (
- <Modal show={show} onHide={onHide} animation={animation} bsSize="large">
- <PropertiesModal slice={slice} onHide={onHide} onSave={onSave} />
+ </form>
</Modal>
);
}
diff --git a/superset-frontend/src/explore/components/SaveModal.jsx
b/superset-frontend/src/explore/components/SaveModal.jsx
index fb32a8e..df6e2be 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 (
- <Modal show onHide={this.props.onHide}>
- <Modal.Header closeButton>
- <Modal.Title>{t('Save Chart')}</Modal.Title>
- </Modal.Header>
- <Modal.Body data-test="save-modal-body">
+ <Modal
+ show
+ onHide={this.props.onHide}
+ title={t('Save Chart')}
+ footer={
+ <div data-test="save-modal-footer">
+ <Button id="btn_cancel" buttonSize="sm"
onClick={this.props.onHide}>
+ {t('Cancel')}
+ </Button>
+ <Button
+ id="btn_modal_save_goto_dash"
+ buttonSize="sm"
+ disabled={
+ !this.state.newSliceName || !this.state.newDashboardName
+ }
+ onClick={this.saveOrOverwrite.bind(this, true)}
+ >
+ {t('Save & go to dashboard')}
+ </Button>
+ <Button
+ id="btn_modal_save"
+ buttonSize="sm"
+ buttonStyle="primary"
+ onClick={this.saveOrOverwrite.bind(this, false)}
+ disabled={!this.state.newSliceName}
+ data-test="btn-modal-save"
+ >
+ {!this.props.can_overwrite && this.props.slice
+ ? t('Save as new chart')
+ : t('Save')}
+ </Button>
+ </div>
+ }
+ >
+ <div data-test="save-modal-body">
{(this.state.alert || this.props.alert) && (
<Alert>
{this.state.alert ? this.state.alert : this.props.alert}
@@ -207,37 +238,7 @@ class SaveModal extends React.Component {
}
/>
</FormGroup>
- </Modal.Body>
-
- <Modal.Footer data-test="save-modal-footer">
- <div className="float-right">
- <Button id="btn_cancel" buttonSize="sm"
onClick={this.props.onHide}>
- {t('Cancel')}
- </Button>
- <Button
- id="btn_modal_save_goto_dash"
- buttonSize="sm"
- disabled={
- !this.state.newSliceName || !this.state.newDashboardName
- }
- onClick={this.saveOrOverwrite.bind(this, true)}
- >
- {t('Save & go to dashboard')}
- </Button>
- <Button
- id="btn_modal_save"
- buttonSize="sm"
- buttonStyle="primary"
- onClick={this.saveOrOverwrite.bind(this, false)}
- disabled={!this.state.newSliceName}
- data-test="btn-modal-save"
- >
- {!this.props.can_overwrite && this.props.slice
- ? t('Save as new chart')
- : t('Save')}
- </Button>
- </div>
- </Modal.Footer>
+ </div>
</Modal>
);
}
diff --git
a/superset-frontend/src/explore/components/controls/VizTypeControl.jsx
b/superset-frontend/src/explore/components/controls/VizTypeControl.jsx
index 4f196a7..d659b46 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 });
- }
+ useEffect(() => {
+ if (showModal) {
+ searchRef?.current?.focus();
+ }
+ }, [showModal]);
- setSearchRef(searchRef) {
- this.searchRef = searchRef;
- }
+ const onChange = vizType => {
+ props.onChange(vizType);
+ setShowModal(false);
+ };
- toggleModal() {
- this.setState(prevState => ({ showModal: !prevState.showModal }));
- }
+ const toggleModal = () => {
+ setShowModal(prevState => !prevState);
+ };
- changeSearch(event) {
- this.setState({ filter: event.target.value });
- }
+ const changeSearch = event => {
+ setFilter(event.target.value);
+ };
- focusSearch() {
- if (this.searchRef) {
- this.searchRef.focus();
+ const focusSearch = () => {
+ if (searchRef) {
+ searchRef.focus();
}
- }
+ };
- renderItem(entry) {
- const { value } = this.props;
+ 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)}
>
<img
alt={type.name}
@@ -157,86 +150,84 @@ export default class VizTypeControl extends
React.PureComponent {
</div>
</div>
);
+ };
+
+ const { value, labelBsStyle } = props;
+ 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 rows = [];
+ for (let i = 0; i <= filteredTypes.length; i += IMAGE_PER_ROW) {
+ rows.push(
+ <Row data-test="viz-row" key={`row-${i}`}>
+ {filteredTypes.slice(i, i + IMAGE_PER_ROW).map(entry => (
+ <Col md={12 / IMAGE_PER_ROW} key={`grid-col-${entry.key}`}>
+ {renderItem(entry)}
+ </Col>
+ ))}
+ </Row>,
+ );
}
- render() {
- const { filter, showModal } = this.state;
- const { value, labelBsStyle } = this.props;
-
- 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 rows = [];
- for (let i = 0; i <= filteredTypes.length; i += IMAGE_PER_ROW) {
- rows.push(
- <Row data-test="viz-row" key={`row-${i}`}>
- {filteredTypes.slice(i, i + IMAGE_PER_ROW).map(entry => (
- <Col md={12 / IMAGE_PER_ROW} key={`grid-col-${entry.key}`}>
- {this.renderItem(entry)}
- </Col>
- ))}
- </Row>,
- );
- }
-
- return (
- <div>
- <ControlHeader {...this.props} />
- <OverlayTrigger
- placement="right"
- overlay={
- <Tooltip id="error-tooltip">
- {t('Click to change visualization type')}
- </Tooltip>
- }
- >
- <>
- <Label onClick={this.toggleModal} bsStyle={labelBsStyle}>
- {registry.has(value) ? registry.get(value).name : `${value}`}
- </Label>
- {!registry.has(value) && (
- <div className="text-danger">
- <i className="fa fa-exclamation-circle text-danger" />{' '}
- <small>{t('This visualization type is not supported.')}</small>
- </div>
- )}
- </>
- </OverlayTrigger>
- <Modal
- show={showModal}
- onHide={this.toggleModal}
- onEnter={this.focusSearch}
- onExit={this.setSearchRef}
- bsSize="large"
- >
- <Modal.Header closeButton>
- <Modal.Title>{t('Select a visualization type')}</Modal.Title>
- </Modal.Header>
- <Modal.Body>
- <div className="viztype-control-search-box">
- <FormControl
- inputRef={this.setSearchRef}
- type="text"
- value={filter}
- placeholder={t('Search')}
- onChange={this.changeSearch}
- />
+ return (
+ <div>
+ <ControlHeader {...props} />
+ <OverlayTrigger
+ placement="right"
+ overlay={
+ <Tooltip id="error-tooltip">
+ {t('Click to change visualization type')}
+ </Tooltip>
+ }
+ >
+ <>
+ <Label onClick={toggleModal} bsStyle={labelBsStyle}>
+ {registry.has(value) ? registry.get(value).name : `${value}`}
+ </Label>
+ {!registry.has(value) && (
+ <div className="text-danger">
+ <i className="fa fa-exclamation-circle text-danger" />{' '}
+ <small>{t('This visualization type is not supported.')}</small>
</div>
- {rows}
- </Modal.Body>
- </Modal>
- </div>
- );
- }
-}
+ )}
+ </>
+ </OverlayTrigger>
+ <Modal
+ show={showModal}
+ onHide={toggleModal}
+ onEnter={focusSearch}
+ title={t('Select a visualization type')}
+ responsive
+ hideFooter
+ forceRender
+ >
+ <div className="viztype-control-search-box">
+ <FormControl
+ inputRef={ref => {
+ searchRef.current = ref;
+ }}
+ type="text"
+ value={filter}
+ placeholder={t('Search')}
+ onChange={changeSearch}
+ />
+ </div>
+ {rows}
+ </Modal>
+ </div>
+ );
+};
VizTypeControl.propTypes = propTypes;
VizTypeControl.defaultProps = defaultProps;
+
+export default VizTypeControl;
