Improve anchor button visualization for titles and headings

e2e/specs/st_tooltips_2.spec.js

@@ -21,47 +21,39 @@ describe("displays tooltips on text elements properly", () => {
   });
 
   it("Display text properly on tooltips on markdown", () => {
-    cy.getIndexed(`.stMarkdown .stTooltipIcon`, 0)
-      .invoke("show")
-      .click();
+    cy.getIndexed(`.stMarkdown .stTooltipIcon`, 0).invoke("show").click();
     cy.get("[data-testid=stMarkdownContainer]").should(
       "contain",
       "This is example tooltip displayed on markdown."
     );
   });
 
   it("Display text properly on tooltips on text", () => {
-    cy.get(`.stTextLabelWrapper > * > .stTooltipIcon`)
-      .invoke("show")
-      .click();
+    cy.get(`.stTextLabelWrapper > * > .stTooltipIcon`).invoke("show").click();
     cy.get("[data-testid=stMarkdownContainer]").should(
       "contain",
       "This is example tooltip displayed on text."
     );
   });
 
   it("Display text properly on tooltips on latex", () => {
-    cy.getIndexed(`.stMarkdown .stTooltipIcon`, 1)
-      .invoke("show")
-      .click();
+    cy.getIndexed(`.stMarkdown .stTooltipIcon`, 1).invoke("show").click();
     cy.get("[data-testid=stMarkdownContainer]").should(
       "contain",
       "This is example tooltip displayed on latex."
     );
   });
 
   it("Display text properly on tooltips on caption", () => {
-    cy.getIndexed(`.stMarkdown .stTooltipIcon`, 2)
-      .invoke("show")
-      .click();
+    cy.getIndexed(`.stMarkdown .stTooltipIcon`, 2).invoke("show").click();
     cy.get("[data-testid=stMarkdownContainer]").should(
       "contain",
       "This is example tooltip displayed on caption."
     );
   });
 
   it("Display text properly on tooltips on title", () => {
-    cy.getIndexed(`.stMarkdown .stTooltipIcon`, 3)
+    cy.getIndexed(`[data-testid=stMarkdownContainer] .stTooltipIcon`, 0)
       .invoke("show")
       .click();
     cy.get("[data-testid=stMarkdownContainer]").should(
@@ -71,7 +63,7 @@ describe("displays tooltips on text elements properly", () => {
   });
 
   it("Display text properly on tooltips on header", () => {
-    cy.getIndexed(`.stMarkdown .stTooltipIcon`, 4)
+    cy.getIndexed(`[data-testid=stMarkdownContainer] .stTooltipIcon`, 1)
       .invoke("show")
       .click();
     cy.get("[data-testid=stMarkdownContainer]").should(
@@ -81,7 +73,7 @@ describe("displays tooltips on text elements properly", () => {
   });
 
   it("Display text properly on tooltips on subheader", () => {
-    cy.getIndexed(`.stMarkdown .stTooltipIcon`, 5)
+    cy.getIndexed(`[data-testid=stMarkdownContainer] .stTooltipIcon`, 2)
       .invoke("show")
       .click();
     cy.get("[data-testid=stMarkdownContainer]").should(
@@ -91,12 +83,20 @@ describe("displays tooltips on text elements properly", () => {
   });
 
   it("Tooltips match image snapshots", () => {
-    cy.getIndexed(".stTooltipContent", 0).matchImageSnapshot("stTooltipMarkdown")
-    cy.getIndexed(".stTooltipContent", 1).matchImageSnapshot("stTooltipText")
-    cy.getIndexed(".stTooltipContent", 2).matchImageSnapshot("stTooltipLatex")
-    cy.getIndexed(".stTooltipContent", 3).matchImageSnapshot("stTooltipCaption")
-    cy.getIndexed(".stTooltipContent", 4).matchImageSnapshot("stTooltipTitle")
-    cy.getIndexed(".stTooltipContent", 5).matchImageSnapshot("stTooltipHeader")
-    cy.getIndexed(".stTooltipContent", 6).matchImageSnapshot("stTooltipSubheader")
-  })
+    cy.getIndexed(".stTooltipContent", 0).matchImageSnapshot(
+      "stTooltipMarkdown"
+    );
+    cy.getIndexed(".stTooltipContent", 1).matchImageSnapshot("stTooltipText");
+    cy.getIndexed(".stTooltipContent", 2).matchImageSnapshot("stTooltipLatex");
+    cy.getIndexed(".stTooltipContent", 3).matchImageSnapshot(
+      "stTooltipCaption"
+    );
+    cy.getIndexed(".stTooltipContent", 4).matchImageSnapshot("stTooltipTitle");
+    cy.getIndexed(".stTooltipContent", 5).matchImageSnapshot(
+      "stTooltipHeader"
+    );
+    cy.getIndexed(".stTooltipContent", 6).matchImageSnapshot(
+      "stTooltipSubheader"
+    );
+  });
 });

e2e_playwright/st_dialog.py

@@ -28,6 +28,7 @@ def dialog_with_images():
         pd.DataFrame(np.zeros((1000, 6)), columns=["A", "B", "C", "D", "E", "F"])
     )
 
+    st.subheader("Images", help="Some images are generated")
     # render multiple images. This will make the Close button to go out of
     # screen and allows scrollability of the dialog
     for _ in range(0, 3):
@@ -65,6 +66,15 @@ def large_width_dialog():
 if st.button("Open large-width Dialog"):
     large_width_dialog()
 
+
+@st.experimental_dialog("Dialog with headings")
+def headings_dialog():
+    st.header("Header", help="Some tooltip!")
+
+
+if st.button("Open headings Dialog"):
+    headings_dialog()
+
 # We use this dialog for a screenshot test as loading images via the browser
 # is non-deterministic
 with st.sidebar:

e2e_playwright/st_dialog_test.py

@@ -32,6 +32,10 @@
     app.get_by_text("Open large-width Dialog").click()
 
 
+def open_headings_dialogs(app: Page):
+    app.get_by_text("Open headings Dialog").click()
+
+
 def open_sidebar_dialog(app: Page):
     app.get_by_text("Open Sidebar-Dialog").click()
 
@@ -88,7 +92,7 @@
         open_dialog_without_images(app)
         wait_for_app_run(app)
         main_dialog = app.get_by_test_id(modal_test_id)
         expect(main_dialog).to_have_count(1)
         app.wait_for_timeout(250)
 
         click_to_dismiss(app)
@@ -126,7 +130,7 @@
 
 
 def test_fullscreen_is_disabled_for_dialog_elements(app: Page):
-    """Test that elemenets within the dialog do not show the fullscreen option."""
+    """Test that elements within the dialog do not show the fullscreen option."""
     open_dialog_with_images(app)
     wait_for_app_run(app)
     main_dialog = app.get_by_test_id(modal_test_id)
@@ -141,6 +145,27 @@
     expect(dataframe_toolbar).to_have_count(2)
 
 
+def test_actions_for_dialog_headings(app: Page):
+    """Test that headings within the dialog show the tooltip icon but not the link icon."""
+    open_headings_dialogs(app)
+    wait_for_app_run(app)
+    main_dialog = app.get_by_test_id(modal_test_id)
+    expect(main_dialog).to_have_count(1)
+
+    # check that the actions-element is there
+    action_elements = app.get_by_test_id("stHeaderActionElements")
+    expect(action_elements).to_have_count(1)
+
+    # check that the tooltip icon is there and hoverable
+    tooltip_element = action_elements.get_by_test_id("stTooltipIcon")
+    expect(tooltip_element).to_have_count(1)
+    tooltip_element.hover()
+    expect(app.get_by_text("Some tooltip!")).to_be_visible()
+
+    # check that the link-icon does not exist
+    expect(tooltip_element.locator("a")).not_to_be_attached()
+
+
 def test_dialog_displays_correctly(app: Page, assert_snapshot: ImageCompareFunction):
     open_dialog_without_images(app)
     wait_for_app_run(app)

e2e_playwright/st_heading_test.py

@@ -22,17 +22,17 @@
 
 def _get_title_elements(app: Page) -> Locator:
     """Title elements are rendered as h1 elements"""
-    return app.locator(".element-container .stMarkdown h1")
+    return app.get_by_test_id("stHeading").locator("h1")
 
 
 def _get_header_elements(app: Page) -> Locator:
     """Header elements are rendered as h2 elements"""
-    return app.locator(".element-container .stMarkdown h2")
+    return app.get_by_test_id("stHeading").locator("h2")
 
 
 def _get_subheader_elements(app: Page) -> Locator:
     """Subheader elements are rendered as h3 elements"""
-    return app.locator(".element-container .stMarkdown h3")
+    return app.get_by_test_id("stHeading").locator("h3")
 
 
 _header_divider_filter_text = re.compile(r"[a-zA-Z]+ Header Divider:")
@@ -66,7 +66,7 @@ def test_correct_number_and_content_of_header_elements(app: Page):
 
 def test_correct_number_and_content_of_subheader_elements(app: Page):
     """Test that correct number of st.subheader (=> h3) exist with the right content"""
-    subheaders = app.locator(".element-container .stMarkdown h3").filter(
+    subheaders = _get_subheader_elements(app).filter(
         has_not_text=_subheader_divider_filter_text
     )
     expect(subheaders).to_have_count(7)
@@ -130,6 +130,18 @@ def test_display_subheaders_with_anchors_and_style_icons(app: Page):
     expect(third_header.locator("svg")).not_to_be_attached()
 
 
+def test_clicking_on_anchor_changes_url(app: Page):
+    import re
+
+    headers = _get_header_elements(app)
+    first_header = headers.nth(0)
+    first_header.hover()
+    link = first_header.locator("a")
+    expect(link).to_have_attribute("href", "#this-header-is-awesome")
+    link.click()
+    expect(app).to_have_url(re.compile(".*#this-header-is-awesome"))
+
+
 def test_headers_snapshot_match(
     themed_app: Page, assert_snapshot: ImageCompareFunction
 ):
@@ -144,22 +156,23 @@ def test_headers_hovered_snapshot_match(
 ):
     headers = _get_header_elements(themed_app)
     header = headers.nth(0)
+    link_container = header.get_by_test_id("stHeaderActionElements").locator("a")
+    expect(link_container).to_have_css("visibility", "hidden")
     header.hover()
-    link_container = header.get_by_test_id("StyledLinkIconContainer")
-    expect(link_container).to_have_css("opacity", "1")
-    assert_snapshot(link_container, name="st_header-hover_with_visible_anchor")
+    expect(link_container).to_have_css("visibility", "visible")
+    assert_snapshot(header, name="st_header-hover_with_visible_anchor")
 
     header = headers.nth(3)
+    link_container = header.get_by_test_id("stHeaderActionElements").locator("a")
+    expect(link_container).to_have_css("visibility", "hidden")
     header.hover()
-    link_container = header.get_by_test_id("StyledLinkIconContainer")
-    expect(link_container).to_have_css("opacity", "1")
-    assert_snapshot(link_container, name="st_header-hover_with_help_and_anchor")
+    expect(link_container).to_have_css("visibility", "visible")
+    assert_snapshot(header, name="st_header-hover_with_help_and_anchor")
 
     header = headers.nth(4)
-    header.hover()
-    link_container = header.get_by_test_id("StyledLinkIconContainer")
-    expect(link_container).to_have_css("opacity", "1")
-    assert_snapshot(link_container, name="st_header-hover_with_help_and_hidden_anchor")
+    link_container = header.get_by_test_id("stHeaderActionElements").locator("a")
+    expect(link_container).not_to_be_attached()
+    assert_snapshot(header, name="st_header-hover_with_help_and_hidden_anchor")
 
 
 def test_subheaders_snapshot_match(
@@ -176,24 +189,23 @@ def test_subheaders_hovered_snapshot_match(
 ):
     headers = _get_subheader_elements(themed_app)
     header = headers.nth(0)
+    link_container = header.get_by_test_id("stHeaderActionElements").locator("a")
+    expect(link_container).to_have_css("visibility", "hidden")
     header.hover()
-    link_container = header.get_by_test_id("StyledLinkIconContainer")
-    expect(link_container).to_have_css("opacity", "1")
-    assert_snapshot(link_container, name="st_subheader-hover_with_visible_anchor")
+    expect(link_container).to_have_css("visibility", "visible")
+    assert_snapshot(header, name="st_subheader-hover_with_visible_anchor")
 
     header = headers.nth(5)
+    link_container = header.get_by_test_id("stHeaderActionElements").locator("a")
+    expect(link_container).to_have_css("visibility", "hidden")
     header.hover()
-    link_container = header.get_by_test_id("StyledLinkIconContainer")
-    expect(link_container).to_have_css("opacity", "1")
-    assert_snapshot(link_container, name="st_subheader-hover_with_help_and_anchor")
+    expect(link_container).to_have_css("visibility", "visible")
+    assert_snapshot(header, name="st_subheader-hover_with_help_and_anchor")
 
     header = headers.nth(6)
-    header.hover()
-    link_container = header.get_by_test_id("StyledLinkIconContainer")
-    expect(link_container).to_have_css("opacity", "1")
-    assert_snapshot(
-        link_container, name="st_subheader-hover_with_help_and_hidden_anchor"
-    )
+    link_container = header.get_by_test_id("stHeaderActionElements").locator("a")
+    expect(link_container).not_to_be_attached()
+    assert_snapshot(header, name="st_subheader-hover_with_help_and_hidden_anchor")
 
 
 def test_links_are_rendered_correctly_snapshot(

frontend/lib/src/components/shared/StreamlitMarkdown/Heading.test.tsx

@@ -20,6 +20,8 @@ import "@testing-library/jest-dom"
 import { screen } from "@testing-library/react"
 import { render } from "@streamlit/lib/src/test_util"
 import { Heading as HeadingProto } from "@streamlit/lib/src/proto"
+import IsDialogContext from "@streamlit/lib/src/components/core/IsDialogContext"
+import IsSidebarContext from "@streamlit/lib/src/components/core/IsSidebarContext"
 import Heading, { HeadingProtoProps } from "./Heading"
 
 const getHeadingProps = (
@@ -59,17 +61,38 @@ describe("Heading", () => {
     const props = getHeadingProps({ body: "hello" })
     render(<Heading {...props} />)
 
-    const link = screen.getByRole("link")
+    // trying to trigger the :hover css state did not work, so using 'hidden: true' here. We have an e2e test to check the hovering.
+    const link = screen.getByRole("link", { hidden: true })
     expect(link).toHaveAttribute("href", "#some-anchor")
   })
 
-  it("does not renders anchor link when it is hidden", () => {
+  it("does not render anchor link when it is hidden", () => {
     const props = getHeadingProps({ body: "hello", hideAnchor: true })
     render(<Heading {...props} />)
 
     expect(screen.queryByRole("link")).not.toBeInTheDocument()
   })
 
+  it("does not render anchor link in sidebar", () => {
+    const props = getHeadingProps()
+    render(
+      <IsSidebarContext.Provider value={true}>
+        <Heading {...props} />
+      </IsSidebarContext.Provider>
+    )
+    expect(screen.queryByRole("link")).not.toBeInTheDocument()
+  })
+
+  it("does not render anchor link in dialog", () => {
+    const props = getHeadingProps()
+    render(
+      <IsDialogContext.Provider value={true}>
+        <Heading {...props} />
+      </IsDialogContext.Provider>
+    )
+    expect(screen.queryByRole("link")).not.toBeInTheDocument()
+  })
+
   it("renders properly with help text", () => {
     const props = getHeadingProps({ body: "hello", help: "help text" })
     render(<Heading {...props} />)
@@ -81,6 +104,36 @@ describe("Heading", () => {
     expect(tooltip).toBeInTheDocument()
   })
 
+  it("renders properly with help text in sidebar", () => {
+    const props = getHeadingProps({ body: "hello", help: "help text" })
+    render(
+      <IsSidebarContext.Provider value={true}>
+        <Heading {...props} />
+      </IsSidebarContext.Provider>
+    )
+
+    expect(screen.getByRole("heading")).toHaveTextContent("hello")
+    expect(screen.getAllByTestId("stMarkdownContainer")).toHaveLength(1)
+
+    const tooltip = screen.getByTestId("stTooltipIcon")
+    expect(tooltip).toBeInTheDocument()
+  })
+
+  it("renders properly with help text in dialog", () => {
+    const props = getHeadingProps({ body: "hello", help: "help text" })
+    render(
+      <IsDialogContext.Provider value={true}>
+        <Heading {...props} />
+      </IsDialogContext.Provider>
+    )
+
+    expect(screen.getByRole("heading")).toHaveTextContent("hello")
+    expect(screen.getAllByTestId("stMarkdownContainer")).toHaveLength(1)
+
+    const tooltip = screen.getByTestId("stTooltipIcon")
+    expect(tooltip).toBeInTheDocument()
+  })
+
   it("does not render ol block", () => {
     const props = getHeadingProps({ body: "1) hello" })
     render(<Heading {...props} />)