Add `TableInfoPopover` and add it to Table cards on the `/browse` route (#19322)

* add TableInfoPopover

* Finish TableInfo implementation

* Add TableInfoPopover to TableBrowser

* TableInfo & TableInfoPopover styling tweaks

* add a cypress test for the /browse popover

* TableInfo unit tests update

* fix smoketest test

* shorter delay

* Add loading state for table metadata

* Fix table fk link

* make everything typescript

* add more unit tests

* fix types

* fix e2e test

* styles cleanup

* review changes

Co-authored-by: Maz Ameli <maz@metabase.com>

frontend/src/metabase-lib/lib/metadata/Table.ts

@@ -15,6 +15,9 @@ import { memoize, createLookupByProperty } from "metabase-lib/lib/utils";
 /** This is the primary way people interact with tables */
 
 export default class Table extends Base {
+  id: number;
+  description?: string;
+
   hasSchema() {
     return (this.schema_name && this.db && this.db.schemas.length > 1) || false;
   }
@@ -116,6 +119,15 @@ export default class Table extends Base {
     return this.fieldsLookup();
   }
 
+  numFields(): number {
+    return this.fields?.length || 0;
+  }
+
+  connectedTables(): Table[] {
+    const fks = this.fks || [];
+    return fks.map(fk => new Table(fk.origin.table));
+  }
+
   /**
    * @private
    * @param {string} description

frontend/src/metabase-lib/lib/metadata/Table.unit.spec.ts

+import { PRODUCTS } from "__support__/sample_dataset_fixture";
+import Table from "./Table";
+
+describe("Table", () => {
+  const productsTable = new Table(PRODUCTS);
+
+  describe("numFields", () => {
+    it("should return the number of fields", () => {
+      expect(productsTable.numFields()).toBe(8);
+    });
+
+    it("should handle scenario where fields prop is missing", () => {
+      const table = new Table({});
+      expect(table.numFields()).toBe(0);
+    });
+  });
+
+  describe("connectedTables", () => {
+    it("should return a list of table instances connected to it via fk", () => {
+      const table = new Table({
+        fks: [
+          {
+            origin: { table: PRODUCTS },
+          },
+        ],
+      });
+
+      expect(table.connectedTables()).toEqual([productsTable]);
+    });
+  });
+});

frontend/src/metabase/browse/components/TableBrowser/TableBrowser.jsx

@@ -9,6 +9,8 @@ import Database from "metabase/entities/databases";
 import EntityItem from "metabase/components/EntityItem";
 import Icon from "metabase/components/Icon";
 import { Grid, GridItem } from "metabase/components/Grid";
+import TableInfoPopover from "metabase/components/MetadataInfo/TableInfoPopover";
+
 import { ANALYTICS_CONTEXT, ITEM_WIDTHS } from "../../constants";
 import BrowseHeader from "../BrowseHeader";
 import { TableActionLink, TableCard, TableLink } from "./TableBrowser.styled";
@@ -44,18 +46,22 @@ const TableBrowser = ({
       <Grid>
         {tables.map(table => (
           <GridItem key={table.id} width={ITEM_WIDTHS}>
-            <TableCard hoverable={isSyncCompleted(table)}>
-              <TableLink
-                to={isSyncCompleted(table) ? getTableUrl(table, metadata) : ""}
-                data-metabase-event={`${ANALYTICS_CONTEXT};Table Click`}
-              >
-                <TableBrowserItem
-                  table={table}
-                  dbId={dbId}
-                  xraysEnabled={xraysEnabled}
-                />
-              </TableLink>
-            </TableCard>
+            <TableInfoPopover tableId={table.id} placement="bottom-start">
+              <TableCard hoverable={isSyncCompleted(table)}>
+                <TableLink
+                  to={
+                    isSyncCompleted(table) ? getTableUrl(table, metadata) : ""
+                  }
+                  data-metabase-event={`${ANALYTICS_CONTEXT};Table Click`}
+                >
+                  <TableBrowserItem
+                    table={table}
+                    dbId={dbId}
+                    xraysEnabled={xraysEnabled}
+                  />
+                </TableLink>
+              </TableCard>
+            </TableInfoPopover>
           </GridItem>
         ))}
       </Grid>

frontend/src/metabase/browse/components/TableBrowser/TableBrowser.styled.jsx

@@ -2,6 +2,7 @@ import styled from "styled-components";
 import { space } from "metabase/styled-components/theme";
 import Card from "metabase/components/Card";
 import Link from "metabase/components/Link";
+import { forwardRefToInnerRef } from "metabase/styled-components/utils";
 
 export const TableLink = styled(Link)`
   display: block;
@@ -14,7 +15,7 @@ export const TableActionLink = styled(Link)`
   margin-left: ${space(1)};
 `;
 
-export const TableCard = styled(Card)`
+export const TableCard = forwardRefToInnerRef(styled(Card)`
   padding-left: ${space(1)};
   padding-right: ${space(1)};
 
@@ -25,4 +26,4 @@ export const TableCard = styled(Card)`
   &:hover ${TableActionLink} {
     visibility: visible;
   }
-`;
+`);

frontend/src/metabase/components/EntityItem.jsx

@@ -180,13 +180,6 @@ const EntityItem = ({
 
       <Flex ml="auto" pr={1} align="center" onClick={e => e.preventDefault()}>
         {buttons}
-        {!loading && item.description && (
-          <Icon
-            tooltip={item.description}
-            name="info"
-            className="ml1 text-medium"
-          />
-        )}
         {loading && <EntityItemSpinner size={24} borderWidth={3} />}
         <EntityItemMenu
           item={item}

frontend/src/metabase/components/MetadataInfo/DimensionInfoPopover/DimensionInfoPopover.tsx

@@ -6,6 +6,7 @@ import TippyPopver, {
   ITippyPopoverProps,
 } from "metabase/components/Popover/TippyPopover";
 import DimensionInfo from "metabase/components/MetadataInfo/DimensionInfo";
+import { isCypressActive } from "metabase/env";
 
 export const POPOVER_DELAY: [number, number] = [1000, 300];
 
@@ -23,7 +24,7 @@ type Props = { dimension: Dimension } & Pick<
 function DimensionInfoPopover({ dimension, children, placement }: Props) {
   return dimension ? (
     <TippyPopver
-      delay={POPOVER_DELAY}
+      delay={isCypressActive ? 0 : POPOVER_DELAY}
       interactive
       placement={placement || "left-start"}
       content={<DimensionInfo dimension={dimension} />}

frontend/src/metabase/components/MetadataInfo/MetadataInfo.styled.jsx → frontend/src/metabase/components/MetadataInfo/MetadataInfo.styled.tsx

@@ -3,15 +3,34 @@ import styled from "styled-components";
 import { space } from "metabase/styled-components/theme";
 import { color } from "metabase/lib/colors";
 import Icon from "metabase/components/Icon";
+import { isReducedMotionPreferred } from "metabase/lib/dom";
+import _LoadingSpinner from "metabase/components/LoadingSpinner";
 
-export const InfoContainer = styled.div`
+const TRANSITION_DURATION = () => (isReducedMotionPreferred() ? "0" : "0.25s");
+
+export const Container = styled.div`
+  position: relative;
   display: flex;
   flex-direction: column;
-  gap: ${space(2)};
-  padding: ${space(2)};
+  gap: ${space(1)};
   overflow: auto;
 `;
 
+export const AbsoluteContainer = styled.div`
+  position: absolute;
+  top: 0;
+  left: 0;
+  right: 0;
+  bottom: 0;
+  display: flex;
+  justify-content: center;
+  align-items: center;
+`;
+
+export const InfoContainer = styled(Container)`
+  padding: ${space(2)};
+`;
+
 export const Description = styled.div`
   font-size: 14px;
   white-space: pre-line;
@@ -33,7 +52,7 @@ export const LabelContainer = styled.div`
 `;
 
 export const Label = styled.span`
-  font-weight: 900;
+  font-weight: bold;
   font-size: 1em;
 `;
 
@@ -49,3 +68,28 @@ export const InvertedColorRelativeSizeIcon = styled(RelativeSizeIcon)`
   border-radius: ${space(0)};
   padding: ${space(0)};
 `;
+
+type FadeProps = {
+  visible?: boolean;
+};
+
+export const Fade = styled.div<FadeProps>`
+  width: 100%;
+  transition: opacity ${TRANSITION_DURATION} linear;
+  opacity: ${({ visible }) => (visible ? "1" : "0")};
+`;
+
+export const FadeAndSlide = styled.div<FadeProps>`
+  width: 100%;
+  transition: opacity ${TRANSITION_DURATION} linear,
+    transform ${TRANSITION_DURATION} linear;
+  opacity: ${({ visible }) => (visible ? "1" : "0")};
+`;
+
+export const LoadingSpinner = styled(_LoadingSpinner)`
+  display: flex;
+  flex-grow: 1;
+  align-self: center;
+  justify-content: center;
+  color: ${color("brand")};
+`;

frontend/src/metabase/components/MetadataInfo/TableInfo/ColumnCount.tsx

+import React from "react";
+import PropTypes from "prop-types";
+import { msgid, ngettext } from "ttag";
+
+import Table from "metabase-lib/lib/metadata/Table";
+
+import { Label, LabelContainer } from "../MetadataInfo.styled";
+
+ColumnCount.propTypes = {
+  table: PropTypes.instanceOf(Table).isRequired,
+};
+
+function ColumnCount({ table }: { table: Table }) {
+  const fieldCount = table.numFields();
+  return (
+    <LabelContainer color="text-dark">
+      <Label>
+        {ngettext(
+          msgid`${fieldCount} column`,
+          `${fieldCount} columns`,
+          fieldCount,
+        )}
+      </Label>
+    </LabelContainer>
+  );
+}
+
+export default ColumnCount;

frontend/src/metabase/components/MetadataInfo/TableInfo/ColumnCount.unit.spec.tsx

+import React from "react";
+import { render, screen } from "@testing-library/react";
+
+import Table from "metabase-lib/lib/metadata/Table";
+
+import ColumnCount from "./ColumnCount";
+
+function setup(table: Table) {
+  return render(<ColumnCount table={table} />);
+}
+
+describe("ColumnCount", () => {
+  it("should show a non-plural label for a table with a single field", () => {
+    const table = new Table({ fields: [{}] });
+    setup(table);
+
+    expect(screen.getByText("1 column")).toBeInTheDocument();
+  });
+
+  it("should show a plural label for a table with multiple fields", () => {
+    const table = new Table({ fields: [{}, {}] });
+    setup(table);
+
+    expect(screen.getByText("2 columns")).toBeInTheDocument();
+  });
+
+  it("should handle a scenario where a table has no fields property", () => {
+    const table = new Table({ id: 123, display_name: "Foo" });
+    setup(table);
+
+    expect(screen.getByText("0 columns")).toBeInTheDocument();
+  });
+});

frontend/src/metabase/components/MetadataInfo/TableInfo/ConnectedTables.styled.tsx

+import styled from "styled-components";
+
+import { color } from "metabase/lib/colors";
+
+import TableLabel from "../TableLabel/TableLabel";
+
+export const InteractiveTableLabel = styled(TableLabel)`
+  cursor: pointer;
+  color: ${color("text-light")};
+
+  &:hover {
+    color: ${color("brand")};
+  }
+`;

frontend/src/metabase/components/MetadataInfo/TableInfo/ConnectedTables.tsx

+import React from "react";
+import PropTypes from "prop-types";
+import { t } from "ttag";
+
+import Table from "metabase-lib/lib/metadata/Table";
+import Link from "metabase/components/Link";
+
+import { Label, LabelContainer, Container } from "../MetadataInfo.styled";
+import { InteractiveTableLabel } from "./ConnectedTables.styled";
+
+ConnectedTables.propTypes = {
+  table: PropTypes.instanceOf(Table).isRequired,
+};
+
+function ConnectedTables({ table }: { table: Table }) {
+  const fkTables = table.connectedTables();
+  return fkTables.length ? (
+    <Container>
+      <LabelContainer color="text-dark">
+        <Label>{t`Connected to these tables`}</Label>
+      </LabelContainer>
+      {fkTables.map(fkTable => {
+        return (
+          <Link key={fkTable.id} to={fkTable.newQuestion().getUrl()}>
+            <InteractiveTableLabel table={fkTable} />
+          </Link>
+        );
+      })}
+    </Container>
+  ) : null;
+}
+
+export default ConnectedTables;

frontend/src/metabase/components/MetadataInfo/TableInfo/ConnectedTables.unit.spec.tsx

+import React from "react";
+import { render, screen } from "@testing-library/react";
+
+import Table from "metabase-lib/lib/metadata/Table";
+
+import ConnectedTables from "./ConnectedTables";
+
+function setup(table: Table) {
+  return render(<ConnectedTables table={table} />);
+}
+
+describe("ConnectedTables", () => {
+  it("should show nothing when the table has no fks", () => {
+    const table = new Table();
+    const { container } = setup(table);
+
+    expect(container.firstChild).toBeNull();
+  });
+
+  it("should show a label for each connected table", () => {
+    const table = new Table({
+      id: 1,
+      fks: [
+        {
+          origin: {
+            table: {
+              id: 2,
+              display_name: "Foo",
+            },
+          },
+        },
+        {
+          origin: {
+            table: {
+              id: 3,
+              display_name: "Bar",
+            },
+          },
+        },
+      ],
+    });
+
+    setup(table);
+
+    expect(screen.getByText("Foo")).toBeInTheDocument();
+    expect(screen.getByText("Bar")).toBeInTheDocument();
+  });
+});

frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.jsx

-import React from "react";
-import PropTypes from "prop-types";
-import { t } from "ttag";
-
-import Table from "metabase-lib/lib/metadata/Table";
-import TableLabel from "metabase/components/MetadataInfo/TableLabel";
-
-import {
-  InfoContainer,
-  Description,
-  EmptyDescription,
-} from "../MetadataInfo.styled";
-
-TableInfo.propTypes = {
-  className: PropTypes.string,
-  table: PropTypes.instanceOf(Table).isRequired,
-};
-
-function TableInfo({ className, table }) {
-  const description = table.description;
-
-  return (
-    <InfoContainer className={className}>
-      {description ? (
-        <Description>{description}</Description>
-      ) : (
-        <EmptyDescription>{t`No description`}</EmptyDescription>
-      )}
-      <TableLabel table={table} />
-    </InfoContainer>
-  );
-}
-
-export default TableInfo;

frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.tsx

+import React, { useState, useEffect } from "react";
+import PropTypes from "prop-types";
+import { t } from "ttag";
+import { connect } from "react-redux";
+
+import { useAsyncFunction } from "metabase/hooks/use-async-function";
+import Tables from "metabase/entities/tables";
+import Table from "metabase-lib/lib/metadata/Table";
+
+import {
+  InfoContainer,
+  Description,
+  EmptyDescription,
+  LoadingSpinner,
+  AbsoluteContainer,
+  Fade,
+  Container,
+} from "../MetadataInfo.styled";
+import ColumnCount from "./ColumnCount";
+import ConnectedTables from "./ConnectedTables";
+
+type OwnProps = {
+  className?: string;
+  tableId: number;
+};
+
+const mapStateToProps = (state: any, props: OwnProps) => {
+  return {
+    table: Tables.selectors.getObject(state, {
+      entityId: props.tableId,
+    }) as Table,
+  };
+};
+
+const mapDispatchToProps: {
+  fetchForeignKeys: (args: { id: number }) => Promise<any>;
+  fetchMetadata: (args: { id: number }) => Promise<any>;
+} = {
+  fetchForeignKeys: Tables.actions.fetchForeignKeys,
+  fetchMetadata: Tables.actions.fetchMetadata,
+};
+
+TableInfo.propTypes = {
+  className: PropTypes.string,
+  tableId: PropTypes.number.isRequired,
+  table: PropTypes.instanceOf(Table).isRequired,
+  fetchForeignKeys: PropTypes.func.isRequired,
+  fetchMetadata: PropTypes.func.isRequired,
+};
+
+type AllProps = OwnProps &
+  ReturnType<typeof mapStateToProps> &
+  typeof mapDispatchToProps;
+
+function useDependentTableMetadata({
+  tableId,
+  table,
+  fetchForeignKeys,
+  fetchMetadata,
+}: Omit<AllProps, "className">) {
+  const shouldFetchMetadata = !table.numFields();
+  const [hasFetchedMetadata, setHasFetchedMetadata] = useState(
+    !shouldFetchMetadata,
+  );
+  const fetchDependentData = useAsyncFunction(() => {
+    return Promise.all([
+      fetchForeignKeys({ id: tableId }),
+      fetchMetadata({ id: tableId }),
+    ]);
+  }, [tableId, fetchForeignKeys, fetchMetadata]);
+
+  useEffect(() => {
+    if (shouldFetchMetadata) {
+      fetchDependentData().then(() => {
+        setHasFetchedMetadata(true);
+      });
+    }
+  }, [fetchDependentData, shouldFetchMetadata]);
+
+  return hasFetchedMetadata;
+}
+
+export function TableInfo({
+  className,
+  tableId,
+  table,
+  fetchForeignKeys,
+  fetchMetadata,
+}: AllProps): JSX.Element {
+  const description = table.description;
+  const hasFetchedMetadata = useDependentTableMetadata({
+    tableId,
+    table,
+    fetchForeignKeys,
+    fetchMetadata,
+  });
+
+  return (
+    <InfoContainer className={className}>
+      {description ? (
+        <Description>{description}</Description>
+      ) : (
+        <EmptyDescription>{t`No description`}</EmptyDescription>
+      )}
+      <Container>
+        <Fade visible={!hasFetchedMetadata}>
+          <AbsoluteContainer>
+            <LoadingSpinner size={24} />
+          </AbsoluteContainer>
+        </Fade>
+        <Fade visible={hasFetchedMetadata}>
+          <ColumnCount table={table} />
+        </Fade>
+        <Fade visible={hasFetchedMetadata}>
+          <ConnectedTables table={table} />
+        </Fade>
+      </Container>
+    </InfoContainer>
+  );
+}
+
+export default connect(mapStateToProps, mapDispatchToProps)(TableInfo);

frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.unit.spec.js

-import React from "react";
-import { render, screen } from "@testing-library/react";
-
-import { PRODUCTS } from "__support__/sample_dataset_fixture";
-import Table from "metabase-lib/lib/metadata/Table";
-
-import TableInfo from "./TableInfo";
-
-const table = new Table(PRODUCTS);
-const tableNoDescription = new Table({ id: 123, display_name: "Foo" });
-
-describe("TableInfo", () => {
-  beforeEach(() => {
-    render(<TableInfo table={table} />);
-  });
-
-  it("should display the given table's name", () => {
-    expect(screen.getByText(PRODUCTS.display_name)).toBeInTheDocument();
-  });
-
-  it("should display the given table's description", () => {
-    expect(screen.getByText(PRODUCTS.description)).toBeInTheDocument();
-  });
-
-  it("should display a placeholder if table has no description", () => {
-    render(<TableInfo table={tableNoDescription} />);
-
-    expect(screen.getByText("No description")).toBeInTheDocument();
-  });
-});

frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.unit.spec.tsx

+import React from "react";
+import { render, screen } from "@testing-library/react";
+
+import { PRODUCTS } from "__support__/sample_dataset_fixture";
+import Table from "metabase-lib/lib/metadata/Table";
+
+import { TableInfo } from "./TableInfo";
+
+const productsTableWithoutMetadata = new Table({
+  ...PRODUCTS,
+  fields: undefined,
+  fks: undefined,
+});
+
+const productsTable = new Table({
+  ...PRODUCTS,
+  fields: [1, 2, 3],
+  fks: [
+    {
+      origin: {
+        table: {
+          id: 111,
+          db_id: 222,
+          display_name: "Connected Table",
+        },
+      },
+    },
+  ],
+});
+const tableWithoutDescription = new Table({
+  id: 123,
+  display_name: "Foo",
+  fields: [],
+});
+
+const fetchForeignKeys = jest.fn();
+const fetchMetadata = jest.fn();
+
+function setup(table: Table) {
+  fetchForeignKeys.mockReset();
+  fetchMetadata.mockReset();
+
+  return render(
+    <TableInfo
+      tableId={table.id}
+      table={table}
+      fetchForeignKeys={fetchForeignKeys}
+      fetchMetadata={fetchMetadata}
+    />,
+  );
+}
+
+describe("TableInfo", () => {
+  it("should send requests fetching table metadata", () => {
+    setup(productsTableWithoutMetadata);
+
+    expect(fetchForeignKeys).toHaveBeenCalledWith({
+      id: PRODUCTS.id,
+    });
+    expect(fetchMetadata).toHaveBeenCalledWith({
+      id: PRODUCTS.id,
+    });
+  });
+
+  it("should not send requests fetching table metadata when metadata is already present", () => {
+    setup(productsTable);
+
+    expect(fetchForeignKeys).not.toHaveBeenCalled();
+    expect(fetchMetadata).not.toHaveBeenCalled();
+  });
+
+  it("should display a placeholder if table has no description", async () => {
+    setup(tableWithoutDescription);
+
+    expect(await screen.findByText("No description")).toBeInTheDocument();
+  });
+
+  describe("after metadata has been fetched", () => {
+    beforeEach(() => {
+      setup(productsTable);
+    });
+
+    it("should display the given table's description", () => {
+      expect(screen.getByText(PRODUCTS.description)).toBeInTheDocument();
+    });
+
+    it("should show a count of columns on the table", () => {
+      expect(screen.getByText("3 columns")).toBeInTheDocument();
+    });
+
+    it("should list connected tables", () => {
+      expect(screen.getByText("Connected Table")).toBeInTheDocument();
+    });
+  });
+});

frontend/src/metabase/components/MetadataInfo/TableInfoPopover/TableInfoPopover.styled.tsx

+import styled from "styled-components";
+
+import TableInfo from "metabase/components/MetadataInfo/TableInfo";
+
+type TableInfoProps = {
+  tableId: number;
+};
+
+export const WidthBoundTableInfo = styled(TableInfo)<TableInfoProps>`
+  width: 300px;
+  font-size: 14px;
+`;

frontend/src/metabase/components/MetadataInfo/TableInfoPopover/TableInfoPopover.tsx

+import React from "react";
+import PropTypes from "prop-types";
+
+import TippyPopver, {
+  ITippyPopoverProps,
+} from "metabase/components/Popover/TippyPopover";
+
+import { WidthBoundTableInfo } from "./TableInfoPopover.styled";
+
+export const POPOVER_DELAY: [number, number] = [500, 300];
+
+const propTypes = {
+  tableId: PropTypes.number.isRequired,
+  children: PropTypes.node,
+  placement: PropTypes.string,
+};
+
+type Props = { tableId: number } & Pick<
+  ITippyPopoverProps,
+  "children" | "placement"
+>;
+
+function TableInfoPopover({ tableId, children, placement }: Props) {
+  placement = placement || "left-start";
+
+  return tableId != null ? (
+    <TippyPopver
+      interactive
+      delay={POPOVER_DELAY}
+      placement={placement}
+      content={<WidthBoundTableInfo tableId={tableId} />}
+    >
+      {children}
+    </TippyPopver>
+  ) : (
+    children
+  );
+}
+
+TableInfoPopover.propTypes = propTypes;
+
+export default TableInfoPopover;

frontend/src/metabase/components/MetadataInfo/TableInfoPopover/index.ts

+export { default } from "./TableInfoPopover";