You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by gr...@apache.org on 2018/09/06 17:24:00 UTC
[incubator-superset] branch master updated: [dashboard] Add alert
on user delete root level tab (#5771)
This is an automated email from the ASF dual-hosted git repository.
graceguo 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 0c98ecb [dashboard] Add alert on user delete root level tab (#5771)
0c98ecb is described below
commit 0c98ecb6d15aaf013731f0a273ead977d9d5a903
Author: Grace Guo <gr...@airbnb.com>
AuthorDate: Thu Sep 6 10:23:56 2018 -0700
[dashboard] Add alert on user delete root level tab (#5771)
---
.../components/gridComponents/Tab_spec.jsx | 18 ++++---
superset/assets/src/components/ModalTrigger.jsx | 11 ++--
.../dashboard/components/DeleteComponentModal.jsx | 62 ++++++++++++++++++++++
.../dashboard/components/gridComponents/Tab.jsx | 10 +++-
.../dashboard/components/menu/WithPopoverMenu.jsx | 23 ++++----
.../src/dashboard/stylesheets/dashboard.less | 36 +++++++++++--
6 files changed, 131 insertions(+), 29 deletions(-)
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 a984565..fae59b2 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 67a83e6..83e8db3 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 0000000..ea7721c
--- /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 b91d808..dad3be4 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 2a047ac..f98b17e 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 56f5d43..2b5242e 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 {