From 06fef3905a4d7bc2cbb926579cbf3efd8bd85e93 Mon Sep 17 00:00:00 2001
From: Kamil Mielnik <kamil@kamilmielnik.com>
Date: Tue, 19 Nov 2024 18:21:00 +0700
Subject: [PATCH] Fix - Remove clause button shrinks in the notebook editor
 (#50150)

* Fix #50128

* Add repro for #50128
---
 .../scenarios/question/notebook.cy.spec.js    | 50 +++++++++++++++++++
 .../ClauseStep/ClauseStep.module.css          |  4 ++
 .../components/ClauseStep/ClauseStep.tsx      |  4 +-
 3 files changed, 56 insertions(+), 2 deletions(-)
 create mode 100644 frontend/src/metabase/querying/notebook/components/ClauseStep/ClauseStep.module.css

diff --git a/e2e/test/scenarios/question/notebook.cy.spec.js b/e2e/test/scenarios/question/notebook.cy.spec.js
index 97436b96a5a..b75570fee28 100644
--- a/e2e/test/scenarios/question/notebook.cy.spec.js
+++ b/e2e/test/scenarios/question/notebook.cy.spec.js
@@ -23,6 +23,7 @@ import {
   selectFilterOperator,
   startNewQuestion,
   summarize,
+  verifyNotebookQuery,
   visitQuestionAdhoc,
   visualize,
 } from "e2e/support/helpers";
@@ -969,6 +970,55 @@ describe("scenarios > question > notebook", { tags: "@slow" }, () => {
         cy.findByText("More…").should("not.exist");
       });
   });
+
+  it("should not shrink the remove clause button (metabase#50128)", () => {
+    const CUSTOM_COLUMN_LONG_NAME = "very-very-very-long-name";
+
+    // The issue is reproducible on all viewports, but the smaller the viewport is,
+    // the more likely the issue is going to occur.
+    cy.viewport(300, 800);
+    createQuestion(
+      {
+        query: {
+          "source-table": ORDERS_ID,
+          expressions: {
+            [CUSTOM_COLUMN_LONG_NAME]: ["+", 1000, 1000],
+          },
+          filter: ["<", ["expression", CUSTOM_COLUMN_LONG_NAME, null], 1000000],
+          aggregation: [["avg", ["expression", CUSTOM_COLUMN_LONG_NAME, null]]],
+          breakout: [["expression", CUSTOM_COLUMN_LONG_NAME, null]],
+          "order-by": [["asc", ["expression", CUSTOM_COLUMN_LONG_NAME, null]]],
+        },
+      },
+      { visitQuestion: true },
+    );
+    openNotebook();
+
+    verifyNotebookQuery("Orders", [
+      {
+        expressions: [CUSTOM_COLUMN_LONG_NAME],
+        filters: [`${CUSTOM_COLUMN_LONG_NAME} is less than 1000000`],
+        aggregations: [`Average of ${CUSTOM_COLUMN_LONG_NAME}`],
+        breakouts: [CUSTOM_COLUMN_LONG_NAME],
+        sort: [{ column: CUSTOM_COLUMN_LONG_NAME, order: "asc" }],
+      },
+    ]);
+
+    cy.findAllByTestId("notebook-cell-item")
+      .filter(`:contains(${CUSTOM_COLUMN_LONG_NAME})`)
+      .then(items => {
+        for (let index = 0; index < items.length; ++index) {
+          cy.wrap(items[index]).within(() => {
+            assertRemoveClauseIconSize();
+          });
+        }
+      });
+
+    function assertRemoveClauseIconSize() {
+      cy.findByLabelText("close icon").invoke("outerWidth").should("eq", 16);
+      cy.findByLabelText("close icon").invoke("outerHeight").should("eq", 16);
+    }
+  });
 });
 
 function assertTableRowCount(expectedCount) {
diff --git a/frontend/src/metabase/querying/notebook/components/ClauseStep/ClauseStep.module.css b/frontend/src/metabase/querying/notebook/components/ClauseStep/ClauseStep.module.css
new file mode 100644
index 00000000000..b11bdb29d0b
--- /dev/null
+++ b/frontend/src/metabase/querying/notebook/components/ClauseStep/ClauseStep.module.css
@@ -0,0 +1,4 @@
+.closeIcon {
+  margin-left: 0.5rem;
+  flex: 0 0 auto;
+}
diff --git a/frontend/src/metabase/querying/notebook/components/ClauseStep/ClauseStep.tsx b/frontend/src/metabase/querying/notebook/components/ClauseStep/ClauseStep.tsx
index 9cc0ee3840b..9047cf47573 100644
--- a/frontend/src/metabase/querying/notebook/components/ClauseStep/ClauseStep.tsx
+++ b/frontend/src/metabase/querying/notebook/components/ClauseStep/ClauseStep.tsx
@@ -7,7 +7,6 @@ import { useMergedRef } from "@mantine/hooks";
 import type { ReactNode, Ref } from "react";
 import { forwardRef, useCallback } from "react";
 
-import CS from "metabase/css/core/index.css";
 import { Icon } from "metabase/ui";
 
 import {
@@ -17,6 +16,7 @@ import {
 } from "../NotebookCell";
 
 import { ClausePopover } from "./ClausePopover";
+import S from "./ClauseStep.module.css";
 
 type RenderItemOpts<T> = {
   item: T;
@@ -67,7 +67,7 @@ export const ClauseStep = <T,>({
         {renderName(item, index)}
         {hasRemoveButton && (
           <Icon
-            className={CS.ml1}
+            className={S.closeIcon}
             name="close"
             onClick={e => {
               e.stopPropagation();
-- 
GitLab