ChartSettingOrderedItems using SortableList (#39561)

* ChartSettingOrderedItems using SortableList * e2e test adjustments * unit tests
metabase · Mar 6, 2024 · 4a78412 · 4a78412
1 parent d97e615
commit 4a78412
Show file tree

Hide file tree

Showing 11 changed files with 211 additions and 209 deletions.
diff --git a/e2e/support/helpers/e2e-ui-elements-helpers.js b/e2e/support/helpers/e2e-ui-elements-helpers.js
@@ -168,6 +168,34 @@ export const moveColumnDown = (column, distance) => {
     .trigger("mouseup", 0, distance * 50, { force: true });
 };
 
+export const moveDnDKitColumnVertical = (column, distance) => {
+  column
+    .trigger("pointerdown", 0, 0, {
+      force: true,
+      isPrimary: true,
+      button: 0,
+    })
+    .wait(200)
+    .trigger("pointermove", 5, 5, {
+      force: true,
+      isPrimary: true,
+      button: 0,
+    })
+    .wait(200)
+    .trigger("pointermove", 0, distance, {
+      force: true,
+      isPrimary: true,
+      button: 0,
+    })
+    .wait(200)
+    .trigger("pointerup", 0, distance, {
+      force: true,
+      isPrimary: true,
+      button: 0,
+    })
+    .wait(200);
+};
+
 export const queryBuilderMain = () => {
   return cy.findByTestId("query-builder-main");
 };

diff --git a/e2e/test/scenarios/dashboard-cards/visualization-options.cy.spec.js b/e2e/test/scenarios/dashboard-cards/visualization-options.cy.spec.js
@@ -9,6 +9,8 @@ import {
   modal,
   saveDashboard,
   getDashboardCardMenu,
+  getDraggableElements,
+  moveDnDKitColumnVertical,
 } from "e2e/support/helpers";
 
 describe("scenarios > dashboard cards > visualization options", () => {
@@ -49,12 +51,7 @@ describe("scenarios > dashboard cards > visualization options", () => {
     getDashboardCard().realHover();
     cy.findByLabelText("Show visualization options").click();
     cy.findByTestId("chartsettings-sidebar").within(() => {
-      cy.findByText("ID")
-        .closest("[data-testid^=draggable-item]")
-        .trigger("mousedown", 0, 0, { force: true })
-        .trigger("mousemove", 5, 5, { force: true })
-        .trigger("mousemove", 0, 100, { force: true })
-        .trigger("mouseup", 0, 100, { force: true });
+      moveDnDKitColumnVertical(getDraggableElements().contains("ID"), 100);
 
       /**
        * When this issue gets fixed, it should be safe to uncomment the following assertion.

diff --git a/e2e/test/scenarios/question/settings.cy.spec.js b/e2e/test/scenarios/question/settings.cy.spec.js
@@ -9,7 +9,7 @@ import {
   popover,
   modal,
   sidebar,
-  moveColumnDown,
+  moveDnDKitColumnVertical,
 } 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");
 
-      moveColumnDown(cy.get("@total"), -2);
+      moveDnDKitColumnVertical(cy.get("@total"), -100);
 
       getSidebarColumns().eq("3").should("contain.text", "Total");
 
@@ -114,12 +114,7 @@ describe("scenarios > question > settings", () => {
         expect($el.scrollTop).to.eql(0);
       });
 
-      cy.get("@title")
-        .trigger("mousedown", 0, 0, { force: true })
-        .trigger("mousemove", 5, 5, { force: true })
-        .trigger("mousemove", 0, 15, { force: true });
-      cy.wait(2000);
-      cy.get("@title").trigger("mouseup", 0, 15, { force: true });
+      moveDnDKitColumnVertical(cy.get("@title"), 15);
 
       cy.findByTestId("chartsettings-sidebar").should(([$el]) => {
         expect($el.scrollTop).to.be.greaterThan(0);
@@ -161,11 +156,7 @@ describe("scenarios > question > settings", () => {
         .contains(/Products? → Category/);
 
       // Drag and drop this column between "Tax" and "Discount" (index 5 in @sidebarColumns array)
-      cy.get("@prod-category")
-        .trigger("mousedown", 0, 0, { force: true })
-        .trigger("mousemove", 5, 5, { force: true })
-        .trigger("mousemove", 0, -350, { force: true })
-        .trigger("mouseup", 0, -350, { force: true });
+      moveDnDKitColumnVertical(cy.get("@prod-category"), -360);
 
       refreshResultsInHeader();
 
@@ -196,12 +187,7 @@ describe("scenarios > question > settings", () => {
       findColumnAtIndex("User → Address", -1).as("user-address");
 
       // Move it one place up
-      cy.get("@user-address")
-        .scrollIntoView()
-        .trigger("mousedown", 0, 0, { force: true })
-        .trigger("mousemove", 5, 5, { force: true })
-        .trigger("mousemove", 0, -100, { force: true })
-        .trigger("mouseup", 0, -100, { force: true });
+      moveDnDKitColumnVertical(cy.get("@user-address"), -100);
 
       findColumnAtIndex("User → Address", -3);
 

diff --git a/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js b/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js
@@ -5,10 +5,10 @@ import {
   visitQuestionAdhoc,
   sidebar,
   getDraggableElements,
-  moveColumnDown,
   popover,
   visitDashboard,
   cypressWaitAll,
+  moveDnDKitColumnVertical,
 } from "e2e/support/helpers";
 
 const { ORDERS, ORDERS_ID, PEOPLE, PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE;
@@ -143,12 +143,14 @@ describe("scenarios > visualizations > bar chart", () => {
     });
 
     it("should allow you to show/hide and reorder columns", () => {
-      moveColumnDown(getDraggableElements().eq(0), 2);
+      moveDnDKitColumnVertical(getDraggableElements().eq(0), 100);
 
-      getDraggableElements().each((element, index) => {
-        const draggableName = element[0].innerText;
-        cy.findAllByTestId("legend-item").eq(index).contains(draggableName);
-      });
+      cy.findAllByTestId("legend-item").eq(0).should("contain.text", "Gadget");
+      cy.findAllByTestId("legend-item").eq(1).should("contain.text", "Gizmo");
+      cy.findAllByTestId("legend-item")
+        .eq(2)
+        .should("contain.text", "Doohickey");
+      cy.findAllByTestId("legend-item").eq(3).should("contain.text", "Widget");
 
       const columnIndex = 1;
 
@@ -190,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", () => {
-      moveColumnDown(getDraggableElements().first(), 2);
+      moveDnDKitColumnVertical(getDraggableElements().first(), 100);
 
       getDraggableElements()
         .eq(1)

diff --git a/e2e/test/scenarios/visualizations-charts/funnel.cy.spec.js b/e2e/test/scenarios/visualizations-charts/funnel.cy.spec.js
@@ -5,8 +5,8 @@ import {
   visitQuestionAdhoc,
   sidebar,
   getDraggableElements,
-  moveColumnDown,
   popover,
+  moveDnDKitColumnVertical,
 } from "e2e/support/helpers";
 
 const { PEOPLE_ID, PEOPLE } = SAMPLE_DATABASE;
@@ -32,7 +32,7 @@ describe("scenarios > visualizations > funnel chart", () => {
     sidebar().findByText("Data").click();
   });
 
-  it("hould allow you to reorder and show/hide rows", () => {
+  it("should allow you to reorder and show/hide rows", () => {
     cy.log("ensure that rows are shown");
     getDraggableElements().should("have.length", 5);
 
@@ -45,7 +45,7 @@ describe("scenarios > visualizations > funnel chart", () => {
           .first()
           .should("have.text", name);
 
-        moveColumnDown(getDraggableElements().first(), 2);
+        moveDnDKitColumnVertical(getDraggableElements().first(), 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", () => {
-    moveColumnDown(getDraggableElements().first(), 2);
+    moveDnDKitColumnVertical(getDraggableElements().first(), 100);
 
     getDraggableElements()
       .eq(1)

diff --git a/...est/scenarios/visualizations-tabular/reproductions/28311-sorting-table-columns.cy.spec.js b/...est/scenarios/visualizations-tabular/reproductions/28311-sorting-table-columns.cy.spec.js
@@ -4,6 +4,7 @@ import {
   restore,
   visitQuestionAdhoc,
   getDraggableElements,
+  moveDnDKitColumnVertical,
 } from "e2e/support/helpers";
 
 const { ORDERS_ID, ORDERS } = SAMPLE_DATABASE;
@@ -61,15 +62,10 @@ describe("issue 25250", () => {
     cy.findByText("Product ID").should("be.visible");
 
     cy.findByTestId("viz-settings-button").click();
-    moveColumnUp(getDraggableElements().contains("Product ID"), 2);
+    moveDnDKitColumnVertical(
+      getDraggableElements().contains("Product ID"),
+      -100,
+    );
     getDraggableElements().eq(0).should("contain", "Product ID");
   });
 });
-
-function moveColumnUp(column, distance) {
-  column
-    .trigger("mousedown", 0, 0, { force: true })
-    .trigger("mousemove", 5, -5, { force: true })
-    .trigger("mousemove", 0, distance * -50, { force: true })
-    .trigger("mouseup", 0, distance * -50, { force: true });
-}
diff --git a/frontend/src/metabase/core/components/Sortable/SortableList.tsx b/frontend/src/metabase/core/components/Sortable/SortableList.tsx
@@ -12,6 +12,10 @@ import _ from "underscore";
 import { isNotNull } from "metabase/lib/types";
 
 type ItemId = number | string;
+export type DragEndEvent = {
+  id: ItemId;
+  newIndex: number;
+};
 
 interface RenderItemProps<T> {
   item: T;
@@ -23,7 +27,7 @@ interface useSortableListProps<T> {
   getId: (item: T) => ItemId;
   renderItem: ({ item, id, isDragOverlay }: RenderItemProps<T>) => JSX.Element;
   onSortStart?: (event: DragStartEvent) => void;
-  onSortEnd?: ({ id, newIndex }: { id: ItemId; newIndex: number }) => void;
+  onSortEnd?: ({ id, newIndex }: DragEndEvent) => void;
   sensors?: SensorDescriptor<any>[];
   modifiers?: Modifier[];
 }