Skip to content

Conversation

sfc-gh-jgarcia
Copy link
Contributor

@sfc-gh-jgarcia sfc-gh-jgarcia commented Jun 12, 2023

Describe your changes

This PR fixes the issue reported here by @sfc-gh-wschmitt where some elements of the Streamlit UI won't change their font family when using a postMessage theming call in SiS.

A piece of this was fixed by @sfc-gh-tteixeira here, but a few things were still un-styled, especially in the sidebar: This PR fixes that.

Before:

Kapture.2023-06-05.at.14.16.12.mp4
Screenshot 2023-05-22 at 13 35 51 Screenshot 2023-05-22 at 13 36 04 Screenshot 2023-05-22 at 13 36 42

After:

Kapture.2023-06-09.at.16.45.56.mp4

Screenshot 2023-06-05 at 1 47 50 PM

Jira Issue Link

Testing Plan


Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@sfc-gh-wschmitt
Copy link

Exciting 🎉

@sfc-gh-jgarcia
Copy link
Contributor Author

One clarification point is: For st.dataframe, there's an issue where the dataframe will keep the default font and will flicker to the new font after interaction, as can be seen here 👇

Kapture.2023-06-05.at.14.16.12.mp4

This seems to be related to this widget being a <canvas> element. A few alternatives we discussed with @sfc-gh-tteixeira are:

  • We could try to force a re-render by passing a dummy prop to the dataframe component, which changes when the theme changes;
  • We could try to tell the underlying dataframe library we’re using to refresh somehow. Need to reach their docs.

However, we discussed this with @sfc-gh-wschmitt and agreed that for now it won't matter in practice because the theme postMessage call would arrive before anything else is drawn on the screen. At some point we might want to address this differently so we don't run into race conditions if we get really good at loading SiS apps, but for now we should be good to ship this with the next Streamlit release in SiS.

@sfc-gh-jgarcia sfc-gh-jgarcia marked this pull request as ready for review June 12, 2023 18:54
@sfc-gh-wihuang sfc-gh-wihuang self-assigned this Jun 13, 2023
Copy link
Contributor

@sfc-gh-wihuang sfc-gh-wihuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other props that need to get passed here? I assume no but just want to double check and ask again.

@sfc-gh-jgarcia
Copy link
Contributor Author

Are there any other props that need to get passed here? I assume no but just want to double check and ask again.

Not for now, we should be fine with this solution until we get really good at loading SiS apps, which won't happen anytime soon 😄

@sfc-gh-wihuang sfc-gh-wihuang merged commit 8d86fdd into develop Jun 20, 2023
@mayagbarnes mayagbarnes deleted the sis-theming-fixes branch June 20, 2023 17:45
tconkling added a commit to tconkling/streamlit that referenced this pull request Jun 20, 2023
* tim/MyPy14:
  whoops
  Fix mypy errors
  Fix on sidebar elements not changing its font (streamlit#6825)
eric-skydio pushed a commit to eric-skydio/streamlit that referenced this pull request Dec 20, 2023
zyxue pushed a commit to zyxue/streamlit that referenced this pull request Mar 22, 2024
zyxue pushed a commit to zyxue/streamlit that referenced this pull request Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants