Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dashboard] Add alert when user deleting root level tab #5771

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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);
});
});

Expand Down
11 changes: 7 additions & 4 deletions superset/assets/src/components/ModalTrigger.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -28,6 +28,7 @@ const defaultProps = {
isMenuItem: false,
bsSize: null,
className: '',
modalTitle: '',
};

export default class ModalTrigger extends React.Component {
Expand Down Expand Up @@ -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>
Expand Down
62 changes: 62 additions & 0 deletions superset/assets/src/dashboard/components/DeleteComponentModal.jsx
Original file line number Diff line number Diff line change
@@ -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;
10 changes: 8 additions & 2 deletions superset/assets/src/dashboard/components/gridComponents/Tab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand All @@ -201,7 +206,8 @@ export default class Tab extends React.PureComponent {
parentComponent.children.length <= 1
? []
: [
<DeleteComponentButton
<DeleteComponentModal
triggerNode={deleteTabIcon}
onDelete={this.handleDeleteComponent}
/>,
]
Expand Down
23 changes: 12 additions & 11 deletions superset/assets/src/dashboard/components/menu/WithPopoverMenu.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah still can't remember why I added these 🤷‍♀️

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) {
Expand All @@ -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);
Expand Down
36 changes: 32 additions & 4 deletions superset/assets/src/dashboard/stylesheets/dashboard.less
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down