Move pinning items in collection to menu (#23408)

frontend/src/metabase/components/EntityItem.jsx

@@ -8,7 +8,6 @@ import Swapper from "metabase/components/Swapper";
 import CheckBox from "metabase/core/components/CheckBox";
 import Ellipsified from "metabase/core/components/Ellipsified";
 import Icon from "metabase/components/Icon";
-import Tooltip from "metabase/components/Tooltip";
 import { isItemPinned } from "metabase/collections/utils";
 
 import {
@@ -17,7 +16,6 @@ import {
   EntityItemSpinner,
   EntityItemWrapper,
   EntityMenuContainer,
-  PinButton,
 } from "./EntityItem.styled";
 
 function EntityIconCheckBox({
@@ -97,12 +95,11 @@ function EntityItemMenu({
   analyticsContext,
 }) {
   const isPinned = isItemPinned(item);
-  const showPinnedAction = onPin && isPinned;
-  const showUnpinnedAction = onPin && !isPinned;
+
   const actions = useMemo(
     () =>
       [
-        showPinnedAction && {
+        onPin && {
           title: isPinned ? t`Unpin` : t`Pin this`,
           icon: "pin",
           action: onPin,
@@ -148,7 +145,6 @@ function EntityItemMenu({
       isBookmarked,
       isPreviewShown,
       isPreviewAvailable,
-      showPinnedAction,
       onPin,
       onMove,
       onCopy,
@@ -163,11 +159,6 @@ function EntityItemMenu({
   }
   return (
     <EntityMenuContainer align="center">
-      {showUnpinnedAction && (
-        <Tooltip tooltip={t`Pin this`}>
-          <PinButton icon="pin" onClick={onPin} />
-        </Tooltip>
-      )}
       <EntityMenu
         className={cx(className, "hover-child")}
         triggerIcon="ellipsis"

frontend/src/metabase/components/EntityItem.styled.jsx

@@ -4,7 +4,6 @@ import { color, darken } from "metabase/lib/colors";
 
 import IconButtonWrapper from "metabase/components/IconButtonWrapper";
 import LoadingSpinner from "metabase/components/LoadingSpinner";
-import Button from "metabase/core/components/Button";
 
 function getPinnedForeground(disabled) {
   return disabled ? darken(color("border"), 0.38) : color("accent4");
@@ -65,15 +64,6 @@ export const EntityMenuContainer = styled.div`
   color: ${color("text-medium")};
 `;
 
-export const PinButton = styled(Button)`
-  color: ${color("text-medium")};
-  border: none;
-
-  &:hover {
-    color: ${color("brand")};
-  }
-`;
-
 export const EntityItemActions = styled.div`
   display: flex;
   align-items: center;

frontend/test/metabase/scenarios/collections/collection-pinned-overview.cy.spec.js

-import { restore, popover } from "__support__/e2e/cypress";
+import { popover, restore } from "__support__/e2e/cypress";
 
 const DASHBOARD_ITEM_NAME = "Orders in a dashboard";
 const CARD_ITEM_NAME = "Orders, Count";
@@ -24,11 +24,7 @@ describe("scenarios > collection pinned items overview", () => {
     cy.wait("@pinnedItemsGET");
 
     // pin a dashboard
-    cy.findByText(DASHBOARD_ITEM_NAME)
-      .closest("tr")
-      .within(() => {
-        cy.icon("pin").click();
-      });
+    pinItem(DASHBOARD_ITEM_NAME);
     cy.wait("@pinnedItemsGET");
 
     // ensure the dashboard card is showing in the pinned section
@@ -44,11 +40,7 @@ describe("scenarios > collection pinned items overview", () => {
     cy.wait("@pinnedItemsGET");
 
     // pin a card
-    cy.findByText(CARD_ITEM_NAME)
-      .closest("tr")
-      .within(() => {
-        cy.icon("pin").click();
-      });
+    pinItem(CARD_ITEM_NAME);
     cy.wait(["@pinnedItemsGET", "@cardQuery"]);
 
     // ensure the card visualization is showing in the pinned section
@@ -62,11 +54,7 @@ describe("scenarios > collection pinned items overview", () => {
     cy.wait(["@pinnedItemsGET", "@cardQuery"]);
 
     // pin a model
-    cy.findByText(MODE_ITEM_NAME)
-      .closest("tr")
-      .within(() => {
-        cy.icon("pin").click();
-      });
+    pinItem(MODE_ITEM_NAME);
     cy.wait("@pinnedItemsGET");
     // ensure the model card is showing in the pinned section
     cy.findByTestId("pinned-items").within(() => {
@@ -135,3 +123,11 @@ describe("scenarios > collection pinned items overview", () => {
     });
   });
 });
+
+const pinItem = name => {
+  cy.findByText(name)
+    .closest("tr")
+    .within(() => cy.icon("ellipsis").click());
+
+  popover().within(() => cy.icon("pin").click());
+};

frontend/test/metabase/scenarios/collections/permissions.cy.spec.js

@@ -431,12 +431,10 @@ describe("collection permissions", () => {
 });
 
 function openEllipsisMenuFor(item, index = 0) {
-  return cy
-    .findAllByText(item)
+  cy.findAllByText(item)
     .eq(index)
     .closest("tr")
-    .find(".Icon-ellipsis")
-    .click({ force: true });
+    .within(() => cy.icon("ellipsis").click());
 }
 
 function clickButton(name) {
@@ -446,11 +444,8 @@ function clickButton(name) {
 }
 
 function pinItem(item) {
-  cy.findAllByText(item)
-    .closest("tr")
-    .within(() => {
-      cy.icon("pin").click();
-    });
+  openEllipsisMenuFor(item);
+  popover().within(() => cy.icon("pin").click());
 }
 
 function exposeChildrenFor(collectionName) {

frontend/test/metabase/scenarios/onboarding/home/homepage.cy.spec.js

-import { restore, visitDashboard } from "__support__/e2e/cypress";
+import { popover, restore, visitDashboard } from "__support__/e2e/cypress";
 
 describe("scenarios > home > homepage", () => {
   beforeEach(() => {
@@ -104,7 +104,7 @@ describe("scenarios > home > homepage", () => {
 
       cy.visit("/collection/root");
       cy.wait("@getCollectionItems");
-      getQuestionRow("Orders, Count").within(() => cy.icon("pin").click());
+      pinItem("Orders, Count");
       cy.wait("@getCollectionItems");
       cy.wait("@getQuestionQuery");
 
@@ -116,8 +116,12 @@ describe("scenarios > home > homepage", () => {
   });
 });
 
-const getQuestionRow = name => {
-  return cy.findByText(name).closest("tr");
+const pinItem = name => {
+  cy.findByText(name)
+    .closest("tr")
+    .within(() => cy.icon("ellipsis").click());
+
+  popover().within(() => cy.icon("pin").click());
 };
 
 const getXrayCandidates = () => [

frontend/test/metabase/scenarios/organization/bookmarks-collection.cy.spec.js

@@ -171,11 +171,10 @@ function bookmarkThenArchive(name) {
 }
 
 function pin(name) {
-  cy.get("td")
-    .contains(name)
-    .closest("tr")
-    .find(".Icon-pin")
-    .click();
+  openEllipsisMenuFor(name);
+  popover().within(() => {
+    cy.findByText("Pin this").click();
+  });
 }
 
 function archive(name) {