superset-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From grace...@apache.org
Subject [incubator-superset] branch master updated: [dashboard] Add alert on user delete root level tab (#5771)
Date Thu, 06 Sep 2018 17:24:00 GMT
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 <grace.guo@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 {


Mime
View raw message