backported "40232 sortable with popovers" (#40474)

* 40232 sortable with popovers (#40378)

* custom sensor

* e2e test, moving sortable

* e2e test adjustment

* combining props

* backporting move DnDKitElement

* adjusting e2e test

---------

Co-authored-by: Nick Fitzpatrick <nick@metabase.com>
Co-authored-by: Nick Fitzpatrick <nickfitz.582@gmail.com>

e2e/support/helpers/e2e-ui-elements-helpers.js

@@ -153,8 +153,11 @@ export const moveColumnDown = (column, distance) => {
     .trigger("mouseup", 0, distance * 50, { force: true });
 };
 
-export const moveDnDKitColumnVertical = (column, distance) => {
-  column
+export const moveDnDKitElement = (
+  element,
+  { horizontal = 0, vertical = 0 } = {},
+) => {
+  element
     .trigger("pointerdown", 0, 0, {
       force: true,
       isPrimary: true,
@@ -167,13 +170,13 @@ export const moveDnDKitColumnVertical = (column, distance) => {
       button: 0,
     })
     .wait(200)
-    .trigger("pointermove", 0, distance, {
+    .trigger("pointermove", horizontal, vertical, {
       force: true,
       isPrimary: true,
       button: 0,
     })
     .wait(200)
-    .trigger("pointerup", 0, distance, {
+    .trigger("pointerup", horizontal, vertical, {
       force: true,
       isPrimary: true,
       button: 0,

e2e/test/scenarios/dashboard-cards/visualization-options.cy.spec.js

@@ -10,7 +10,7 @@ import {
   saveDashboard,
   getDashboardCardMenu,
   getDraggableElements,
-  moveDnDKitColumnVertical,
+  moveDnDKitElement,
 } from "e2e/support/helpers";
 
 describe("scenarios > dashboard cards > visualization options", () => {
@@ -51,7 +51,9 @@ describe("scenarios > dashboard cards > visualization options", () => {
     getDashboardCard().realHover();
     cy.findByLabelText("Show visualization options").click();
     cy.findByTestId("chartsettings-sidebar").within(() => {
-      moveDnDKitColumnVertical(getDraggableElements().contains("ID"), 100);
+      moveDnDKitElement(getDraggableElements().contains("ID"), {
+        vertical: 100,
+      });
 
       /**
        * When this issue gets fixed, it should be safe to uncomment the following assertion.

e2e/test/scenarios/native-filters/reproductions/40232-dragging-filters.cy.spec.js

+import {
+  restore,
+  openNativeEditor,
+  filterWidget,
+  popover,
+  moveDnDKitElement,
+} from "e2e/support/helpers";
+
+import * as SQLFilter from "../helpers/e2e-sql-filter-helpers";
+
+describe("issue 40232", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsAdmin();
+  });
+
+  it("should not start drag and drop from clicks on popovers", () => {
+    openNativeEditor();
+
+    SQLFilter.enterParameterizedQuery("{{foo}} {{bar}}");
+
+    cy.findAllByRole("radio", { name: "Search box" })
+      .first()
+      .click({ force: true });
+    filterWidget().first().click();
+
+    moveDnDKitElement(popover().findByText("Add filter"), {
+      horizontal: 300,
+    });
+
+    filterWidget()
+      .should("have.length", 2)
+      .first()
+      .should("contain.text", "Foo");
+  });
+});

e2e/test/scenarios/native-filters/reproductions/9357.cy.spec.js

@@ -16,6 +16,7 @@ describe("issue 9357", () => {
 
     // Drag the firstparameter to last position
     cy.get("fieldset")
+      .findAllByRole("listitem")
       .first()
       .trigger("pointerdown", 0, 0, { force: true, isPrimary: true, button: 0 })
       .wait(200)

e2e/test/scenarios/question/settings.cy.spec.js

@@ -9,7 +9,7 @@ import {
   popover,
   modal,
   sidebar,
-  moveDnDKitColumnVertical,
+  moveDnDKitElement,
 } from "e2e/support/helpers";
 
 const { ORDERS, ORDERS_ID, PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE;
@@ -100,7 +100,7 @@ describe("scenarios > question > settings", () => {
 
       getSidebarColumns().eq("5").as("total").contains("Total");
 
-      moveDnDKitColumnVertical(cy.get("@total"), -100);
+      moveDnDKitElement(cy.get("@total"), { vertical: -100 });
 
       getSidebarColumns().eq("3").should("contain.text", "Total");
 
@@ -114,7 +114,7 @@ describe("scenarios > question > settings", () => {
         expect($el.scrollTop).to.eql(0);
       });
 
-      moveDnDKitColumnVertical(cy.get("@title"), 15);
+      moveDnDKitElement(cy.get("@title"), { vertical: 15 });
 
       cy.findByTestId("chartsettings-sidebar").should(([$el]) => {
         expect($el.scrollTop).to.be.greaterThan(0);
@@ -156,7 +156,7 @@ describe("scenarios > question > settings", () => {
         .contains(/Products? → Category/);
 
       // Drag and drop this column between "Tax" and "Discount" (index 5 in @sidebarColumns array)
-      moveDnDKitColumnVertical(cy.get("@prod-category"), -360);
+      moveDnDKitElement(cy.get("@prod-category"), { vertical: -360 });
 
       refreshResultsInHeader();
 
@@ -187,7 +187,7 @@ describe("scenarios > question > settings", () => {
       findColumnAtIndex("User → Address", -1).as("user-address");
 
       // Move it one place up
-      moveDnDKitColumnVertical(cy.get("@user-address"), -100);
+      moveDnDKitElement(cy.get("@user-address"), { vertical: -100 });
 
       findColumnAtIndex("User → Address", -3);
 

e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js

@@ -8,7 +8,7 @@ import {
   popover,
   visitDashboard,
   cypressWaitAll,
-  moveDnDKitColumnVertical,
+  moveDnDKitElement,
 } from "e2e/support/helpers";
 
 const { ORDERS, ORDERS_ID, PEOPLE, PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE;
@@ -143,7 +143,7 @@ describe("scenarios > visualizations > bar chart", () => {
     });
 
     it("should allow you to show/hide and reorder columns", () => {
-      moveDnDKitColumnVertical(getDraggableElements().eq(0), 100);
+      moveDnDKitElement(getDraggableElements().eq(0), { vertical: 100 });
 
       cy.findAllByTestId("legend-item").eq(0).should("contain.text", "Gadget");
       cy.findAllByTestId("legend-item").eq(1).should("contain.text", "Gizmo");
@@ -192,7 +192,7 @@ describe("scenarios > visualizations > bar chart", () => {
     });
 
     it("should gracefully handle removing filtered items, and adding new items to the end of the list", () => {
-      moveDnDKitColumnVertical(getDraggableElements().first(), 100);
+      moveDnDKitElement(getDraggableElements().first(), { vertical: 100 });
 
       getDraggableElements()
         .eq(1)

e2e/test/scenarios/visualizations-charts/funnel.cy.spec.js

@@ -6,7 +6,7 @@ import {
   sidebar,
   getDraggableElements,
   popover,
-  moveDnDKitColumnVertical,
+  moveDnDKitElement,
 } from "e2e/support/helpers";
 
 const { PEOPLE_ID, PEOPLE } = SAMPLE_DATABASE;
@@ -45,7 +45,7 @@ describe("scenarios > visualizations > funnel chart", () => {
           .first()
           .should("have.text", name);
 
-        moveDnDKitColumnVertical(getDraggableElements().first(), 100);
+        moveDnDKitElement(getDraggableElements().first(), { vertical: 100 });
 
         getDraggableElements().eq(2).should("have.text", name);
 
@@ -71,7 +71,7 @@ describe("scenarios > visualizations > funnel chart", () => {
   });
 
   it("should handle row items being filterd out and returned gracefully", () => {
-    moveDnDKitColumnVertical(getDraggableElements().first(), 100);
+    moveDnDKitElement(getDraggableElements().first(), { vertical: 100 });
 
     getDraggableElements()
       .eq(1)

e2e/test/scenarios/visualizations-tabular/reproductions/28311-sorting-table-columns.cy.spec.js

@@ -4,7 +4,7 @@ import {
   restore,
   visitQuestionAdhoc,
   getDraggableElements,
-  moveDnDKitColumnVertical,
+  moveDnDKitElement,
 } from "e2e/support/helpers";
 
 const { ORDERS_ID, ORDERS } = SAMPLE_DATABASE;
@@ -62,10 +62,9 @@ describe("issue 25250", () => {
     cy.findByText("Product ID").should("be.visible");
 
     cy.findByTestId("viz-settings-button").click();
-    moveDnDKitColumnVertical(
-      getDraggableElements().contains("Product ID"),
-      -100,
-    );
+    moveDnDKitElement(getDraggableElements().contains("Product ID"), {
+      vertical: -100,
+    });
     getDraggableElements().eq(0).should("contain", "Product ID");
   });
 });

e2e/test/scenarios/visualizations-tabular/table.cy.spec.js

@@ -17,7 +17,7 @@ import {
   getTable,
   leftSidebar,
   sidebar,
-  moveDnDKitColumnVertical,
+  moveDnDKitElement,
 } from "e2e/support/helpers";
 
 describe("scenarios > visualizations > table", () => {
@@ -415,10 +415,9 @@ describe("scenarios > visualizations > table > conditional formatting", () => {
         .first()
         .should("contain.text", "is less than 20");
 
-      moveDnDKitColumnVertical(
-        cy.findAllByTestId("formatting-rule-preview").eq(2),
-        -300,
-      );
+      moveDnDKitElement(cy.findAllByTestId("formatting-rule-preview").eq(2), {
+        vertical: -300,
+      });
 
       cy.findAllByTestId("formatting-rule-preview")
         .first()

frontend/src/metabase/parameters/components/ParameterValueWidget.jsx

@@ -11,6 +11,7 @@ import DateRelativeWidget from "metabase/components/DateRelativeWidget";
 import DateSingleWidget from "metabase/components/DateSingleWidget";
 import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
 import { TextWidget } from "metabase/components/TextWidget";
+import { Sortable } from "metabase/core/components/Sortable";
 import FormattedParameterValue from "metabase/parameters/components/FormattedParameterValue";
 import { WidgetStatusIcon } from "metabase/parameters/components/WidgetStatusIcon";
 import NumberInputWidget from "metabase/parameters/components/widgets/NumberInputWidget";
@@ -64,6 +65,8 @@ class ParameterValueWidget extends Component {
     // Should be used for dashboards and native questions in the parameter bar,
     // Don't use in settings sidebars.
     enableRequiredBehavior: PropTypes.bool,
+    mimicMantine: PropTypes.bool,
+    isSortable: PropTypes.bool,
   };
 
   state = { isFocused: false };
@@ -158,6 +161,21 @@ class ParameterValueWidget extends Component {
     }
   }
 
+  wrapSortable(children) {
+    const { isSortable = false, parameter } = this.props;
+
+    return (
+      <Sortable
+        id={parameter.id}
+        draggingStyle={{ opacity: 0.5 }}
+        disabled={!isSortable}
+        role="listitem"
+      >
+        {children}
+      </Sortable>
+    );
+  }
+
   render() {
     const { parameter, value, isEditing, placeholder, className } = this.props;
     const { isFocused } = this.state;
@@ -167,7 +185,7 @@ class ParameterValueWidget extends Component {
     const showTypeIcon = !isEditing && !hasValue && !isFocused;
 
     if (noPopover) {
-      return (
+      return this.wrapSortable(
         <div
           ref={this.trigger}
           className={cx(S.parameter, S.noPopover, className, {
@@ -188,7 +206,7 @@ class ParameterValueWidget extends Component {
             onPopoverClose={this.onPopoverClose}
           />
           {this.getActionIcon()}
-        </div>
+        </div>,
       );
     }
 
@@ -202,7 +220,7 @@ class ParameterValueWidget extends Component {
       <PopoverWithTrigger
         ref={this.valuePopover}
         targetOffsetX={16}
-        triggerElement={
+        triggerElement={this.wrapSortable(
           <div
             ref={this.trigger}
             className={cx(S.parameter, className, {
@@ -226,8 +244,8 @@ class ParameterValueWidget extends Component {
               />
             </div>
             {this.getActionIcon()}
-          </div>
-        }
+          </div>,
+        )}
         target={this.getTargetRef}
         // make sure the full date picker will expand to fit the dual calendars
         autoWidth={parameter.type === "date/all-options"}

frontend/src/metabase/parameters/components/ParameterWidget/ParameterWidget.jsx

@@ -2,6 +2,8 @@
 import PropTypes from "prop-types";
 import { Component } from "react";
 
+import { Sortable } from "metabase/core/components/Sortable";
+
 import ParameterValueWidget from "../ParameterValueWidget";
 
 import {
@@ -27,6 +29,7 @@ export class ParameterWidget extends Component {
   static defaultProps = {
     parameter: null,
     commitImmediately: false,
+    isSortable: false,
   };
 
   renderPopover(value, setValue, placeholder, isFullscreen) {
@@ -39,6 +42,8 @@ export class ParameterWidget extends Component {
       parameters,
       setParameterValueToDefault,
       enableParameterRequiredBehavior,
+      isSortable,
+      isEditing,
     } = this.props;
 
     const isEditingParameter = editingParameter?.id === parameter.id;
@@ -59,6 +64,7 @@ export class ParameterWidget extends Component {
         commitImmediately={commitImmediately}
         setParameterValueToDefault={setParameterValueToDefault}
         enableRequiredBehavior={enableParameterRequiredBehavior}
+        isSortable={isSortable && isEditing}
       />
     );
   }
@@ -109,18 +115,25 @@ export class ParameterWidget extends Component {
     };
 
     const renderEditing = () => (
-      <ParameterContainer
-        isEditingParameter={isEditingParameter}
-        onClick={() =>
-          setEditingParameter(isEditingParameter ? null : parameter.id)
-        }
+      <Sortable
+        id={parameter.id}
+        draggingStyle={{ opacity: 0.5 }}
+        disabled={!isEditing}
+        role="listitem"
       >
-        <div className="mr1" onClick={e => e.stopPropagation()}>
-          {dragHandle}
-        </div>
-        {parameter.name}
-        <SettingsIcon name="gear" size={16} />
-      </ParameterContainer>
+        <ParameterContainer
+          isEditingParameter={isEditingParameter}
+          onClick={() =>
+            setEditingParameter(isEditingParameter ? null : parameter.id)
+          }
+        >
+          <div className="mr1" onClick={e => e.stopPropagation()}>
+            {dragHandle}
+          </div>
+          {parameter.name}
+          <SettingsIcon name="gear" size={16} />
+        </ParameterContainer>
+      </Sortable>
     );
 
     if (isFullscreen) {

frontend/src/metabase/parameters/components/ParametersList.jsx

@@ -3,7 +3,7 @@ import { useSensor, PointerSensor } from "@dnd-kit/core";
 import cx from "classnames";
 import { useCallback, useMemo } from "react";
 
-import { SortableList, Sortable } from "metabase/core/components/Sortable";
+import { SortableList } from "metabase/core/components/Sortable";
 import { getVisibleParameters } from "metabase/parameters/utils/ui";
 import { Icon } from "metabase/ui";
 
@@ -33,8 +33,9 @@ function ParametersList({
   enableParameterRequiredBehavior,
 }) {
   const pointerSensor = useSensor(PointerSensor, {
-    activationConstraint: { distance: 0 },
+    activationConstraint: { distance: 5 },
   });
+
   const visibleValuePopulatedParameters = useMemo(
     () => getVisibleParameters(parameters, hideParameters),
     [parameters, hideParameters],
@@ -50,40 +51,34 @@ function ParametersList({
   );
 
   const renderItem = ({ item: valuePopulatedParameter, id }) => (
-    <Sortable
-      id={id}
+    <ParameterWidget
       key={`sortable-${id}`}
-      disabled={!isEditing}
-      draggingStyle={{ opacity: 0.5 }}
-      role="listitem"
-    >
-      <ParameterWidget
-        className={cx({ mb2: vertical })}
-        isEditing={isEditing}
-        isFullscreen={isFullscreen}
-        isNightMode={isNightMode}
-        parameter={valuePopulatedParameter}
-        parameters={parameters}
-        question={question}
-        dashboard={dashboard}
-        editingParameter={editingParameter}
-        setEditingParameter={setEditingParameter}
-        setValue={
-          setParameterValue &&
-          (value => setParameterValue(valuePopulatedParameter.id, value))
-        }
-        setParameterValueToDefault={setParameterValueToDefault}
-        enableParameterRequiredBehavior={enableParameterRequiredBehavior}
-        commitImmediately={commitImmediately}
-        dragHandle={
-          isEditing && setParameterIndex ? (
-            <div className="flex layout-centered cursor-grab text-inherit">
-              <Icon name="grabber" />
-            </div>
-          ) : null
-        }
-      />
-    </Sortable>
+      className={cx({ mb2: vertical })}
+      isEditing={isEditing}
+      isFullscreen={isFullscreen}
+      isNightMode={isNightMode}
+      parameter={valuePopulatedParameter}
+      parameters={parameters}
+      question={question}
+      dashboard={dashboard}
+      editingParameter={editingParameter}
+      setEditingParameter={setEditingParameter}
+      setValue={
+        setParameterValue &&
+        (value => setParameterValue(valuePopulatedParameter.id, value))
+      }
+      setParameterValueToDefault={setParameterValueToDefault}
+      enableParameterRequiredBehavior={enableParameterRequiredBehavior}
+      commitImmediately={commitImmediately}
+      dragHandle={
+        isEditing && setParameterIndex ? (
+          <div className="flex layout-centered cursor-grab text-inherit">
+            <Icon name="grabber" />
+          </div>
+        ) : null
+      }
+      isSortable
+    />
   );
 
   return visibleValuePopulatedParameters.length > 0 ? (