From f3ab1f41eebcd8dfe0379e81dd7083f1a9efa52f Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 25 Dec 2020 05:46:37 +0100 Subject: [PATCH] feat: Implement drag & drop for metrics and filters labels (#12184) --- .../components/AdhocFilterControl_spec.jsx | 2 +- .../components/AdhocFilterOption_spec.jsx | 2 +- .../components/AdhocMetricOption_spec.jsx | 5 +- .../components/MetricsControl_spec.jsx | 2 +- .../components/withAsyncVerification_spec.tsx | 4 +- .../src/explore/DndContextProvider.js | 23 ++++ .../explore/components/AdhocFilterOption.jsx | 15 ++- .../explore/components/AdhocMetricOption.jsx | 15 ++- .../components/MetricDefinitionValue.jsx | 23 +++- .../src/explore/components/OptionControls.tsx | 122 +++++++++++++++++- .../controls/AdhocFilterControl.jsx | 19 ++- .../components/controls/MetricsControl.jsx | 19 ++- .../src/explore/components/optionTypes.ts | 22 ++++ 13 files changed, 251 insertions(+), 22 deletions(-) create mode 100644 superset-frontend/src/explore/DndContextProvider.js create mode 100644 superset-frontend/src/explore/components/optionTypes.ts diff --git a/superset-frontend/spec/javascripts/explore/components/AdhocFilterControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/AdhocFilterControl_spec.jsx index 462b651b13f31..94af81ca55517 100644 --- a/superset-frontend/spec/javascripts/explore/components/AdhocFilterControl_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/AdhocFilterControl_spec.jsx @@ -71,7 +71,7 @@ function setup(overrides) { ...overrides, }; const wrapper = shallow(); - const component = wrapper.dive().shallow(); + const component = wrapper.dive().dive().shallow(); return { wrapper, component, onChange }; } diff --git a/superset-frontend/spec/javascripts/explore/components/AdhocFilterOption_spec.jsx b/superset-frontend/spec/javascripts/explore/components/AdhocFilterOption_spec.jsx index 56f643678a986..781f22192c066 100644 --- a/superset-frontend/spec/javascripts/explore/components/AdhocFilterOption_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/AdhocFilterOption_spec.jsx @@ -56,6 +56,6 @@ describe('AdhocFilterOption', () => { const popover = overlay.find(Popover); expect(popover).toHaveLength(1); expect(popover.props().defaultVisible).toBe(false); - expect(overlay.find('OptionControlLabel')).toExist(); + expect(overlay.find('DraggableOptionControlLabel')).toExist(); }); }); diff --git a/superset-frontend/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx b/superset-frontend/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx index af2fa1f22db87..ceef77d5aa9c4 100644 --- a/superset-frontend/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx @@ -45,6 +45,9 @@ function setup(overrides) { savedMetrics: [], onMetricEdit, columns, + onMoveLabel: () => {}, + onDropLabel: () => {}, + index: 0, ...overrides, }; const wrapper = shallow() @@ -57,7 +60,7 @@ describe('AdhocMetricOption', () => { it('renders an overlay trigger wrapper for the label', () => { const { wrapper } = setup(); expect(wrapper.find(Popover)).toExist(); - expect(wrapper.find('OptionControlLabel')).toExist(); + expect(wrapper.find('DraggableOptionControlLabel')).toExist(); }); it('overwrites the adhocMetric in state with onLabelChange', () => { diff --git a/superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx index f21427d20d6ef..638a20e02a49c 100644 --- a/superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx @@ -53,7 +53,7 @@ function setup(overrides) { ...overrides, }; const wrapper = shallow(); - const component = wrapper.dive().shallow(); + const component = wrapper.dive().dive().shallow(); return { wrapper, component, onChange }; } diff --git a/superset-frontend/spec/javascripts/explore/components/withAsyncVerification_spec.tsx b/superset-frontend/spec/javascripts/explore/components/withAsyncVerification_spec.tsx index 44a03330caff8..7d0d2598781ea 100644 --- a/superset-frontend/spec/javascripts/explore/components/withAsyncVerification_spec.tsx +++ b/superset-frontend/spec/javascripts/explore/components/withAsyncVerification_spec.tsx @@ -120,7 +120,9 @@ describe('VerifiedMetricsControl', () => { onChange: mockOnChange, }); - const child = wrapper.find(MetricsControl); + const child = wrapper.find(MetricsControl) as ReactWrapper<{ + onChange: (str: string[]) => void; + }>; child.props().onChange(['abc']); expect(child.length).toBe(1); diff --git a/superset-frontend/src/explore/DndContextProvider.js b/superset-frontend/src/explore/DndContextProvider.js new file mode 100644 index 0000000000000..884807ecfbc7e --- /dev/null +++ b/superset-frontend/src/explore/DndContextProvider.js @@ -0,0 +1,23 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +// TODO: convert to .ts after we upgrade react-dnd +import { DragDropContext } from 'react-dnd'; +import HTML5Backend from 'react-dnd-html5-backend'; + +export default DragDropContext(HTML5Backend); diff --git a/superset-frontend/src/explore/components/AdhocFilterOption.jsx b/superset-frontend/src/explore/components/AdhocFilterOption.jsx index 771eebcad6c16..a6b26c5c44f77 100644 --- a/superset-frontend/src/explore/components/AdhocFilterOption.jsx +++ b/superset-frontend/src/explore/components/AdhocFilterOption.jsx @@ -22,7 +22,8 @@ import AdhocFilter from '../AdhocFilter'; import columnType from '../propTypes/columnType'; import adhocMetricType from '../propTypes/adhocMetricType'; import AdhocFilterPopoverTrigger from './AdhocFilterPopoverTrigger'; -import { OptionControlLabel } from './OptionControls'; +import { DraggableOptionControlLabel } from './OptionControls'; +import { OPTION_TYPES } from './optionTypes'; const propTypes = { adhocFilter: PropTypes.instanceOf(AdhocFilter).isRequired, @@ -37,6 +38,9 @@ const propTypes = { ).isRequired, datasource: PropTypes.object, partitionColumn: PropTypes.string, + onMoveLabel: PropTypes.func, + onDropLabel: PropTypes.func, + index: PropTypes.number, }; const AdhocFilterOption = ({ @@ -46,6 +50,9 @@ const AdhocFilterOption = ({ onFilterEdit, onRemoveFilter, partitionColumn, + onMoveLabel, + onDropLabel, + index, }) => ( - diff --git a/superset-frontend/src/explore/components/AdhocMetricOption.jsx b/superset-frontend/src/explore/components/AdhocMetricOption.jsx index 9322a97df3b77..270db912ff3e6 100644 --- a/superset-frontend/src/explore/components/AdhocMetricOption.jsx +++ b/superset-frontend/src/explore/components/AdhocMetricOption.jsx @@ -21,8 +21,9 @@ import PropTypes from 'prop-types'; import AdhocMetric from '../AdhocMetric'; import columnType from '../propTypes/columnType'; import savedMetricType from '../propTypes/savedMetricType'; -import { OptionControlLabel } from './OptionControls'; +import { DraggableOptionControlLabel } from './OptionControls'; import AdhocMetricPopoverTrigger from './AdhocMetricPopoverTrigger'; +import { OPTION_TYPES } from './optionTypes'; const propTypes = { adhocMetric: PropTypes.instanceOf(AdhocMetric), @@ -32,6 +33,9 @@ const propTypes = { savedMetrics: PropTypes.arrayOf(savedMetricType), savedMetric: savedMetricType, datasourceType: PropTypes.string, + onMoveLabel: PropTypes.func, + onDropLabel: PropTypes.func, + index: PropTypes.number, }; class AdhocMetricOption extends React.PureComponent { @@ -53,6 +57,9 @@ class AdhocMetricOption extends React.PureComponent { savedMetrics, savedMetric, datasourceType, + onMoveLabel, + onDropLabel, + index, } = this.props; return ( - diff --git a/superset-frontend/src/explore/components/MetricDefinitionValue.jsx b/superset-frontend/src/explore/components/MetricDefinitionValue.jsx index c2ef35cb4add4..1a65da1227d78 100644 --- a/superset-frontend/src/explore/components/MetricDefinitionValue.jsx +++ b/superset-frontend/src/explore/components/MetricDefinitionValue.jsx @@ -18,18 +18,21 @@ */ import React from 'react'; import PropTypes from 'prop-types'; - import AdhocMetricOption from './AdhocMetricOption'; import AdhocMetric from '../AdhocMetric'; import columnType from '../propTypes/columnType'; import savedMetricType from '../propTypes/savedMetricType'; import adhocMetricType from '../propTypes/adhocMetricType'; -import { OptionControlLabel } from './OptionControls'; +import { DraggableOptionControlLabel } from './OptionControls'; +import { OPTION_TYPES } from './optionTypes'; const propTypes = { option: PropTypes.oneOfType([savedMetricType, adhocMetricType]).isRequired, + index: PropTypes.number.isRequired, onMetricEdit: PropTypes.func, onRemoveMetric: PropTypes.func, + onMoveLabel: PropTypes.func, + onDropLabel: PropTypes.func, columns: PropTypes.arrayOf(columnType), savedMetrics: PropTypes.arrayOf(savedMetricType), multi: PropTypes.bool, @@ -43,6 +46,9 @@ export default function MetricDefinitionValue({ columns, savedMetrics, datasourceType, + onMoveLabel, + onDropLabel, + index, }) { const getSavedMetricByName = metricName => savedMetrics.find(metric => metric.metric_name === metricName); @@ -65,6 +71,9 @@ export default function MetricDefinitionValue({ savedMetrics, datasourceType, adhocMetric, + onMoveLabel, + onDropLabel, + index, savedMetric: savedMetric ?? {}, }; @@ -72,7 +81,15 @@ export default function MetricDefinitionValue({ } if (typeof option === 'string') { return ( - + ); } return null; diff --git a/superset-frontend/src/explore/components/OptionControls.tsx b/superset-frontend/src/explore/components/OptionControls.tsx index a894d3105a275..31de6ae36cbd4 100644 --- a/superset-frontend/src/explore/components/OptionControls.tsx +++ b/superset-frontend/src/explore/components/OptionControls.tsx @@ -17,12 +17,28 @@ * under the License. */ import React from 'react'; +import { findDOMNode } from 'react-dom'; +// Current version of react-dnd (2.5.4) doesn't work well with typescript +// TODO: remove ts-ignore after we upgrade react-dnd +// @ts-ignore +import { DragSource, DropTarget } from 'react-dnd'; import { styled, useTheme } from '@superset-ui/core'; import { ColumnOption } from '@superset-ui/chart-controls'; import Icon from '../../components/Icon'; import { savedMetricType } from '../types'; -const OptionControlContainer = styled.div<{ isAdhoc?: boolean }>` +const TYPE = 'label-dnd'; + +const DragContainer = styled.div` + margin-bottom: ${({ theme }) => theme.gridUnit}px; + :last-child { + margin-bottom: 0; + } +`; + +const OptionControlContainer = styled.div<{ + isAdhoc?: boolean; +}>` display: flex; align-items: center; width: 100%; @@ -31,10 +47,6 @@ const OptionControlContainer = styled.div<{ isAdhoc?: boolean }>` background-color: ${({ theme }) => theme.colors.grayscale.light3}; border-radius: 3px; cursor: ${({ isAdhoc }) => (isAdhoc ? 'pointer' : 'default')}; - margin-bottom: ${({ theme }) => theme.gridUnit}px; - :last-child { - margin-bottom: 0; - } `; const Label = styled.div` @@ -113,12 +125,81 @@ export const AddIconButton = styled.button` } `; +const labelSource = { + beginDrag({ index, type }: { index: number; type: string }) { + return { + index, + type, + }; + }, +}; + +const labelTarget = { + hover(props: Record, monitor: any, component: any) { + const { index: dragIndex, type: dragType } = monitor.getItem(); + const { index: hoverIndex, type: hoverType } = props; + + // Don't replace items with themselves + // Don't allow to drag items between filters and metrics boxes + if (dragIndex === hoverIndex || dragType !== hoverType) { + return; + } + + // Determine rectangle on screen + // TODO: refactor with references when we upgrade react-dnd + // For now we disable warnings about findDOMNode, but we should refactor after we upgrade react-dnd + // Current version (2.5.4) doesn't work well with refs + // @ts-ignore + // eslint-disable-next-line react/no-find-dom-node + const hoverBoundingRect = findDOMNode(component)?.getBoundingClientRect(); + + // Get vertical middle + const hoverMiddleY = (hoverBoundingRect.bottom - hoverBoundingRect.top) / 2; + + // Determine mouse position + const clientOffset = monitor.getClientOffset(); + + // Get pixels to the top + const hoverClientY = clientOffset.y - hoverBoundingRect.top; + + // Only perform the move when the mouse has crossed half of the items height + // When dragging downwards, only move when the cursor is below 50% + // When dragging upwards, only move when the cursor is above 50% + + // Dragging downwards + if (dragIndex < hoverIndex && hoverClientY < hoverMiddleY) { + return; + } + + // Dragging upwards + if (dragIndex > hoverIndex && hoverClientY > hoverMiddleY) { + return; + } + + // Time to actually perform the action + props.onMoveLabel?.(dragIndex, hoverIndex); + + // Note: we're mutating the monitor item here! + // Generally it's better to avoid mutations, + // but it's good here for the sake of performance + // to avoid expensive index searches. + // eslint-disable-next-line no-param-reassign + monitor.getItem().index = hoverIndex; + }, + drop(props: Record) { + return props.onDropLabel?.(); + }, +}; + export const OptionControlLabel = ({ label, savedMetric, onRemove, isAdhoc, isFunction, + isDraggable, + connectDragSource, + connectDropTarget, ...props }: { label: string | React.ReactNode; @@ -126,6 +207,9 @@ export const OptionControlLabel = ({ onRemove: () => void; isAdhoc?: boolean; isFunction?: boolean; + isDraggable?: boolean; + connectDragSource?: any; + connectDropTarget?: any; }) => { const theme = useTheme(); const getLabelContent = () => { @@ -139,7 +223,8 @@ export const OptionControlLabel = ({ } return label; }; - return ( + + const getOptionControlContent = () => ( ); + + return ( + + {isDraggable + ? connectDragSource( + connectDropTarget(
{getOptionControlContent()}
), + ) + : getOptionControlContent()} + + ); }; + +export const DraggableOptionControlLabel = DropTarget( + TYPE, + labelTarget, + (connect: any) => ({ + connectDropTarget: connect.dropTarget(), + }), +)( + DragSource(TYPE, labelSource, (connect: any) => ({ + connectDragSource: connect.dragSource(), + isDraggable: true, + }))(OptionControlLabel), +); + +DraggableOptionControlLabel.displayName = 'DraggableOptionControlLabel'; diff --git a/superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx b/superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx index 613c7f769c52d..91f5725ba427f 100644 --- a/superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx +++ b/superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx @@ -18,7 +18,6 @@ */ import React from 'react'; import PropTypes from 'prop-types'; - import { t, logging, SupersetClient, withTheme } from '@superset-ui/core'; import ControlHeader from '../ControlHeader'; @@ -39,6 +38,7 @@ import { } from '../OptionControls'; import Icon from '../../../components/Icon'; import AdhocFilterPopoverTrigger from '../AdhocFilterPopoverTrigger'; +import DndWithHTML5Backend from '../../DndContextProvider'; const propTypes = { name: PropTypes.string, @@ -75,6 +75,7 @@ class AdhocFilterControl extends React.Component { this.onRemoveFilter = this.onRemoveFilter.bind(this); this.onNewFilter = this.onNewFilter.bind(this); this.onFilterEdit = this.onFilterEdit.bind(this); + this.moveLabel = this.moveLabel.bind(this); this.onChange = this.onChange.bind(this); this.getMetricExpression = this.getMetricExpression.bind(this); @@ -86,11 +87,14 @@ class AdhocFilterControl extends React.Component { this.valueRenderer = (adhocFilter, index) => ( this.onRemoveFilter(index)} + onMoveLabel={this.moveLabel} + onDropLabel={() => this.props.onChange(this.state.values)} /> ); this.state = { @@ -256,6 +260,17 @@ class AdhocFilterControl extends React.Component { ).expression; } + moveLabel(dragIndex, hoverIndex) { + const { values } = this.state; + + const newValues = [...values]; + [newValues[hoverIndex], newValues[dragIndex]] = [ + newValues[dragIndex], + newValues[hoverIndex], + ]; + this.setState({ values: newValues }); + } + optionsForSelect(props) { const options = [ ...props.columns, @@ -349,4 +364,4 @@ class AdhocFilterControl extends React.Component { AdhocFilterControl.propTypes = propTypes; AdhocFilterControl.defaultProps = defaultProps; -export default withTheme(AdhocFilterControl); +export default DndWithHTML5Backend(withTheme(AdhocFilterControl)); diff --git a/superset-frontend/src/explore/components/controls/MetricsControl.jsx b/superset-frontend/src/explore/components/controls/MetricsControl.jsx index 1bc88c6938c40..f643e2b769ee1 100644 --- a/superset-frontend/src/explore/components/controls/MetricsControl.jsx +++ b/superset-frontend/src/explore/components/controls/MetricsControl.jsx @@ -20,7 +20,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import { t, withTheme } from '@superset-ui/core'; import { isEqual } from 'lodash'; - import ControlHeader from '../ControlHeader'; import MetricDefinitionOption from '../MetricDefinitionOption'; import MetricDefinitionValue from '../MetricDefinitionValue'; @@ -41,6 +40,7 @@ import { HeaderContainer, LabelsContainer, } from '../OptionControls'; +import DndWithHTML5Backend from '../../DndContextProvider'; const propTypes = { name: PropTypes.string.isRequired, @@ -116,6 +116,7 @@ class MetricsControl extends React.PureComponent { this.onMetricEdit = this.onMetricEdit.bind(this); this.onNewMetric = this.onNewMetric.bind(this); this.onRemoveMetric = this.onRemoveMetric.bind(this); + this.moveLabel = this.moveLabel.bind(this); this.checkIfAggregateInInput = this.checkIfAggregateInInput.bind(this); this.optionsForSelect = this.optionsForSelect.bind(this); this.selectFilterOption = this.selectFilterOption.bind(this); @@ -124,12 +125,15 @@ class MetricsControl extends React.PureComponent { this.valueRenderer = (option, index) => ( this.onRemoveMetric(index)} columns={this.props.columns} savedMetrics={this.props.savedMetrics} datasourceType={this.props.datasourceType} + onMoveLabel={this.moveLabel} + onDropLabel={() => this.props.onChange(this.state.value)} /> ); this.select = null; @@ -238,6 +242,17 @@ class MetricsControl extends React.PureComponent { this.props.onChange(this.props.multi ? optionValues : optionValues[0]); } + moveLabel(dragIndex, hoverIndex) { + const { value } = this.state; + + const newValues = [...value]; + [newValues[hoverIndex], newValues[dragIndex]] = [ + newValues[dragIndex], + newValues[hoverIndex], + ]; + this.setState({ value: newValues }); + } + isAddNewMetricDisabled() { return !this.props.multi && this.state.value.length > 0; } @@ -377,4 +392,4 @@ class MetricsControl extends React.PureComponent { MetricsControl.propTypes = propTypes; MetricsControl.defaultProps = defaultProps; -export default withTheme(MetricsControl); +export default DndWithHTML5Backend(withTheme(MetricsControl)); diff --git a/superset-frontend/src/explore/components/optionTypes.ts b/superset-frontend/src/explore/components/optionTypes.ts new file mode 100644 index 0000000000000..4d86a8fd12319 --- /dev/null +++ b/superset-frontend/src/explore/components/optionTypes.ts @@ -0,0 +1,22 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +export const OPTION_TYPES = { + metric: 'metric', + filter: 'filter', +};