Skip to content

Conversation

@willhuang1997
Copy link
Contributor

@willhuang1997 willhuang1997 commented Nov 10, 2022

📚 Context

Right now, the current implementation of st.plotly_chart(...,theme="streamlit") doesn't allow overriding properties within a graph; it will always applies streamlit theme to that graph. As a result, it's not very usable for a developer who wants to override the colors or other such specific things but also get Streamlit's layout changes. As a result, this PR is for that.
This is mainly refactoring and allowing theme="streamlit" to be the default. We are using Plotly's inbuilt theme object and then passing that to the frontend because we want to swap the colors when the settings are swapped from light to dark and the backend has no knowledge of that.

Demo app: https://streamlit-feature-demos-plotlystreamlitstreamlit-app-jjumlv.streamlit.app

Please describe the project or issue background here

  • What kind of change does this PR introduce?

    • Bugfix
    • Feature
    • Refactoring
    • Other, please describe:

🧠 Description of Changes

  • Remove Most of the overriding frontend code

  • Add in Plotly's built in theme

  • Add in some hard coded temporary colors on the backend

  • Add frontend code to replace those hard coded temporary colors

  • Add bullet points summarizing your changes here

    • This is a breaking API change
    • This is a visible (user-facing) change

Revised:
Insert screenshot of your updated UI/code here
Screen Shot 2022-11-11 at 12 05 49 PM

Current:
Screen Shot 2022-11-11 at 12 05 51 PM

Insert screenshot of existing UI/code here

🧪 Testing Done

  • Screenshots included
  • Added/Updated unit tests
  • Added/Updated e2e tests

🌐 References

Does this depend on other work, documents, or tickets?

  • Issue: Closes #XXXX

Contribution License Agreement

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

@willhuang1997 willhuang1997 marked this pull request as ready for review November 11, 2022 20:44
@willhuang1997 willhuang1997 marked this pull request as draft November 11, 2022 20:49
@willhuang1997 willhuang1997 changed the title Current progress for plotly customization Plotly Customization Nov 14, 2022
@willhuang1997 willhuang1997 marked this pull request as ready for review November 14, 2022 18:38
@lukasmasuch
Copy link
Collaborator

lukasmasuch commented Nov 15, 2022

The title in this chart is cut off:

image

Copy link
Collaborator

@lukasmasuch lukasmasuch left a comment

Choose a reason for hiding this comment

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

Overall LGTM 👍 I added a couple of comments, but mostly small aspects :)

@willhuang1997 willhuang1997 merged commit f1193d8 into streamlit:develop Nov 16, 2022
tconkling added a commit to tconkling/streamlit that referenced this pull request Nov 18, 2022
* develop: (25 commits)
  Fix CORS acronym in docstring (streamlit#5727)
  Add integration tests for Snowpark (streamlit#5543)
  Release/1.15.0 (streamlit#5720)
  Add audit_frontend_dependencies script to CODEOWNERS (streamlit#5708)
  Add label_visibility option for st.checkbox (streamlit#5705)
  Display existing column names in st.map exception and make st.map work with capital letters (streamlit#5679)
  Plotly Customization (streamlit#5681)
  Turn off theme for now (streamlit#5701)
  Add all vendored code to `make notices` (streamlit#5704)
  Audit frontend licenses (streamlit#5664)
  Surround labels in quotes (streamlit#5703)
  Add info about voting on features (streamlit#5660)
  Update issue labeling scheme to adopt new standards (streamlit#5702)
  Cached media (audio+video) replay (streamlit#5695)
  Fix docstring line wrap (streamlit#5698)
  Use specialized assertion functions (streamlit#5680)
  Release 1.14.1 (streamlit#5693)
  Image replay in cached functions (streamlit#5675)
  Add docstrings for `experimental_allow_widgets` (streamlit#5670)
  Remove `st.write` from `CachedStFunctionWarning` (streamlit#5669)
  ...
kmcgrady pushed a commit that referenced this pull request Nov 30, 2022
Right now, the current implementation of st.plotly_chart(...,theme="streamlit") doesn't allow overriding properties within a graph; it will always applies streamlit theme to that graph. As a result, it's not very usable for a developer who wants to override the colors or other such specific things but also get Streamlit's layout changes. As a result, this PR is for that.
This is mainly refactoring and allowing theme="streamlit" to be the default. We are using Plotly's inbuilt theme object and then passing that to the frontend because we want to swap the colors when the settings are swapped from light to dark and the backend has no knowledge of that.

Demo app: https://streamlit-feature-demos-plotlystreamlitstreamlit-app-jjumlv.streamlit.app
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants