PanelEdit: Drag and drop query order & UI tweaks (#27502)

* PanelEdit: Drag and drop query order & UI tweaks

* Fixed width of title issue

* added correct color and hover

* Updated e2e tests

e2e/suite1/specs/panelEdit_queries.spec.ts

@@ -75,44 +75,6 @@ e2e.scenario({
 
     e2e().wait('@apiPostQuery');
 
-    // Change order or query rows
-    // Check the order of the rows before
-    e2e.components.QueryEditorRows.rows()
-      .eq(0)
-      .within(() => {
-        e2e.components.QueryEditorRow.title('B').should('be.visible');
-      });
-
-    e2e.components.QueryEditorRows.rows()
-      .eq(1)
-      .within(() => {
-        e2e.components.QueryEditorRow.title('A').should('be.visible');
-      });
-
-    // Change so A is first
-    e2e.components.QueryEditorRow.actionButton('Move query up')
-      .eq(1)
-      .click();
-
-    e2e().wait('@apiPostQuery');
-
-    // Avoid flaky tests
-    // Maybe the virtual dom performs optimzations such as node position swapping, meaning 1 becomes 0 and it gets that element before the change because and never finds title 'A'
-    e2e().wait(250);
-
-    // Check the order of the rows after change
-    e2e.components.QueryEditorRows.rows()
-      .eq(0)
-      .within(() => {
-        e2e.components.QueryEditorRow.title('A').should('be.visible');
-      });
-
-    e2e.components.QueryEditorRows.rows()
-      .eq(1)
-      .within(() => {
-        e2e.components.QueryEditorRow.title('B').should('be.visible');
-      });
-
     // Disable / enable row
     expectInspectorResultAndClose(keys => {
       const length = keys.length;
@@ -120,7 +82,7 @@ e2e.scenario({
       expect(keys[length - 1].innerText).equals('B:');
     });
 
-    // Disable row with refId B
+    // Disable row with refId A
     e2e.components.QueryEditorRow.actionButton('Disable/enable query')
       .eq(1)
       .should('be.visible')
@@ -130,7 +92,7 @@ e2e.scenario({
 
     expectInspectorResultAndClose(keys => {
       const length = keys.length;
-      expect(keys[length - 1].innerText).equals('A:');
+      expect(keys[length - 1].innerText).equals('B:');
     });
 
     // Enable row with refId B

public/app/core/components/QueryOperationRow/QueryOperationRow.tsx

 import React, { useState, useCallback } from 'react';
-import { HorizontalGroup, Icon, renderOrCallToRender, stylesFactory, useTheme } from '@grafana/ui';
+import { Icon, renderOrCallToRender, stylesFactory, useTheme } from '@grafana/ui';
 import { GrafanaTheme } from '@grafana/data';
 import { css } from 'emotion';
 import { useUpdateEffect } from 'react-use';
@@ -66,20 +66,20 @@ export const QueryOperationRow: React.FC<QueryOperationRowProps> = ({
 
   const rowHeader = (
     <div className={styles.header}>
-      <HorizontalGroup justify="space-between">
       <div className={styles.titleWrapper} onClick={onRowToggle} aria-label="Query operation row title">
-          {draggable && (
-            <Icon title="Drag and drop to reorder" name="draggabledots" size="lg" className={styles.dragIcon} />
-          )}
         <Icon name={isContentVisible ? 'angle-down' : 'angle-right'} className={styles.collapseIcon} />
         {title && <span className={styles.title}>{titleElement}</span>}
         {headerElement}
       </div>
       {actions && actionsElement}
-      </HorizontalGroup>
+      {draggable && (
+        <Icon title="Drag and drop to reorder" name="draggabledots" size="lg" className={styles.dragIcon} />
+      )}
     </div>
   );
-  return draggable ? (
+
+  if (draggable) {
+    return (
       <Draggable draggableId={id} index={index}>
         {provided => {
           return (
@@ -92,7 +92,10 @@ export const QueryOperationRow: React.FC<QueryOperationRowProps> = ({
           );
         }}
       </Draggable>
-  ) : (
+    );
+  }
+
+  return (
     <div className={styles.wrapper}>
       {rowHeader}
       {isContentVisible && <div className={styles.content}>{children}</div>}
@@ -116,8 +119,11 @@ const getQueryOperationRowStyles = stylesFactory((theme: GrafanaTheme) => {
       justify-content: space-between;
     `,
     dragIcon: css`
-      opacity: 0.4;
       cursor: drag;
+      color: ${theme.colors.textWeak};
+      &:hover {
+        color: ${theme.colors.text};
+      }
     `,
     collapseIcon: css`
       color: ${theme.colors.textWeak};
@@ -128,7 +134,10 @@ const getQueryOperationRowStyles = stylesFactory((theme: GrafanaTheme) => {
     titleWrapper: css`
       display: flex;
       align-items: center;
+      flex-grow: 1;
       cursor: pointer;
+      overflow: hidden;
+      margin-right: ${theme.spacing.sm};
     `,
     title: css`
       font-weight: ${theme.typography.weight.semibold};

public/app/features/dashboard/components/TransformationsEditor/TransformationOperationRow.tsx

@@ -27,7 +27,7 @@ export const TransformationOperationRow: React.FC<TransformationOperationRowProp
 
   const renderActions = ({ isOpen }: { isOpen: boolean }) => {
     return (
-      <HorizontalGroup align="center">
+      <HorizontalGroup align="center" width="auto">
         <QueryOperationAction
           title="Debug"
           disabled={!isOpen}

public/app/features/dashboard/panel_editor/QueryEditorRow.tsx

@@ -37,7 +37,6 @@ interface Props {
   index: number;
   onAddQuery: (query?: DataQuery) => void;
   onRemoveQuery: (query: DataQuery) => void;
-  onMoveQuery: (query: DataQuery, direction: number) => void;
   onChange: (query: DataQuery) => void;
 }
 
@@ -232,7 +231,7 @@ export class QueryEditorRow extends PureComponent<Props, State> {
     const isDisabled = query.hide;
 
     return (
-      <HorizontalGroup>
+      <HorizontalGroup width="auto">
         {hasTextEditMode && (
           <QueryOperationAction
             title="Toggle text edit mode"
@@ -242,13 +241,6 @@ export class QueryEditorRow extends PureComponent<Props, State> {
             }}
           />
         )}
-        <QueryOperationAction
-          title="Move query down"
-          icon="arrow-down"
-          onClick={() => this.props.onMoveQuery(query, 1)}
-        />
-        <QueryOperationAction title="Move query up" icon="arrow-up" onClick={() => this.props.onMoveQuery(query, -1)} />
-
         <QueryOperationAction title="Duplicate query" icon="copy" onClick={this.onCopyQuery} />
         <QueryOperationAction
           title="Disable/enable query"
@@ -297,6 +289,7 @@ export class QueryEditorRow extends PureComponent<Props, State> {
       <div aria-label={selectors.components.QueryEditorRows.rows}>
         <QueryOperationRow
           id={id}
+          draggable={true}
           index={index}
           title={this.renderTitle}
           actions={this.renderActions}

public/app/features/dashboard/panel_editor/QueryEditorRows.tsx

 // Libraries
 import React, { PureComponent } from 'react';
 
-// @ts-ignore ignoring this for now, otherwise we would have to extend _ interface with move
-import _ from 'lodash';
-
 // Types
 import { PanelModel } from '../state/PanelModel';
 import { DataQuery, PanelData, DataSourceSelectItem } from '@grafana/data';
 import { DashboardModel } from '../state/DashboardModel';
 import { QueryEditorRow } from './QueryEditorRow';
 import { addQuery } from 'app/core/utils/query';
+import { DragDropContext, Droppable, DropResult } from 'react-beautiful-dnd';
 
 interface Props {
   // The query configuration
@@ -44,16 +42,6 @@ export class QueryEditorRows extends PureComponent<Props> {
     panel.refresh();
   };
 
-  onMoveQuery = (query: DataQuery, direction: number) => {
-    const { queries, onChangeQueries, panel } = this.props;
-
-    const index = _.indexOf(queries, query);
-    // @ts-ignore
-    _.move(queries, index, index + direction);
-    onChangeQueries(queries);
-    panel.refresh();
-  };
-
   onChangeQuery(query: DataQuery, index: number) {
     const { queries, onChangeQueries } = this.props;
 
@@ -76,9 +64,35 @@ export class QueryEditorRows extends PureComponent<Props> {
     );
   }
 
+  onDragEnd = (result: DropResult) => {
+    const { queries, onChangeQueries, panel } = this.props;
+
+    if (!result || !result.destination) {
+      return;
+    }
+
+    const startIndex = result.source.index;
+    const endIndex = result.destination.index;
+    if (startIndex === endIndex) {
+      return;
+    }
+
+    const update = Array.from(queries);
+    const [removed] = update.splice(startIndex, 1);
+    update.splice(endIndex, 0, removed);
+    onChangeQueries(update);
+    panel.refresh();
+  };
+
   render() {
     const { props } = this;
-    return props.queries.map((query, index) => (
+    return (
+      <DragDropContext onDragEnd={this.onDragEnd}>
+        <Droppable droppableId="transformations-list" direction="vertical">
+          {provided => {
+            return (
+              <div ref={provided.innerRef} {...provided.droppableProps}>
+                {props.queries.map((query, index) => (
                   <QueryEditorRow
                     dataSourceValue={query.datasource || props.datasource.value}
                     id={query.refId}
@@ -91,9 +105,14 @@ export class QueryEditorRows extends PureComponent<Props> {
                     onChange={query => this.onChangeQuery(query, index)}
                     onRemoveQuery={this.onRemoveQuery}
                     onAddQuery={this.onAddQuery}
-        onMoveQuery={this.onMoveQuery}
                     inMixedMode={props.datasource.meta.mixed}
                   />
-    ));
+                ))}
+              </div>
+            );
+          }}
+        </Droppable>
+      </DragDropContext>
+    );
   }
 }