Fix tests

streamlit · May 3, 2024 · e9eb1d0 · e9eb1d0
1 parent 2605d6c
commit e9eb1d0
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 58 deletions.
diff --git a/frontend/lib/src/components/shared/StreamlitMarkdown/StreamlitMarkdown.test.tsx b/frontend/lib/src/components/shared/StreamlitMarkdown/StreamlitMarkdown.test.tsx
@@ -30,7 +30,7 @@ import StreamlitMarkdown, {
   CustomCodeTag,
   CustomCodeTagProps,
 } from "./StreamlitMarkdown"
-import IsDialogContext from "src/components/core/IsDialogContext"
+import IsDialogContext from "@streamlit/lib/src/components/core/IsDialogContext"
 
 // Fixture Generator
 const getMarkdownElement = (body: string): ReactElement => {
@@ -115,7 +115,9 @@ describe("StreamlitMarkdown", () => {
         <StreamlitMarkdown source={source} allowHTML={false} />
       </IsSidebarContext.Provider>
     )
-    expect(screen.getByTestId("StyledLinkIconContainer")).toBeInTheDocument()
+    expect(
+      screen.getByTestId("stStyledHeadingWithActionElements")
+    ).toBeInTheDocument()
   })
 
   it("renders header anchors when isInDialog is false", () => {
@@ -125,7 +127,9 @@ describe("StreamlitMarkdown", () => {
         <StreamlitMarkdown source={source} allowHTML={false} />
       </IsDialogContext.Provider>
     )
-    expect(screen.getByTestId("StyledLinkIconContainer")).toBeInTheDocument()
+    expect(
+      screen.getByTestId("stStyledHeadingWithActionElements")
+    ).toBeInTheDocument()
   })
 
   it("passes props properly", () => {
@@ -147,7 +151,7 @@ describe("StreamlitMarkdown", () => {
       </IsSidebarContext.Provider>
     )
     expect(
-      screen.queryByTestId("StyledLinkIconContainer")
+      screen.queryByTestId("stStyledHeadingWithActionElements=1")
     ).not.toBeInTheDocument()
   })
 
@@ -159,7 +163,7 @@ describe("StreamlitMarkdown", () => {
       </IsDialogContext.Provider>
     )
     expect(
-      screen.queryByTestId("StyledLinkIconContainer")
+      screen.queryByTestId("stStyledHeadingWithActionElements")
     ).not.toBeInTheDocument()
   })
 

diff --git a/frontend/lib/src/components/shared/StreamlitMarkdown/StreamlitMarkdown.tsx b/frontend/lib/src/components/shared/StreamlitMarkdown/StreamlitMarkdown.tsx
@@ -44,24 +44,18 @@ import CodeBlock from "@streamlit/lib/src/components/elements/CodeBlock"
 import IsDialogContext from "@streamlit/lib/src/components/core/IsDialogContext"
 import IsSidebarContext from "@streamlit/lib/src/components/core/IsSidebarContext"
 import ErrorBoundary from "@streamlit/lib/src/components/shared/ErrorBoundary"
-import {
-  InlineTooltipIcon,
-  // StyledLabelHelpWrapper,
-} from "@streamlit/lib/src/components/shared/TooltipIcon"
+import { InlineTooltipIcon } from "@streamlit/lib/src/components/shared/TooltipIcon"
 import {
   getMarkdownTextColors,
   getMarkdownBgColors,
 } from "@streamlit/lib/src/theme"
 
 import { LibContext } from "@streamlit/lib/src/components/core/LibContext"
 import {
-  // StyledHeaderContainer,
-  // StyledHeaderContent,
   StyledLinkIcon,
-  // StyledLinkIconContainer,
-  StyledHeaderElements,
+  StyledHeadingActionElements,
   StyledStreamlitMarkdown,
-  StyledHeaderWithAnchor,
+  StyledHeadingWithActionElements,
 } from "./styled-components"
 
 import "katex/dist/katex.min.css"
@@ -165,22 +159,22 @@ const HeaderActionElements: FunctionComponent<HeadingActionElements> = ({
 
   return (
     <>
-      <StyledHeaderElements>
+      <StyledHeadingActionElements
+        data-testid="stHeaderActionElements"
+        className="stHeaderActionElements"
+      >
         {help && <InlineTooltipIcon iconSize="18" content={help} />}
         {elementId && !hideAnchor && (
-          <StyledLinkIcon
-            className="header-action-link"
-            href={`#${elementId}`}
-          >
+          <StyledLinkIcon href={`#${elementId}`}>
             <LinkIcon size="18" />
           </StyledLinkIcon>
         )}
-      </StyledHeaderElements>
+      </StyledHeadingActionElements>
     </>
   )
 }
 
-interface HeadingWithAnchorProps {
+interface HeadingWithActionElementsProps {
   tag: string
   anchor?: string
   hideAnchor?: boolean
@@ -190,7 +184,7 @@ interface HeadingWithAnchorProps {
 }
 
 export const HeadingWithActionElements: FunctionComponent<
-  React.PropsWithChildren<HeadingWithAnchorProps>
+  React.PropsWithChildren<HeadingWithActionElementsProps>
 > = ({ tag, anchor: propsAnchor, help, hideAnchor, children, tagProps }) => {
   const isInSidebar = React.useContext(IsSidebarContext)
   const isInDialog = React.useContext(IsDialogContext)
@@ -217,12 +211,11 @@ export const HeadingWithActionElements: FunctionComponent<
 
   const ref = React.useCallback(
     (node: any) => {
-      if (node === null || node.textContent === null) {
+      if (node === null) {
         return
       }
 
-      const anchor =
-        propsAnchor || createAnchorFromText(node.textContent.trim())
+      const anchor = propsAnchor || createAnchorFromText(node.textContent)
       setElementId(anchor)
       if (window.location.hash.slice(1) === anchor) {
         setTarget(node)
@@ -242,21 +235,28 @@ export const HeadingWithActionElements: FunctionComponent<
     />
   )
 
+  // We nest the action-elements (tooltip, link-icon) into the header element (e.g. h1),
+  // so that it appears inline. For context: we also tried setting the h's display attribute to 'inline', but
+  // then we would need to add padding to the outer container and fiddle with the vertical alignment.
+  const headerElementWithActions = React.createElement(
+    tag,
+    {
+      ...tagProps,
+      ref,
+      id: elementId,
+    },
+    <>
+      {children}
+      {actionElements}
+    </>
+  )
   return (
-    <StyledHeaderWithAnchor>
-      {React.createElement(
-        tag,
-        {
-          ...tagProps,
-          ref,
-          id: elementId,
-        },
-        <>
-          {children}
-          {actionElements}
-        </>
-      )}
-    </StyledHeaderWithAnchor>
+    <StyledHeadingWithActionElements
+      className="stHeadingWithActionElements"
+      data-testid="stStyledHeadingWithActionElements"
+    >
+      {headerElementWithActions}
+    </StyledHeadingWithActionElements>
   )
 }
 

diff --git a/frontend/lib/src/components/shared/StreamlitMarkdown/styled-components.ts b/frontend/lib/src/components/shared/StreamlitMarkdown/styled-components.ts
@@ -134,7 +134,7 @@ export const StyledStreamlitMarkdown =
     }
   )
 
-export const StyledHeaderWithAnchor = styled.div(({ theme }) => ({
+export const StyledHeadingWithActionElements = styled.div(({ theme }) => ({
   "h1, h2, h3, h4, h5, h6, span": {
     scrollMarginTop: theme.spacing.threeXL,
   },
@@ -146,17 +146,19 @@ export const StyledHeaderWithAnchor = styled.div(({ theme }) => ({
   textWrap: "pretty",
 
   // show link-icon when hovering somewhere over the heading
-  "& .header-action-link": {
-    visibility: "hidden",
+  // we use opacity instead of visibility because the react-testing-library seems
+  // to have issues with queryByRole('link') otherwise, even when triggering hover-events
+  "& .stHeaderActionElements > a": {
+    opacity: 0,
   },
   ":hover": {
-    "& .header-action-link": {
-      visibility: "visible",
+    "& .stHeaderActionElements > a": {
+      opacity: 1,
     },
   },
 }))
 
-export const StyledHeaderElements = styled.span(({ theme }) => ({
+export const StyledHeadingActionElements = styled.span(({ theme }) => ({
   marginLeft: "0.5rem",
   display: "inline-flex",
   gap: "0.5rem",
@@ -189,19 +191,6 @@ export const StyledLinkIcon = styled.a(({ theme }) => ({
   },
 }))
 
-// export const StyledHeaderContainer = styled.div(({ theme }) => ({
-//   "h1, h2, h3, h4, h5, h6, span": {
-//     scrollMarginTop: theme.spacing.threeXL,
-//   },
-//   ...sharedMarkdownStyle(theme),
-// }))
-
-export const StyledHeaderContent = styled.span(() => ({
-  position: "relative",
-  flex: "1",
-  marginLeft: "calc(2.5rem + 0.5rem)",
-}))
-
 export interface StyledDividerProps {
   rainbow: boolean
   color: string