From aec41f458b5c5122e9361ebdcb7d972906ca7f33 Mon Sep 17 00:00:00 2001
From: Raphael Krut-Landau <raphael.kl@gmail.com>
Date: Fri, 10 May 2024 12:03:28 -0400
Subject: [PATCH] squash (#42111)

---
 .../browse/components/BrowseModels.tsx        |  4 +--
 .../components/BrowseModels.unit.spec.tsx     | 13 ++++++----
 ...ollectionBreadcrumbsWithTooltip.styled.tsx | 10 ++++++-
 .../CollectionBreadcrumbsWithTooltip.tsx      | 19 +++++++++++---
 .../browse/components/ModelsTable.styled.tsx  | 10 +++++++
 .../browse/components/ModelsTable.tsx         | 26 +++++++++++++++----
 .../ui/components/icons/Icon/Icon.tsx         |  7 +++--
 7 files changed, 68 insertions(+), 21 deletions(-)
 create mode 100644 frontend/src/metabase/browse/components/ModelsTable.styled.tsx

diff --git a/frontend/src/metabase/browse/components/BrowseModels.tsx b/frontend/src/metabase/browse/components/BrowseModels.tsx
index 149690171ef..c8c73ed6276 100644
--- a/frontend/src/metabase/browse/components/BrowseModels.tsx
+++ b/frontend/src/metabase/browse/components/BrowseModels.tsx
@@ -9,7 +9,8 @@ import { PLUGIN_CONTENT_VERIFICATION } from "metabase/plugins";
 import { Box, Flex, Group, Icon, Stack, Title } from "metabase/ui";
 import type { ModelResult, SearchRequest } from "metabase-types/api";
 
-import { filterModels, type ActualModelFilters } from "../utils";
+import type { ActualModelFilters } from "../utils";
+import { filterModels } from "../utils";
 
 import {
   BrowseContainer,
@@ -72,7 +73,6 @@ export const BrowseModelsBody = ({
     model_ancestors: true,
     filter_items_in_personal_collection: "exclude",
   };
-
   const { data, error, isLoading } = useSearchQuery(query);
 
   const models = useMemo(() => {
diff --git a/frontend/src/metabase/browse/components/BrowseModels.unit.spec.tsx b/frontend/src/metabase/browse/components/BrowseModels.unit.spec.tsx
index e2309778f6c..41aa6ce3f88 100644
--- a/frontend/src/metabase/browse/components/BrowseModels.unit.spec.tsx
+++ b/frontend/src/metabase/browse/components/BrowseModels.unit.spec.tsx
@@ -13,7 +13,7 @@ import { createMockSetupState } from "metabase-types/store/mocks";
 
 import { BrowseModels } from "./BrowseModels";
 
-const renderBrowseModels = (modelCount: number) => {
+const setup = (modelCount: number) => {
   const models = mockModels.slice(0, modelCount);
   setupSearchEndpoints(models);
   setupSettingsEndpoints([]);
@@ -264,20 +264,23 @@ const mockModels: SearchResult[] = [
 
 describe("BrowseModels", () => {
   it("displays a 'no models' message in the Models tab when no models exist", async () => {
-    renderBrowseModels(0);
+    setup(0);
     expect(await screen.findByText("No models here yet")).toBeInTheDocument();
   });
 
   it("displays the Our Analytics collection if it has a model", async () => {
-    renderBrowseModels(25);
-    await screen.findAllByText("Our analytics");
+    setup(25);
+    expect(await screen.findByRole("table")).toBeInTheDocument();
+    expect(
+      await screen.findAllByTestId("path-for-collection: Our analytics"),
+    ).toHaveLength(2);
     expect(await screen.findByText("Model 20")).toBeInTheDocument();
     expect(await screen.findByText("Model 21")).toBeInTheDocument();
     expect(await screen.findByText("Model 22")).toBeInTheDocument();
   });
 
   it("displays collection breadcrumbs", async () => {
-    renderBrowseModels(25);
+    setup(25);
     expect(await screen.findByText("Model 1")).toBeInTheDocument();
     expect(
       await screen.findAllByTestId("breadcrumbs-for-collection: Alpha"),
diff --git a/frontend/src/metabase/browse/components/CollectionBreadcrumbsWithTooltip.styled.tsx b/frontend/src/metabase/browse/components/CollectionBreadcrumbsWithTooltip.styled.tsx
index eabe069f49b..951849fb0f8 100644
--- a/frontend/src/metabase/browse/components/CollectionBreadcrumbsWithTooltip.styled.tsx
+++ b/frontend/src/metabase/browse/components/CollectionBreadcrumbsWithTooltip.styled.tsx
@@ -4,7 +4,7 @@ import type { AnchorHTMLAttributes } from "react";
 import { ResponsiveChild } from "metabase/components/ResponsiveContainer/ResponsiveContainer";
 import { color } from "metabase/lib/colors";
 import type { AnchorProps } from "metabase/ui";
-import { Anchor } from "metabase/ui";
+import { Anchor, FixedSizeIcon, Group } from "metabase/ui";
 
 import type { RefProp } from "./types";
 
@@ -54,3 +54,11 @@ export const CollectionBreadcrumbsWrapper = styled(ResponsiveChild)`
     `;
   }}
 `;
+
+export const BreadcrumbGroup = styled(Group)`
+  flex-flow: row nowrap;
+`;
+
+export const CollectionsIcon = styled(FixedSizeIcon)`
+  margin-inline-end: 0.5rem;
+`;
diff --git a/frontend/src/metabase/browse/components/CollectionBreadcrumbsWithTooltip.tsx b/frontend/src/metabase/browse/components/CollectionBreadcrumbsWithTooltip.tsx
index 07d3e5338de..ff679198b37 100644
--- a/frontend/src/metabase/browse/components/CollectionBreadcrumbsWithTooltip.tsx
+++ b/frontend/src/metabase/browse/components/CollectionBreadcrumbsWithTooltip.tsx
@@ -7,14 +7,16 @@ import { useAreAnyTruncated } from "metabase/hooks/use-is-truncated";
 import resizeObserver from "metabase/lib/resize-observer";
 import * as Urls from "metabase/lib/urls";
 import type { FlexProps } from "metabase/ui";
-import { FixedSizeIcon, Flex, Group, Text, Tooltip } from "metabase/ui";
+import { Flex, Text, Tooltip } from "metabase/ui";
 import type { CollectionEssentials } from "metabase-types/api";
 
 import { getCollectionName } from "../utils";
 
 import {
   Breadcrumb,
+  BreadcrumbGroup,
   CollectionBreadcrumbsWrapper,
+  CollectionsIcon,
 } from "./CollectionBreadcrumbsWithTooltip.styled";
 import { pathSeparatorChar } from "./constants";
 import type { RefProp } from "./types";
@@ -83,11 +85,20 @@ export const CollectionBreadcrumbsWithTooltip = ({
         w="auto"
       >
         <Flex align="center" w="100%" lh="1" style={{ flexFlow: "row nowrap" }}>
-          <FixedSizeIcon name="folder" style={{ marginInlineEnd: ".5rem" }} />
+          <CollectionsIcon
+            name="folder"
+            // Stopping propagation so that the parent <tr>'s onclick won't fire
+            onClick={(e: React.MouseEvent) => e.stopPropagation()}
+          />
           {shownCollections.map((collection, index) => {
             const key = `collection${collection.id}`;
             return (
-              <Group spacing={0} style={{ flexFlow: "row nowrap" }} key={key}>
+              <BreadcrumbGroup
+                spacing={0}
+                key={key}
+                // Stopping propagation so that the parent <tr>'s onclick won't fire
+                onClick={(e: React.MouseEvent) => e.stopPropagation()}
+              >
                 {index > 0 && <PathSeparator />}
                 <CollectionBreadcrumbsWrapper
                   containerName={containerName}
@@ -115,7 +126,7 @@ export const CollectionBreadcrumbsWithTooltip = ({
                     {getCollectionName(collection)}
                   </Breadcrumb>
                 </CollectionBreadcrumbsWrapper>
-              </Group>
+              </BreadcrumbGroup>
             );
           })}
         </Flex>
diff --git a/frontend/src/metabase/browse/components/ModelsTable.styled.tsx b/frontend/src/metabase/browse/components/ModelsTable.styled.tsx
new file mode 100644
index 00000000000..ca088cd201f
--- /dev/null
+++ b/frontend/src/metabase/browse/components/ModelsTable.styled.tsx
@@ -0,0 +1,10 @@
+import styled from "@emotion/styled";
+
+import { color } from "metabase/lib/colors";
+
+export const ModelTableRow = styled.tr`
+  cursor: pointer;
+  :outline {
+    outline: 2px solid ${color("brand")};
+  }
+`;
diff --git a/frontend/src/metabase/browse/components/ModelsTable.tsx b/frontend/src/metabase/browse/components/ModelsTable.tsx
index 4bea8f58d7e..77d82620e68 100644
--- a/frontend/src/metabase/browse/components/ModelsTable.tsx
+++ b/frontend/src/metabase/browse/components/ModelsTable.tsx
@@ -1,4 +1,5 @@
 import { useState } from "react";
+import { push } from "react-router-redux";
 import { t } from "ttag";
 
 import EntityItem from "metabase/components/EntityItem";
@@ -7,7 +8,6 @@ import {
   type SortingOptions,
 } from "metabase/components/ItemsTable/BaseItemsTable";
 import {
-  ColumnHeader,
   ItemCell,
   ItemLink,
   ItemNameCell,
@@ -18,7 +18,7 @@ import {
 import { Columns, SortDirection } from "metabase/components/ItemsTable/Columns";
 import type { ResponsiveProps } from "metabase/components/ItemsTable/utils";
 import { color } from "metabase/lib/colors";
-import { useSelector } from "metabase/lib/redux";
+import { useDispatch, useSelector } from "metabase/lib/redux";
 import * as Urls from "metabase/lib/urls";
 import { getLocale } from "metabase/setup/selectors";
 import { Icon, type IconProps } from "metabase/ui";
@@ -29,6 +29,7 @@ import { getCollectionName, getIcon } from "../utils";
 
 import { CollectionBreadcrumbsWithTooltip } from "./CollectionBreadcrumbsWithTooltip";
 import { EllipsifiedWithMarkdown } from "./EllipsifiedWithMarkdown";
+import { ModelTableRow } from "./ModelsTable.styled";
 import { getModelDescription, sortModels } from "./utils";
 
 export interface ModelsTableProps {
@@ -76,7 +77,9 @@ export const ModelsTable = ({ models }: ModelsTableProps) => {
             sortingOptions={sortingOptions}
             onSortingOptionsChange={setSortingOptions}
           />
-          <ColumnHeader {...descriptionProps}>{t`Description`}</ColumnHeader>
+          <SortableColumnHeader name="description" {...descriptionProps}>
+            {t`Description`}
+          </SortableColumnHeader>
           <SortableColumnHeader
             name="collection"
             sortingOptions={sortingOptions}
@@ -100,9 +103,22 @@ export const ModelsTable = ({ models }: ModelsTableProps) => {
 const TBodyRow = ({ model }: { model: ModelResult }) => {
   const icon = getIcon(model);
   const containerName = `collections-path-for-${model.id}`;
+  const dispatch = useDispatch();
+  const { id, name } = model;
 
   return (
-    <tr>
+    <ModelTableRow
+      onClick={(e: React.MouseEvent) => {
+        const url = Urls.model({ id, name });
+        if ((e.ctrlKey || e.metaKey) && e.button === 0) {
+          window.open(url, "_blank");
+        } else {
+          dispatch(push(url));
+        }
+      }}
+      tabIndex={0}
+      key={model.id}
+    >
       {/* Name */}
       <NameCell
         model={model}
@@ -138,7 +154,7 @@ const TBodyRow = ({ model }: { model: ModelResult }) => {
 
       {/* Adds a border-radius to the table */}
       <Columns.RightEdge.Cell />
-    </tr>
+    </ModelTableRow>
   );
 };
 
diff --git a/frontend/src/metabase/ui/components/icons/Icon/Icon.tsx b/frontend/src/metabase/ui/components/icons/Icon/Icon.tsx
index 403779e338a..4b86fdaff4a 100644
--- a/frontend/src/metabase/ui/components/icons/Icon/Icon.tsx
+++ b/frontend/src/metabase/ui/components/icons/Icon/Icon.tsx
@@ -43,8 +43,7 @@ export const Icon = forwardRef<SVGSVGElement, IconProps>(function Icon(
   return tooltip ? <Tooltip label={tooltip}>{icon}</Tooltip> : icon;
 });
 
-/** An icon that does not shrink when the viewport gets narrower **/
-export const FixedSizeIcon = styled(Icon)<{ size?: number }>`
-  min-width: ${({ size }) => size ?? 16}px;
-  min-height: ${({ size }) => size ?? 16}px;
+/** An icon that does not shrink when its container is too narrow **/
+export const FixedSizeIcon = styled(Icon)`
+  flex-shrink: 0;
 `;
-- 
GitLab