feat: Implement drag & drop for metrics and filters labels (#12184)

superset-frontend/spec/javascripts/explore/components/AdhocFilterControl_spec.jsx

@@ -71,7 +71,7 @@ function setup(overrides) {
     ...overrides,
   };
   const wrapper = shallow(<AdhocFilterControl {...props} />);
-  const component = wrapper.dive().shallow();
+  const component = wrapper.dive().dive().shallow();
   return { wrapper, component, onChange };
 }
 

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();
   });
 });

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(<AdhocMetricOption {...props} />)
@@ -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', () => {

superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx

@@ -53,7 +53,7 @@ function setup(overrides) {
     ...overrides,
   };
   const wrapper = shallow(<MetricsControl {...props} />);
-  const component = wrapper.dive().shallow();
+  const component = wrapper.dive().dive().shallow();
   return { wrapper, component, onChange };
 }
 

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);

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);

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,
 }) => (
   <AdhocFilterPopoverTrigger
     adhocFilter={adhocFilter}
@@ -54,9 +61,13 @@ const AdhocFilterOption = ({
     onFilterEdit={onFilterEdit}
     partitionColumn={partitionColumn}
   >
-    <OptionControlLabel
+    <DraggableOptionControlLabel
       label={adhocFilter.getDefaultLabel()}
       onRemove={onRemoveFilter}
+      onMoveLabel={onMoveLabel}
+      onDropLabel={onDropLabel}
+      index={index}
+      type={OPTION_TYPES.filter}
       isAdhoc
     />
   </AdhocFilterPopoverTrigger>

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 (
       <AdhocMetricPopoverTrigger
@@ -63,10 +70,14 @@ class AdhocMetricOption extends React.PureComponent {
         savedMetric={savedMetric}
         datasourceType={datasourceType}
       >
-        <OptionControlLabel
+        <DraggableOptionControlLabel
           savedMetric={savedMetric}
           label={adhocMetric.label}
           onRemove={this.onRemoveMetric}
+          onMoveLabel={onMoveLabel}
+          onDropLabel={onDropLabel}
+          index={index}
+          type={OPTION_TYPES.metric}
           isAdhoc
           isFunction
         />

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 (
-      <OptionControlLabel label={option} onRemove={onRemoveMetric} isFunction />
+      <DraggableOptionControlLabel
+        label={option}
+        onRemove={onRemoveMetric}
+        onMoveLabel={onMoveLabel}
+        onDropLabel={onDropLabel}
+        type={OPTION_TYPES.metric}
+        index={index}
+        isFunction
+      />
     );
   }
   return null;

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<string, any>, 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<string, any>) {
+    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 = () => (
     <OptionControlContainer
       isAdhoc={isAdhoc}
       data-test="option-label"
@@ -163,4 +248,29 @@ export const OptionControlLabel = ({
       )}
     </OptionControlContainer>
   );
+
+  return (
+    <DragContainer>
+      {isDraggable
+        ? connectDragSource(
+            connectDropTarget(<div>{getOptionControlContent()}</div>),
+          )
+        : getOptionControlContent()}
+    </DragContainer>
+  );
 };
+
+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';

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) => (
       <AdhocFilterOption
         key={index}
+        index={index}
         adhocFilter={adhocFilter}
         onFilterEdit={this.onFilterEdit}
         options={this.state.options}
         datasource={this.props.datasource}
         onRemoveFilter={() => 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));

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) => (
       <MetricDefinitionValue
         key={index}
+        index={index}
         option={option}
         onMetricEdit={this.onMetricEdit}
         onRemoveMetric={() => 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));

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',
+};