You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2018/09/06 17:23:58 UTC
[GitHub] graceguo-supercat closed pull request #5771: [dashboard] Add alert
when user deleting root level tab
graceguo-supercat closed pull request #5771: [dashboard] Add alert when user deleting root level tab
URL: https://github.com/apache/incubator-superset/pull/5771
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/superset/assets/spec/javascripts/dashboard/components/gridComponents/Tab_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/gridComponents/Tab_spec.jsx
index a984565b4c..fae59b2590 100644
--- a/superset/assets/spec/javascripts/dashboard/components/gridComponents/Tab_spec.jsx
+++ b/superset/assets/spec/javascripts/dashboard/components/gridComponents/Tab_spec.jsx
@@ -6,7 +6,7 @@ import { expect } from 'chai';
import sinon from 'sinon';
import DashboardComponent from '../../../../../src/dashboard/containers/DashboardComponent';
-import DeleteComponentButton from '../../../../../src/dashboard/components/DeleteComponentButton';
+import DeleteComponentModal from '../../../../../src/dashboard/components/DeleteComponentModal';
import DragDroppable from '../../../../../src/dashboard/components/dnd/DragDroppable';
import EditableTitle from '../../../../../src/components/EditableTitle';
import WithPopoverMenu from '../../../../../src/dashboard/components/menu/WithPopoverMenu';
@@ -86,14 +86,14 @@ describe('Tabs', () => {
expect(wrapper.find(WithPopoverMenu)).to.have.length(1);
});
- it('should render a DeleteComponentButton when focused if its not the only tab', () => {
+ it('should render a DeleteComponentModal when focused if its not the only tab', () => {
let wrapper = setup();
wrapper.find(WithPopoverMenu).simulate('click'); // focus
- expect(wrapper.find(DeleteComponentButton)).to.have.length(0);
+ expect(wrapper.find(DeleteComponentModal)).to.have.length(0);
wrapper = setup({ editMode: true });
wrapper.find(WithPopoverMenu).simulate('click');
- expect(wrapper.find(DeleteComponentButton)).to.have.length(1);
+ expect(wrapper.find(DeleteComponentModal)).to.have.length(1);
wrapper = setup({
editMode: true,
@@ -103,16 +103,18 @@ describe('Tabs', () => {
},
});
wrapper.find(WithPopoverMenu).simulate('click');
- expect(wrapper.find(DeleteComponentButton)).to.have.length(0);
+ expect(wrapper.find(DeleteComponentModal)).to.have.length(0);
});
- it('should call deleteComponent when deleted', () => {
+ it('should show modal when clicked delete icon', () => {
const deleteComponent = sinon.spy();
const wrapper = setup({ editMode: true, deleteComponent });
wrapper.find(WithPopoverMenu).simulate('click'); // focus
- wrapper.find(DeleteComponentButton).simulate('click');
+ wrapper.find('.icon-button').simulate('click');
- expect(deleteComponent.callCount).to.equal(1);
+ const modal = document.getElementsByClassName('modal');
+ expect(modal).to.have.length(1);
+ expect(deleteComponent.callCount).to.equal(0);
});
});
diff --git a/superset/assets/src/components/ModalTrigger.jsx b/superset/assets/src/components/ModalTrigger.jsx
index 67a83e6c21..83e8db361f 100644
--- a/superset/assets/src/components/ModalTrigger.jsx
+++ b/superset/assets/src/components/ModalTrigger.jsx
@@ -8,7 +8,7 @@ import Button from './Button';
const propTypes = {
animation: PropTypes.bool,
triggerNode: PropTypes.node.isRequired,
- modalTitle: PropTypes.node.isRequired,
+ modalTitle: PropTypes.node,
modalBody: PropTypes.node, // not required because it can be generated by beforeOpen
modalFooter: PropTypes.node,
beforeOpen: PropTypes.func,
@@ -28,6 +28,7 @@ const defaultProps = {
isMenuItem: false,
bsSize: null,
className: '',
+ modalTitle: '',
};
export default class ModalTrigger extends React.Component {
@@ -59,9 +60,11 @@ export default class ModalTrigger extends React.Component {
bsSize={this.props.bsSize}
className={this.props.className}
>
- <Modal.Header closeButton>
- <Modal.Title>{this.props.modalTitle}</Modal.Title>
- </Modal.Header>
+ {this.props.modalTitle &&
+ <Modal.Header closeButton>
+ <Modal.Title>{this.props.modalTitle}</Modal.Title>
+ </Modal.Header>
+ }
<Modal.Body>
{this.props.modalBody}
</Modal.Body>
diff --git a/superset/assets/src/dashboard/components/DeleteComponentModal.jsx b/superset/assets/src/dashboard/components/DeleteComponentModal.jsx
new file mode 100644
index 0000000000..ea7721c037
--- /dev/null
+++ b/superset/assets/src/dashboard/components/DeleteComponentModal.jsx
@@ -0,0 +1,62 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { Button } from 'react-bootstrap';
+
+import ModalTrigger from '../../components/ModalTrigger';
+import { t } from '../../locales';
+
+const propTypes = {
+ triggerNode: PropTypes.node.isRequired,
+ onDelete: PropTypes.func.isRequired,
+};
+
+export default class DeleteComponentModal extends React.PureComponent {
+ constructor(props) {
+ super(props);
+
+ this.modal = null;
+ this.close = this.close.bind(this);
+ this.deleteTab = this.deleteTab.bind(this);
+ this.setModalRef = this.setModalRef.bind(this);
+ }
+
+ setModalRef(ref) {
+ this.modal = ref;
+ }
+
+ close() {
+ this.modal.close();
+ }
+
+ deleteTab() {
+ this.modal.close();
+ this.props.onDelete();
+ }
+
+ render() {
+ return (
+ <ModalTrigger
+ ref={this.setModalRef}
+ triggerNode={this.props.triggerNode}
+ modalBody={
+ <div className="delete-component-modal">
+ <h1>{t('Delete dashboard tab?')}</h1>
+ <div>
+ Deleting a tab will remove all content within it. You may still
+ reverse this action with the <b>undo</b> button (cmd + z) until
+ you save your changes.
+ </div>
+ <div className="delete-modal-actions-container">
+ <Button onClick={this.close}>{t('Cancel')}</Button>
+ <Button bsStyle="primary" onClick={this.deleteTab}>
+ {t('Delete')}
+ </Button>
+ </div>
+ </div>
+ }
+ />
+ );
+ }
+}
+
+DeleteComponentModal.propTypes = propTypes;
diff --git a/superset/assets/src/dashboard/components/gridComponents/Tab.jsx b/superset/assets/src/dashboard/components/gridComponents/Tab.jsx
index b91d8089fd..dad3be4c72 100644
--- a/superset/assets/src/dashboard/components/gridComponents/Tab.jsx
+++ b/superset/assets/src/dashboard/components/gridComponents/Tab.jsx
@@ -4,7 +4,7 @@ import PropTypes from 'prop-types';
import DashboardComponent from '../../containers/DashboardComponent';
import DragDroppable from '../dnd/DragDroppable';
import EditableTitle from '../../../components/EditableTitle';
-import DeleteComponentButton from '../DeleteComponentButton';
+import DeleteComponentModal from '../DeleteComponentModal';
import WithPopoverMenu from '../menu/WithPopoverMenu';
import { componentShape } from '../../util/propShapes';
import { DASHBOARD_ROOT_DEPTH } from '../../util/constants';
@@ -178,6 +178,11 @@ export default class Tab extends React.PureComponent {
renderTab() {
const { isFocused } = this.state;
const { component, parentComponent, index, depth, editMode } = this.props;
+ const deleteTabIcon = (
+ <div className="icon-button">
+ <span className="fa fa-trash" />
+ </div>
+ );
return (
<DragDroppable
@@ -201,7 +206,8 @@ export default class Tab extends React.PureComponent {
parentComponent.children.length <= 1
? []
: [
- <DeleteComponentButton
+ <DeleteComponentModal
+ triggerNode={deleteTabIcon}
onDelete={this.handleDeleteComponent}
/>,
]
diff --git a/superset/assets/src/dashboard/components/menu/WithPopoverMenu.jsx b/superset/assets/src/dashboard/components/menu/WithPopoverMenu.jsx
index 2a047ac573..f98b17e650 100644
--- a/superset/assets/src/dashboard/components/menu/WithPopoverMenu.jsx
+++ b/superset/assets/src/dashboard/components/menu/WithPopoverMenu.jsx
@@ -20,7 +20,8 @@ const defaultProps = {
onPressDelete() {},
menuItems: [],
isFocused: false,
- shouldFocus: (event, container) => container.contains(event.target),
+ shouldFocus: (event, container) =>
+ container && container.contains(event.target),
style: null,
};
@@ -36,19 +37,19 @@ class WithPopoverMenu extends React.PureComponent {
componentWillReceiveProps(nextProps) {
if (nextProps.editMode && nextProps.isFocused && !this.state.isFocused) {
- document.addEventListener('click', this.handleClick, true);
- document.addEventListener('drag', this.handleClick, true);
+ document.addEventListener('click', this.handleClick);
+ document.addEventListener('drag', this.handleClick);
this.setState({ isFocused: true });
} else if (this.state.isFocused && !nextProps.editMode) {
- document.removeEventListener('click', this.handleClick, true);
- document.removeEventListener('drag', this.handleClick, true);
+ document.removeEventListener('click', this.handleClick);
+ document.removeEventListener('drag', this.handleClick);
this.setState({ isFocused: false });
}
}
componentWillUnmount() {
- document.removeEventListener('click', this.handleClick, true);
- document.removeEventListener('drag', this.handleClick, true);
+ document.removeEventListener('click', this.handleClick);
+ document.removeEventListener('drag', this.handleClick);
}
setRef(ref) {
@@ -69,15 +70,15 @@ class WithPopoverMenu extends React.PureComponent {
if (!disableClick && shouldFocus && !this.state.isFocused) {
// if not focused, set focus and add a window event listener to capture outside clicks
// this enables us to not set a click listener for ever item on a dashboard
- document.addEventListener('click', this.handleClick, true);
- document.addEventListener('drag', this.handleClick, true);
+ document.addEventListener('click', this.handleClick);
+ document.addEventListener('drag', this.handleClick);
this.setState(() => ({ isFocused: true }));
if (onChangeFocus) {
onChangeFocus(true);
}
} else if (!shouldFocus && this.state.isFocused) {
- document.removeEventListener('click', this.handleClick, true);
- document.removeEventListener('drag', this.handleClick, true);
+ document.removeEventListener('click', this.handleClick);
+ document.removeEventListener('drag', this.handleClick);
this.setState(() => ({ isFocused: false }));
if (onChangeFocus) {
onChangeFocus(false);
diff --git a/superset/assets/src/dashboard/stylesheets/dashboard.less b/superset/assets/src/dashboard/stylesheets/dashboard.less
index 56f5d43774..2b5242e87a 100644
--- a/superset/assets/src/dashboard/stylesheets/dashboard.less
+++ b/superset/assets/src/dashboard/stylesheets/dashboard.less
@@ -129,10 +129,38 @@ body {
}
}
-.modal img.loading {
- width: 50px;
- margin: 0;
- position: relative;
+.modal {
+ img.loading {
+ width: 50px;
+ margin: 0;
+ position: relative;
+ }
+
+ .modal-body {
+ padding: 24px 24px 29px 24px;
+
+ div {
+ margin-top: 24px;
+
+ &:first-child {
+ margin-top: 0;
+ }
+ }
+ }
+
+ .delete-modal-actions-container {
+ .btn {
+ margin-right: 16px;
+ &:last-child {
+ margin-right: 0;
+ }
+
+ &.btn-primary {
+ background: @pink !important;
+ border-color: @pink !important;
+ }
+ }
+ }
}
.react-bs-container-body {
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org