refactor: Replace react-bootstrap dialogs with Antd dialogs (#11527)

* Refactor Dialogs in dashboard properties modal

* Refactor Dialogs in explore properties modal

* Refactor dialogs in DatasourceModal

* Refactor dialogs in ExploreResultsButton

* Remove react-bootstrap-dialog from ExploreCtasResultsButton

* Remove react-bootstrap-dialog dependency

* Remove unnecessary functions from Modal

* Bump antd version to fix a bug

* Fix unit tests

* Fix e2e test

* Change antd version to 4.5.4 to fix tests

* Reenable all tests in control

* Another version bump to fix tests
This commit is contained in:
Kamil Gabryjelski 2020-11-04 07:07:21 +01:00 committed by GitHub
parent 1ebeffa104
commit 937f9ca277
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 617 additions and 434 deletions

View File

@ -60,7 +60,7 @@ describe('Datasource control', () => {
.clear() .clear()
.type(`${newMetricName}{enter}`); .type(`${newMetricName}{enter}`);
cy.get('[data-test="datasource-modal-save"]').click(); cy.get('[data-test="datasource-modal-save"]').click();
cy.get('.modal-footer button').contains('OK').click(); cy.get('.ant-modal-confirm-btns button').contains('OK').click();
// select new metric // select new metric
cy.get('[data-test=metrics]') cy.get('[data-test=metrics]')
.find('.Select__control input') .find('.Select__control input')
@ -79,7 +79,7 @@ describe('Datasource control', () => {
.find('.fa-trash') .find('.fa-trash')
.click(); .click();
cy.get('[data-test="datasource-modal-save"]').click(); cy.get('[data-test="datasource-modal-save"]').click();
cy.get('.modal-footer button').contains('OK').click(); cy.get('.ant-modal-confirm-btns button').contains('OK').click();
cy.get('.Select__multi-value__label') cy.get('.Select__multi-value__label')
.contains(newMetricName) .contains(newMetricName)
.should('not.exist'); .should('not.exist');

File diff suppressed because it is too large Load Diff

View File

@ -94,7 +94,7 @@
"@superset-ui/preset-chart-xy": "^0.15.10", "@superset-ui/preset-chart-xy": "^0.15.10",
"@vx/responsive": "^0.0.195", "@vx/responsive": "^0.0.195",
"abortcontroller-polyfill": "^1.1.9", "abortcontroller-polyfill": "^1.1.9",
"antd": "^4.5.2", "antd": "^4.6.6",
"array-move": "^2.2.1", "array-move": "^2.2.1",
"bootstrap": "^3.4.1", "bootstrap": "^3.4.1",
"bootstrap-slider": "^10.0.0", "bootstrap-slider": "^10.0.0",
@ -128,7 +128,6 @@
"react": "^16.13.1", "react": "^16.13.1",
"react-ace": "^5.10.0", "react-ace": "^5.10.0",
"react-bootstrap": "^0.33.1", "react-bootstrap": "^0.33.1",
"react-bootstrap-dialog": "^0.13.0",
"react-bootstrap-slider": "2.1.5", "react-bootstrap-slider": "2.1.5",
"react-checkbox-tree": "^1.5.1", "react-checkbox-tree": "^1.5.1",
"react-color": "^2.13.8", "react-color": "^2.13.8",

View File

@ -26,6 +26,7 @@ import {
ThemeProvider, ThemeProvider,
} from '@superset-ui/core'; } from '@superset-ui/core';
import Modal from 'src/common/components/Modal';
import PropertiesModal from 'src/dashboard/components/PropertiesModal'; import PropertiesModal from 'src/dashboard/components/PropertiesModal';
import { mockStore } from '../fixtures/mockStore'; import { mockStore } from '../fixtures/mockStore';
@ -118,7 +119,7 @@ describe('PropertiesModal', () => {
const wrapper = setup(); const wrapper = setup();
const modalInstance = wrapper.find('PropertiesModal').instance(); const modalInstance = wrapper.find('PropertiesModal').instance();
it('will raise an error', () => { it('will raise an error', () => {
const spy = jest.spyOn(modalInstance.dialog, 'show'); const spy = jest.spyOn(Modal, 'error');
expect(() => expect(() =>
modalInstance.onColorSchemeChange('THIS_WILL_NOT_WORK'), modalInstance.onColorSchemeChange('THIS_WILL_NOT_WORK'),
).toThrowError('A valid color scheme is required'); ).toThrowError('A valid color scheme is required');

View File

@ -97,7 +97,9 @@ describe('DatasourceModal', () => {
}); });
await waitForComponentToPaint(wrapper); await waitForComponentToPaint(wrapper);
act(() => { act(() => {
const okButton = wrapper.find('[className="btn btn-sm btn-primary"]'); const okButton = wrapper.find(
'.ant-modal-confirm .ant-modal-confirm-btns .ant-btn-primary',
);
okButton.simulate('click'); okButton.simulate('click');
}); });
await waitForComponentToPaint(wrapper); await waitForComponentToPaint(wrapper);

View File

@ -20,7 +20,6 @@ import React from 'react';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import { bindActionCreators } from 'redux'; import { bindActionCreators } from 'redux';
import { connect } from 'react-redux'; import { connect } from 'react-redux';
import Dialog from 'react-bootstrap-dialog';
import { t } from '@superset-ui/core'; import { t } from '@superset-ui/core';
import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
@ -99,11 +98,6 @@ class ExploreCtasResultsButton extends React.PureComponent {
/>{' '} />{' '}
{t('Explore')} {t('Explore')}
</Button> </Button>
<Dialog
ref={el => {
this.dialog = el;
}}
/>
</> </>
); );
} }

View File

@ -22,11 +22,11 @@ import PropTypes from 'prop-types';
import { bindActionCreators } from 'redux'; import { bindActionCreators } from 'redux';
import { connect } from 'react-redux'; import { connect } from 'react-redux';
import { Alert } from 'react-bootstrap'; import { Alert } from 'react-bootstrap';
import Dialog from 'react-bootstrap-dialog';
import { t } from '@superset-ui/core'; import { t } from '@superset-ui/core';
import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
import shortid from 'shortid'; import shortid from 'shortid';
import Modal from 'src/common/components/Modal';
import Button from 'src/components/Button'; import Button from 'src/components/Button';
import { exploreChart } from '../../explore/exploreUtils'; import { exploreChart } from '../../explore/exploreUtils';
import * as actions from '../actions/sqlLab'; import * as actions from '../actions/sqlLab';
@ -57,30 +57,16 @@ class ExploreResultsButton extends React.PureComponent {
const { timeout } = this.props; const { timeout } = this.props;
const msg = this.renderInvalidColumnMessage(); const msg = this.renderInvalidColumnMessage();
if (Math.round(this.getQueryDuration()) > timeout) { if (Math.round(this.getQueryDuration()) > timeout) {
this.dialog.show({ Modal.confirm({
title: t('Explore'), title: t('Explore'),
body: this.renderTimeoutWarning(), content: this.renderTimeoutWarning(),
actions: [ onOk: this.visualize,
Dialog.CancelAction(), icon: null,
Dialog.OKAction(() => {
this.visualize();
}),
],
bsSize: 'large',
onHide: dialog => {
dialog.hide();
},
}); });
} else if (msg) { } else if (msg) {
this.dialog.show({ Modal.warning({
title: t('Explore'), title: t('Explore'),
body: msg, content: msg,
actions: [Dialog.DefaultAction('Ok', () => {})],
bsSize: 'large',
bsStyle: 'warning',
onHide: dialog => {
dialog.hide();
},
}); });
} else { } else {
this.visualize(); this.visualize();
@ -228,11 +214,6 @@ class ExploreResultsButton extends React.PureComponent {
/>{' '} />{' '}
{t('Explore')} {t('Explore')}
</Button> </Button>
<Dialog
ref={el => {
this.dialog = el;
}}
/>
</> </>
); );
} }

View File

@ -106,7 +106,7 @@ const StyledModal = styled(BaseModal)<StyledModalProps>`
} }
`; `;
export default function Modal({ const CustomModal = ({
children, children,
disablePrimaryButton = false, disablePrimaryButton = false,
onHide, onHide,
@ -123,7 +123,7 @@ export default function Modal({
hideFooter, hideFooter,
wrapProps, wrapProps,
...rest ...rest
}: ModalProps) { }: ModalProps) => {
const modalFooter = isNil(footer) const modalFooter = isNil(footer)
? [ ? [
<Button key="back" onClick={onHide} cta data-test="modal-cancel-button"> <Button key="back" onClick={onHide} cta data-test="modal-cancel-button">
@ -165,4 +165,14 @@ export default function Modal({
{children} {children}
</StyledModal> </StyledModal>
); );
} };
CustomModal.displayName = 'Modal';
const Modal = Object.assign(CustomModal, {
error: BaseModal.error,
warning: BaseModal.warning,
confirm: BaseModal.confirm,
useModal: BaseModal.useModal,
});
export default Modal;

View File

@ -21,7 +21,6 @@ import PropTypes from 'prop-types';
import { Row, Col, FormControl } from 'react-bootstrap'; import { Row, Col, FormControl } from 'react-bootstrap';
import jsonStringify from 'json-stringify-pretty-compact'; import jsonStringify from 'json-stringify-pretty-compact';
import Button from 'src/components/Button'; import Button from 'src/components/Button';
import Dialog from 'react-bootstrap-dialog';
import { AsyncSelect } from 'src/components/Select'; import { AsyncSelect } from 'src/components/Select';
import rison from 'rison'; import rison from 'rison';
import { import {
@ -65,6 +64,45 @@ const defaultProps = {
onlyApply: false, onlyApply: false,
}; };
const handleErrorResponse = async response => {
const { error, statusText, message } = await getClientErrorObject(response);
let errorText = error || statusText || t('An error has occurred');
if (typeof message === 'object' && message.json_metadata) {
errorText = message.json_metadata;
} else if (typeof message === 'string') {
errorText = message;
if (message === 'Forbidden') {
errorText = t('You do not have permission to edit this dashboard');
}
}
Modal.error({
title: 'Error',
content: errorText,
okButtonProps: { danger: true, className: 'btn-danger' },
});
};
const loadOwnerOptions = (input = '') => {
const query = rison.encode({ filter: input });
return SupersetClient.get({
endpoint: `/api/v1/dashboard/related/owners?q=${query}`,
}).then(
response => {
return response.json.result.map(item => ({
value: item.value,
label: item.text,
}));
},
badResponse => {
handleErrorResponse(badResponse);
return [];
},
);
};
class PropertiesModal extends React.PureComponent { class PropertiesModal extends React.PureComponent {
constructor(props) { constructor(props) {
super(props); super(props);
@ -85,8 +123,6 @@ class PropertiesModal extends React.PureComponent {
this.onOwnersChange = this.onOwnersChange.bind(this); this.onOwnersChange = this.onOwnersChange.bind(this);
this.submit = this.submit.bind(this); this.submit = this.submit.bind(this);
this.toggleAdvanced = this.toggleAdvanced.bind(this); this.toggleAdvanced = this.toggleAdvanced.bind(this);
this.loadOwnerOptions = this.loadOwnerOptions.bind(this);
this.handleErrorResponse = this.handleErrorResponse.bind(this);
this.onColorSchemeChange = this.onColorSchemeChange.bind(this); this.onColorSchemeChange = this.onColorSchemeChange.bind(this);
} }
@ -104,12 +140,10 @@ class PropertiesModal extends React.PureComponent {
: {}; : {};
if (!colorChoices.includes(value)) { if (!colorChoices.includes(value)) {
this.dialog.show({ Modal.error({
title: 'Error', title: 'Error',
bsSize: 'medium', content: t('A valid color scheme is required'),
bsStyle: 'danger', okButtonProps: { danger: true, className: 'btn-danger' },
actions: [Dialog.DefaultAction('Ok', () => {}, 'btn-danger')],
body: t('A valid color scheme is required'),
}); });
throw new Error('A valid color scheme is required'); throw new Error('A valid color scheme is required');
} }
@ -170,25 +204,7 @@ class PropertiesModal extends React.PureComponent {
label: `${owner.first_name} ${owner.last_name}`, label: `${owner.first_name} ${owner.last_name}`,
})); }));
this.onOwnersChange(initialSelectedOwners); this.onOwnersChange(initialSelectedOwners);
}, this.handleErrorResponse); }, handleErrorResponse);
}
loadOwnerOptions(input = '') {
const query = rison.encode({ filter: input });
return SupersetClient.get({
endpoint: `/api/v1/dashboard/related/owners?q=${query}`,
}).then(
response => {
return response.json.result.map(item => ({
value: item.value,
label: item.text,
}));
},
badResponse => {
this.handleErrorResponse(badResponse);
return [];
},
);
} }
updateFormState(name, value) { updateFormState(name, value) {
@ -206,29 +222,6 @@ class PropertiesModal extends React.PureComponent {
})); }));
} }
async handleErrorResponse(response) {
const { error, statusText, message } = await getClientErrorObject(response);
let errorText = error || statusText || t('An error has occurred');
if (typeof message === 'object' && message.json_metadata) {
errorText = message.json_metadata;
} else if (typeof message === 'string') {
errorText = message;
if (message === 'Forbidden') {
errorText = t('You do not have permission to edit this dashboard');
}
}
this.dialog.show({
title: 'Error',
bsSize: 'medium',
bsStyle: 'danger',
actions: [Dialog.DefaultAction('Ok', () => {}, 'btn-danger')],
body: errorText,
});
}
submit(e) { submit(e) {
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
@ -286,7 +279,7 @@ class PropertiesModal extends React.PureComponent {
colorScheme: metadataColorScheme || colorScheme, colorScheme: metadataColorScheme || colorScheme,
}); });
this.props.onHide(); this.props.onHide();
}, this.handleErrorResponse); }, handleErrorResponse);
} }
} }
@ -322,11 +315,6 @@ class PropertiesModal extends React.PureComponent {
> >
{saveLabel} {saveLabel}
</Button> </Button>
<Dialog
ref={ref => {
this.dialog = ref;
}}
/>
</> </>
} }
responsive responsive
@ -373,7 +361,7 @@ class PropertiesModal extends React.PureComponent {
name="owners" name="owners"
isMulti isMulti
value={values.owners} value={values.owners}
loadOptions={this.loadOwnerOptions} loadOptions={loadOwnerOptions}
defaultOptions // load options on render defaultOptions // load options on render
cacheOptions cacheOptions
onChange={this.onOwnersChange} onChange={this.onOwnersChange}

View File

@ -19,7 +19,6 @@
import React, { FunctionComponent, useState, useRef } from 'react'; import React, { FunctionComponent, useState, useRef } from 'react';
import { Alert } from 'react-bootstrap'; import { Alert } from 'react-bootstrap';
import Button from 'src/components/Button'; import Button from 'src/components/Button';
import Dialog from 'react-bootstrap-dialog';
import { styled, t, SupersetClient } from '@superset-ui/core'; import { styled, t, SupersetClient } from '@superset-ui/core';
import Modal from 'src/common/components/Modal'; import Modal from 'src/common/components/Modal';
@ -84,6 +83,7 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
const [errors, setErrors] = useState<any[]>([]); const [errors, setErrors] = useState<any[]>([]);
const [isSaving, setIsSaving] = useState(false); const [isSaving, setIsSaving] = useState(false);
const dialog = useRef<any>(null); const dialog = useRef<any>(null);
const [modal, contextHolder] = Modal.useModal();
const onConfirmSave = () => { const onConfirmSave = () => {
// Pull out extra fields into the extra object // Pull out extra fields into the extra object
@ -118,12 +118,10 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
.catch(response => { .catch(response => {
setIsSaving(false); setIsSaving(false);
getClientErrorObject(response).then(({ error }) => { getClientErrorObject(response).then(({ error }) => {
dialog.current.show({ modal.error({
title: 'Error', title: 'Error',
bsSize: 'medium', content: error || t('An error has occurred'),
bsStyle: 'danger', okButtonProps: { danger: true, className: 'btn-danger' },
actions: [Dialog.DefaultAction('Ok', () => {}, 'btn-danger')],
body: error || t('An error has occurred'),
}); });
}); });
}); });
@ -140,13 +138,13 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
setErrors(err); setErrors(err);
}; };
const closeDialog = () => {
dialog.current?.destroy();
};
const renderSaveDialog = () => ( const renderSaveDialog = () => (
<div> <div>
<Alert <Alert bsStyle="warning" className="pointer" onClick={closeDialog}>
bsStyle="warning"
className="pointer"
onClick={dialog.current.hideAlert}
>
<div> <div>
<i className="fa fa-exclamation-triangle" />{' '} <i className="fa fa-exclamation-triangle" />{' '}
{t(`The dataset configuration exposed here {t(`The dataset configuration exposed here
@ -161,11 +159,11 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
); );
const onClickSave = () => { const onClickSave = () => {
dialog.current.show({ dialog.current = modal.confirm({
title: t('Confirm save'), title: t('Confirm save'),
bsSize: 'medium', content: renderSaveDialog(),
actions: [Dialog.CancelAction(), Dialog.OKAction(onConfirmSave)], onOk: onConfirmSave,
body: renderSaveDialog(), icon: null,
}); });
}; };
@ -212,7 +210,6 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
> >
{t('Cancel')} {t('Cancel')}
</Button> </Button>
<Dialog ref={dialog} />
</> </>
} }
responsive responsive
@ -223,6 +220,7 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
datasource={currentDatasource} datasource={currentDatasource}
onChange={onDatasourceChange} onChange={onDatasourceChange}
/> />
{contextHolder}
</StyledDatasourceModal> </StyledDatasourceModal>
); );
}; };

View File

@ -16,7 +16,7 @@
* specific language governing permissions and limitations * specific language governing permissions and limitations
* under the License. * under the License.
*/ */
import React, { useState, useEffect, useRef, useCallback } from 'react'; import React, { useState, useEffect, useCallback } from 'react';
import { import {
Row, Row,
Col, Col,
@ -26,7 +26,6 @@ import {
} from 'react-bootstrap'; } from 'react-bootstrap';
import Modal from 'src/common/components/Modal'; import Modal from 'src/common/components/Modal';
import Button from 'src/components/Button'; import Button from 'src/components/Button';
import Dialog from 'react-bootstrap-dialog';
import { OptionsType } from 'react-select/src/types'; import { OptionsType } from 'react-select/src/types';
import { AsyncSelect } from 'src/components/Select'; import { AsyncSelect } from 'src/components/Select';
import rison from 'rison'; import rison from 'rison';
@ -54,7 +53,6 @@ export default function PropertiesModal({
show, show,
}: PropertiesModalProps) { }: PropertiesModalProps) {
const [submitting, setSubmitting] = useState(false); const [submitting, setSubmitting] = useState(false);
const errorDialog = useRef<any>(null);
// values of form inputs // values of form inputs
const [name, setName] = useState(slice.slice_name || ''); const [name, setName] = useState(slice.slice_name || '');
@ -69,12 +67,10 @@ export default function PropertiesModal({
if (message === 'Forbidden') { if (message === 'Forbidden') {
errorText = t('You do not have permission to edit this chart'); errorText = t('You do not have permission to edit this chart');
} }
errorDialog.current.show({ Modal.error({
title: 'Error', title: 'Error',
bsSize: 'medium', content: errorText,
bsStyle: 'danger', okButtonProps: { danger: true, className: 'btn-danger' },
actions: [Dialog.DefaultAction('Ok', () => {}, 'btn-danger')],
body: errorText,
}); });
} }
@ -185,7 +181,6 @@ export default function PropertiesModal({
> >
{t('Save')} {t('Save')}
</Button> </Button>
<Dialog ref={errorDialog} />
</> </>
} }
responsive responsive