Markdown support in pinned items - improvements (#31851)

* Extract getIsTruncated

* Extract useIsTruncated

* Extract useIsTruncated to a separate file

* Show tooltip in MarkdownPreview only if the text is truncated

* Update hr colors in markdown and in markdown tooltip

* Add a test to show tooltip only when ellipsis is applied

* Use Element instead of HTMLElement in resize-observer and useIsTruncated
- Use clientWidth instead of offsetWidth to detect horizontal ellipsis (offsetWidth comes from HTMLElement.prototype)

* Introduce "dark" prop to Markdown and make tooltips use is consistently throughout the code

* Use first child instead element of a div query selector to hopefully improve code safety

frontend/src/metabase/collections/components/BaseTableItem.jsx

@@ -129,7 +129,7 @@ export function BaseTableItem({
                 name="info"
                 size={16}
                 tooltip={
-                  <Markdown disallowHeading unstyleLinks>
+                  <Markdown dark disallowHeading unstyleLinks>
                     {item.description}
                   </Markdown>
                 }

frontend/src/metabase/collections/components/PinnedItemCard/PinnedItemCard.unit.spec.js

@@ -130,6 +130,29 @@ describe("PinnedItemCard", () => {
   });
 
   describe("description", () => {
+    const originalScrollWidth = Object.getOwnPropertyDescriptor(
+      HTMLElement.prototype,
+      "scrollWidth",
+    );
+
+    beforeAll(() => {
+      // emulate ellipsis
+      Object.defineProperty(HTMLElement.prototype, "scrollWidth", {
+        configurable: true,
+        value: 100,
+      });
+    });
+
+    afterAll(() => {
+      if (originalScrollWidth) {
+        Object.defineProperty(
+          HTMLElement.prototype,
+          "scrollWidth",
+          originalScrollWidth,
+        );
+      }
+    });
+
     it("should render description markdown as plain text", () => {
       setup({ item: getCollectionItem({ description: MARKDOWN }) });
 

frontend/src/metabase/core/components/Ellipsified/Ellipsified.tsx

-import {
-  CSSProperties,
-  ReactNode,
-  useLayoutEffect,
-  useRef,
-  useState,
-} from "react";
+import { CSSProperties, ReactNode } from "react";
 // eslint-disable-next-line import/named
 import { Placement } from "tippy.js";
 
 import Tooltip from "metabase/core/components/Tooltip";
-import resizeObserver from "metabase/lib/resize-observer";
+import { useIsTruncated } from "metabase/hooks/use-is-truncated";
+
 import { EllipsifiedRoot } from "./Ellipsified.styled";
 
 interface EllipsifiedProps {
@@ -37,26 +32,7 @@ const Ellipsified = ({
   placement = "top",
   "data-testid": dataTestId,
 }: EllipsifiedProps) => {
-  const [isTruncated, setIsTruncated] = useState(false);
-  const rootRef = useRef<HTMLDivElement | null>(null);
-
-  useLayoutEffect(() => {
-    const element = rootRef.current;
-    if (!element) {
-      return;
-    }
-    const handleResize = () => {
-      const isTruncated =
-        element.scrollHeight > element.clientHeight ||
-        element.offsetWidth < element.scrollWidth;
-      setIsTruncated(isTruncated);
-    };
-
-    handleResize();
-    resizeObserver.subscribe(element, handleResize);
-
-    return () => resizeObserver.unsubscribe(element, handleResize);
-  }, []);
+  const { isTruncated, ref } = useIsTruncated<HTMLDivElement>();
 
   return (
     <Tooltip
@@ -66,7 +42,7 @@ const Ellipsified = ({
       placement={placement}
     >
       <EllipsifiedRoot
-        ref={rootRef}
+        ref={ref}
         className={className}
         lines={lines}
         style={style}

frontend/src/metabase/core/components/Markdown/Markdown.styled.tsx

@@ -28,6 +28,12 @@ export const MarkdownRoot = styled(getComponent(ReactMarkdown))<MarkdownProps>`
     max-width: 100%;
     height: auto;
   }
+
+  hr {
+    border: none;
+    border-bottom: 1px solid
+      ${props => (props.dark ? color("bg-dark") : color("border"))};
+  }
 `;
 
 function getComponent<P>(component: (props: P) => ReactElement): FC<P> {

frontend/src/metabase/core/components/Markdown/Markdown.tsx

@@ -8,6 +8,7 @@ const REMARK_PLUGINS = [remarkGfm];
 export interface MarkdownProps
   extends ComponentPropsWithRef<typeof ReactMarkdown> {
   className?: string;
+  dark?: boolean;
   disallowHeading?: boolean;
   unstyleLinks?: boolean;
   children: string;
@@ -16,6 +17,7 @@ export interface MarkdownProps
 const Markdown = ({
   className,
   children = "",
+  dark,
   disallowHeading = false,
   unstyleLinks = false,
   ...rest
@@ -30,6 +32,7 @@ const Markdown = ({
   return (
     <MarkdownRoot
       className={className}
+      dark={dark}
       remarkPlugins={REMARK_PLUGINS}
       linkTarget={"_blank"}
       unstyleLinks={unstyleLinks}

frontend/src/metabase/core/components/MarkdownPreview/MarkdownPreview.tsx

-import { ComponentProps } from "react";
+import { ComponentProps, LegacyRef } from "react";
+
+import { useIsTruncated } from "metabase/hooks/use-is-truncated";
 
 import Markdown from "../Markdown";
 import Tooltip from "../Tooltip";
@@ -17,24 +19,38 @@ export const MarkdownPreview = ({
   children,
   className,
   tooltipMaxWidth,
-}: Props) => (
-  <Tooltip
-    maxWidth={tooltipMaxWidth}
-    placement="bottom"
-    tooltip={
-      <Markdown disallowHeading unstyleLinks>
-        {children}
-      </Markdown>
-    }
-  >
-    <div>
-      <TruncatedMarkdown
-        allowedElements={ALLOWED_ELEMENTS}
-        className={className}
-        unwrapDisallowed
-      >
-        {children}
-      </TruncatedMarkdown>
-    </div>
-  </Tooltip>
-);
+}: Props) => {
+  const { isTruncated, ref } = useIsTruncated();
+
+  const setReactMarkdownRef: LegacyRef<HTMLDivElement> = div => {
+    /**
+     * react-markdown API does not allow passing ref to the container div.
+     * We can acquire the reference through its parent.
+     */
+    const reactMarkdownRoot = div?.firstElementChild;
+    ref.current = reactMarkdownRoot || null;
+  };
+
+  return (
+    <Tooltip
+      maxWidth={tooltipMaxWidth}
+      placement="bottom"
+      isEnabled={isTruncated}
+      tooltip={
+        <Markdown dark disallowHeading unstyleLinks>
+          {children}
+        </Markdown>
+      }
+    >
+      <div ref={setReactMarkdownRef}>
+        <TruncatedMarkdown
+          allowedElements={ALLOWED_ELEMENTS}
+          className={className}
+          unwrapDisallowed
+        >
+          {children}
+        </TruncatedMarkdown>
+      </div>
+    </Tooltip>
+  );
+};

frontend/src/metabase/core/components/MarkdownPreview/MarkdownPreview.unit.spec.tsx

@@ -35,20 +35,52 @@ describe("MarkdownPreview", () => {
     expect(screen.getByText(MARKDOWN_AS_TEXT)).toBeInTheDocument();
   });
 
-  it("should show tooltip with markdown formatting on hover", () => {
+  it("should not show tooltip with markdown formatting on hover when text is not truncated", () => {
     setup();
 
     userEvent.hover(screen.getByText(MARKDOWN_AS_TEXT));
+    expect(screen.queryByRole("tooltip")).not.toBeInTheDocument();
+  });
+
+  describe("Tooltip on ellipsis", () => {
+    const originalScrollWidth = Object.getOwnPropertyDescriptor(
+      HTMLElement.prototype,
+      "scrollWidth",
+    );
+
+    beforeAll(() => {
+      // emulate ellipsis
+      Object.defineProperty(HTMLElement.prototype, "scrollWidth", {
+        configurable: true,
+        value: 100,
+      });
+    });
+
+    afterAll(() => {
+      if (originalScrollWidth) {
+        Object.defineProperty(
+          HTMLElement.prototype,
+          "scrollWidth",
+          originalScrollWidth,
+        );
+      }
+    });
+
+    it("should show tooltip with markdown formatting on hover when text is truncated", () => {
+      setup();
+
+      userEvent.hover(screen.getByText(MARKDOWN_AS_TEXT));
 
-    const tooltip = screen.getByRole("tooltip");
-    expect(tooltip).not.toHaveTextContent(MARKDOWN);
-    expect(tooltip).not.toHaveTextContent(HEADING_1_MARKDOWN);
-    expect(tooltip).not.toHaveTextContent(HEADING_2_MARKDOWN);
-    expect(tooltip).toHaveTextContent(MARKDOWN_AS_TEXT);
+      const tooltip = screen.getByRole("tooltip");
+      expect(tooltip).not.toHaveTextContent(MARKDOWN);
+      expect(tooltip).not.toHaveTextContent(HEADING_1_MARKDOWN);
+      expect(tooltip).not.toHaveTextContent(HEADING_2_MARKDOWN);
+      expect(tooltip).toHaveTextContent(MARKDOWN_AS_TEXT);
 
-    const image = within(tooltip).getByRole("img");
-    expect(image).toBeInTheDocument();
-    expect(image).toHaveAttribute("alt", "alt");
-    expect(image).toHaveAttribute("src", "https://example.com/img.jpg");
+      const image = within(tooltip).getByRole("img");
+      expect(image).toBeInTheDocument();
+      expect(image).toHaveAttribute("alt", "alt");
+      expect(image).toHaveAttribute("src", "https://example.com/img.jpg");
+    });
   });
 });

frontend/src/metabase/hooks/use-is-truncated.ts

+import { useLayoutEffect, useRef, useState } from "react";
+
+import resizeObserver from "metabase/lib/resize-observer";
+
+export const useIsTruncated = <E extends Element>() => {
+  const ref = useRef<E | null>(null);
+  const [isTruncated, setIsTruncated] = useState(false);
+
+  useLayoutEffect(() => {
+    const element = ref.current;
+
+    if (!element) {
+      return;
+    }
+
+    const handleResize = () => {
+      setIsTruncated(getIsTruncated(element));
+    };
+
+    handleResize();
+    resizeObserver.subscribe(element, handleResize);
+
+    return () => {
+      resizeObserver.unsubscribe(element, handleResize);
+    };
+  }, []);
+
+  return { isTruncated, ref };
+};
+
+const getIsTruncated = (element: Element): boolean => {
+  return (
+    element.scrollHeight > element.clientHeight ||
+    element.scrollWidth > element.clientWidth
+  );
+};

frontend/src/metabase/lib/resize-observer.ts

@@ -17,13 +17,13 @@ function createResizeObserver() {
 
   return {
     observer,
-    subscribe(target: HTMLElement, callback: ResizeObserverCallback) {
+    subscribe(target: Element, callback: ResizeObserverCallback) {
       observer.observe(target);
       const callbacks = callbacksMap.get(target) ?? [];
       callbacks.push(callback);
       callbacksMap.set(target, callbacks);
     },
-    unsubscribe(target: HTMLElement, callback: ResizeObserverCallback) {
+    unsubscribe(target: Element, callback: ResizeObserverCallback) {
       const callbacks = callbacksMap.get(target) ?? [];
       if (callbacks.length === 1) {
         observer.unobserve(target);

frontend/src/metabase/visualizations/components/ScalarValue/ScalarValue.jsx

@@ -79,7 +79,7 @@ export const ScalarTitle = ({ title, description, onClick }) => (
       <ScalarDescriptionContainer className="hover-child">
         <Tooltip
           tooltip={
-            <Markdown disallowHeading unstyleLinks>
+            <Markdown dark disallowHeading unstyleLinks>
               {description}
             </Markdown>
           }

frontend/src/metabase/visualizations/components/legend/LegendCaption.jsx

@@ -42,7 +42,7 @@ const LegendCaption = ({
         {description && (
           <Tooltip
             tooltip={
-              <Markdown disallowHeading unstyleLinks>
+              <Markdown dark disallowHeading unstyleLinks>
                 {description}
               </Markdown>
             }

frontend/src/metabase/visualizations/components/skeletons/SkeletonCaption/SkeletonCaption.tsx

@@ -38,7 +38,7 @@ const SkeletonCaption = ({
           <Tooltip
             maxWidth="22em"
             tooltip={
-              <Markdown disallowHeading unstyleLinks>
+              <Markdown dark disallowHeading unstyleLinks>
                 {description}
               </Markdown>
             }

frontend/src/metabase/visualizations/components/skeletons/StaticSkeleton/StaticSkeleton.unit.spec.tsx

@@ -27,6 +27,29 @@ function setup({ description }: { description?: string } = {}) {
 
 describe("StaticSkeleton", () => {
   describe("description", () => {
+    const originalScrollWidth = Object.getOwnPropertyDescriptor(
+      HTMLElement.prototype,
+      "scrollWidth",
+    );
+
+    beforeAll(() => {
+      // emulate ellipsis
+      Object.defineProperty(HTMLElement.prototype, "scrollWidth", {
+        configurable: true,
+        value: 100,
+      });
+    });
+
+    afterAll(() => {
+      if (originalScrollWidth) {
+        Object.defineProperty(
+          HTMLElement.prototype,
+          "scrollWidth",
+          originalScrollWidth,
+        );
+      }
+    });
+
     it("should render description markdown as plain text", () => {
       setup({ description: MARKDOWN });