From 0c98ecb6d15aaf013731f0a273ead977d9d5a903 Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Thu, 6 Sep 2018 10:23:56 -0700 Subject: [PATCH] [dashboard] Add alert on user delete root level tab (#5771) --- .../components/gridComponents/Tab_spec.jsx | 18 +++--- .../assets/src/components/ModalTrigger.jsx | 11 ++-- .../components/DeleteComponentModal.jsx | 62 +++++++++++++++++++ .../components/gridComponents/Tab.jsx | 10 ++- .../components/menu/WithPopoverMenu.jsx | 23 +++---- .../src/dashboard/stylesheets/dashboard.less | 36 +++++++++-- 6 files changed, 131 insertions(+), 29 deletions(-) create mode 100644 superset/assets/src/dashboard/components/DeleteComponentModal.jsx 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} > - - {this.props.modalTitle} - + {this.props.modalTitle && + + {this.props.modalTitle} + + } {this.props.modalBody} 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 ( + +

{t('Delete dashboard tab?')}

+
+ Deleting a tab will remove all content within it. You may still + reverse this action with the undo button (cmd + z) until + you save your changes. +
+
+ + +
+ + } + /> + ); + } +} + +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 = ( +
+ +
+ ); return ( , ] 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 {