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
2020-11-02 08:04:53 +01:00 · 2020-11-02 08:04:53 +01:00 · 19f2deb27f
parent 854a4614a8
commit 19f2deb27f
7 changed files with 205 additions and 211 deletions
--- 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 });
    });
  });

--- 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', () => {
--- 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();
  });
 });
--- 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}
--- 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>
  );
 }
--- 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>
    );
  }
--- 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)}
      >
        <img
          alt={type.name}
@ -157,86 +150,84 @@ export default class VizTypeControl extends React.PureComponent {
        </div>
      </div>
    );
-  }
+  };

-  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(
-        <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}
-              />
-            </div>
-            {rows}
-          </Modal.Body>
-        </Modal>
-      </div>
+  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>,
    );
  }
-}
+
+  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>
+          )}
+        </>
+      </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;