Skip to content

Commit

Permalink
Fix empty anchor scrolling (streamlit#9206)
Browse files Browse the repository at this point in the history
## Describe your changes

The page currently auto-scrolls if there is an empty header element,
even if no anchor was used via window hash. This PR fixes the issue by
only applying the anchor scroll if there is actually a window hash.

## GitHub Issue Link (if applicable)

- Closes streamlit#9203

## Testing Plan

- Added e2e test.

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
  • Loading branch information
lukasmasuch authored Aug 5, 2024
1 parent 1090674 commit ff31e2a
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
3 changes: 3 additions & 0 deletions e2e_playwright/st_heading.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,6 @@
for color in colors:
st.subheader(f"{color.capitalize()} Subheader Divider:", divider=color)
st.write(lorem_ipsum_text)

# Empty subheader to test correct anchor behavior:
st.subheader("")
15 changes: 14 additions & 1 deletion e2e_playwright/st_heading_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def test_correct_number_and_content_of_subheader_elements(app: Page):
subheaders = _get_subheader_elements(app).filter(
has_not_text=_subheader_divider_filter_text
)
expect(subheaders).to_have_count(7)
expect(subheaders).to_have_count(8)

expect(subheaders.nth(0)).to_have_text("info This subheader is awesome!")
expect(subheaders.nth(1)).to_have_text("This subheader is awesome too!")
Expand Down Expand Up @@ -269,3 +269,16 @@ def test_help_tooltip_works(app: Page):

title_with_help = _get_title_elements(app).nth(1)
expect_help_tooltip(app, title_with_help, tooltip_text)


def test_not_scrolled_on_empty_anchor_tag(app: Page):
"""Test that the page is not scrolled when the page contains an empty
header/anchor tag and no window hash."""

# Check if the page is still scrolled to the top
# after one second timeout.
app.wait_for_timeout(1000)
scroll_position = app.evaluate("window.scrollY")
# Usage of assert is fine here since we just need to verify that
# this is still scrolled to top, no need to wait for this to happen.
assert scroll_position == 0
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ export const HeadingWithActionElements: FunctionComponent<

const anchor = propsAnchor || createAnchorFromText(node.textContent)
setElementId(anchor)
if (window.location.hash.slice(1) === anchor) {
const windowHash = window.location.hash.slice(1)
if (windowHash && windowHash === anchor) {
setTarget(node)
}
},
Expand Down

0 comments on commit ff31e2a

Please sign in to comment.