Add anchors to Markdown headers

[ghost]

Closes #824.

Implements as custom react-markdown renderers. Handles:

• st.title('blah', anchor='some-anchor')
• st.header('blah', anchor='some-anchor')
• st.subheader('blah', anchor='some-anchor')
• Headings in Markdown
• Defaulting anchor to a slug based on the text
• Dynamically scrolling to the anchor after the page has loaded

[ghost]

Tests are actually passing, only jslint was failing, just fixed.

Edit: never mind, couple tests failing, fixing.

[karriebear]

I could see a E2E test for this functionality, possibly using snapshots? 🤷 A small unit test for valid anchor types could be helpful.

The rest of my comments are more food for thought. I think this will probably get revamped for multipage apps but in the meantime I think this works.

But we should definitely add documentation for this.

[karriebear]

I don't think this would work with cloud. I assume we will want the hash to be in the parent window url bar but the app is in a iframe. Shouldn't block us but is something we should raise to them to support.

Also, if for some reason we have 2 headers with the same anchor, this could scroll through each one until the last one. Not sure if thats desirable? I think this could be good for now but I could see later where we would want to move the scrolling to a global action.

[ghost]

The issue I ran into was that we need to use refs to grab the header elements after they're mounted (so we can use element.textContent to get the text to generate the anchor), and there's not really a callback like "when all refs have been received," so there's no natural place to call the global action.

I fixed this for now by just keeping track of an alreadyScrolled variable and only scrolling the first time. Good concern, we'll definitely circle back to this.

[karriebear]

I believe anchors are generally populated as the id or name attribute. Multipage app is something we will want to do later and we may need to switch to id later depending how we proceed.

[ghost]

Addressing this comment and your other comment together:

Here all we're doing is finding a way to pass the anchor to the frontend, which eventually intercepts the data-anchor in the ReactMarkdown renderer, and never actually prints it out in the <h2> element.

The <a data-anchor="..."> is because I used name at first, but apparently React doesn't support the name property on <a> tags. Don't know why.

Can you say more about multipage app and how that relates to using id?

[ghost]

Update: I just removed the <a data-anchor> altogether as it wasn't doing anything.

[karriebear]

Can you say more about multipage app and how that relates to using id?

My first thought for multipage app is using react router and I believe the hash link from react router would scroll to the element matching the ID. Then if we passed the ID here we could just spit out the html as is without doing any modifications.

[karriebear]

Could be id or name on the tag instead? See below for more

[ghost]

See #2513 (comment)

[karriebear]

Minus the typo and build failure looks good for now.

I am leaning towards some global scroller listening for url changes for the future as we handle url changes. We don't really do anything with the URL besides our experimental query params so I think this can be punted. The current changes should handle the common use case of sending an url with a hash and allowing people to open a page navigated to that anchor.

[karriebear]

Hm so this alreadyScrolled would prevent scrolling back to the same element when the anchor value changes. But if we somehow changed the hash and then changed it back, I don't believe this will get called / scroll into view. Alternatively if we changed the anchor and changed it back to match the current hash, it wouldn't scroll. Not sure how many scrolling use cases we want to get into with all this scrolling business. Could be something we refine as part of a future task but defer to @asaini.

I'm not sure if this would also prevent us from scrolling to multiple elements? alreadyScrolled looks to be locally defined so each tag with an anchor would get it's own.

[ghost]

Ok, I think I fixed this by just passing the anchor as the header's id. The dynamic scrolling handles scrolling on render, and after that, the browser handles everything else for us (clicking on a #link, manually changing the url, etc.)

I'm not sure if this would also prevent us from scrolling to multiple elements? alreadyScrolled looks to be locally defined so each tag with an anchor would get it's own.

createHeadingWithAnchor() is only called once to initialize HeaderWithAnchor, so all instances of <HeaderWithAnchor> will use that variable stored in the closure. You're right though, it's confusing, I'll just make it global.

[asaini]

@bh-streamlit : I am able to see the anchor links working in Chrome(v87.0) but not in Safari(v14.0).

I am using the following code:

import streamlit as st
import pandas as pd
import numpy as np

st.title('blah-one', anchor='anchor-one')

chart_data = pd.DataFrame(
    np.random.randn(20, 3),
    columns=['a', 'b', 'c'])

st.line_chart(chart_data)

st.header('blah-two', anchor='anchor-two')

chart_data = pd.DataFrame(
     np.random.randn(20, 3),
     columns=['a', 'b', 'c'])

st.line_chart(chart_data)

st.subheader('blah-three', anchor='anchor-three')

chart_data = pd.DataFrame(
     np.random.randn(20, 3),
     columns=['a', 'b', 'c'])

st.line_chart(chart_data)

st.subheader('blah-four', anchor='anchor-four')

chart_data = pd.DataFrame(
     np.random.randn(20, 3),
     columns=['a', 'b', 'c'])

st.line_chart(chart_data)

I compiled your branch and on chrome when I try to open localhost:3000/#anchor-four, I am taken to the correct header and chart, but on safari opening the same URL just refreshes the page.

Are you able to repro?

[ghost]

Ok, I investigated. Turns out Safari always refreshes when you press Enter in url bar, unlike Chrome detecting if only the hash was changed and simply scrolling to it.

As for it not scrolling after the page, that's because at the moment of scroll, the tables aren't loaded yet, so it "scrolls" to #anchor-four (when the page is more or less empty), then the tables fill. That's super obnoxious, I'll find a workaround.

[asaini]

Works well in Safari now 👍

[kantuni]

Only couple of small changes. Otherwise, looks good.
Thanks for an awesome feature!

[ghost]

@kantuni Made requested changes.

[kantuni]

With few nitpicks

[asaini]

Discussed with Thaigo and we’ll need additional review for this. Please hold on merging

[kantuni]

Thanks again, looks great!

[ghost]

Got Thiago's approval as well. Just waiting for Karrie's snapshot fix, then merging.