Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

[cherry-picks] a few dashboard improvement PRs #109

Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class BuilderComponentPane extends React.PureComponent {
>
<div className="component-layer slide-content">
<div className="dashboard-builder-sidepane-header">
<span>{t('Insert')}</span>
<span>{t('Insert components')}</span>
<i
className="fa fa-times trigger"
onClick={this.props.toggleBuilderPane}
Expand Down Expand Up @@ -101,7 +101,7 @@ class BuilderComponentPane extends React.PureComponent {
role="none"
>
<i className="fa fa-arrow-left trigger" />
<span>{t('All components')}</span>
<span>{t('Your charts and filters')}</span>
</div>
<SliceAdder
height={
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);
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
11 changes: 11 additions & 0 deletions superset/assets/src/dashboard/stylesheets/components/markdown.less
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
.dashboard-markdown {
overflow: hidden;

h4, h5 {
font-weight: 300;
}
h5 {
color: @gray-heading;
}
h6 {
font-weight: 400;
font-size: 12px;
}

.dashboard-component-chart-holder {
overflow-y: auto;
overflow-x: hidden;
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
1 change: 1 addition & 0 deletions superset/assets/src/dashboard/stylesheets/variables.less
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
@gray: #879399;
@gray-light: #CFD8DC;
@gray-bg: #f5f5f5;
@gray-heading: #A3A3A3;
@menu-hover: #F2F3F5;

/* builder component pane */
Expand Down