Conversation
Contributor
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
Contributor
✅ PR preview is ready!
|
# Conflicts: # frontend/app/src/App.tsx # proto/streamlit/proto/ClientState.proto
# Conflicts: # e2e_playwright/st_context.py # e2e_playwright/st_context_test.py
kajarenc
added
security-assessment-completed
impact:internal
kajarenc
changed the title
kajarenc
changed the title
type field for now "light" or "dark"
lukasmasuch
reviewed
May 27, 2025
Comment on lines
+580
to
+584
| if ( | ||
| _prevProps.theme.activeTheme.name !== this.props.theme.activeTheme.name | ||
| ) { | ||
| this.sendRerunBackMsg() | ||
| } |
Collaborator
There was a problem hiding this comment.
Does this also cover scenarios like the theme getting changed via host communication or via theme editor? And do we want to do something when the system theme changes during runtime?
Also, is there a risk of redundant reruns -> e.g. does this lead to always having another rerun when an themed app sessions starts?
Contributor
Author
There was a problem hiding this comment.
Does this also cover scenarios like the theme getting changed via host communication or via theme editor? And do we want to do something when the system theme changes during runtime?
Yes, tested it manually and also added e2e playwright for sending a dark theme host to guest message, and it works as expected.
I'm not sure about redundant reruns, I think componentDidUpdate lifecycle method should correctly handle initial render.
always return StreamlitTheme, type could be None
## Describe your changes Move AttributeDictionary to main util file, because now it is also imported in context.py <!-- If it's a visual change, please include a screenshot or video! --> ## GitHub Issue Link (if applicable) ## Testing Plan Simple refactoring of moving class from one file to another, covered by existing tests - Explanation of why no additional tests are needed - Unit Tests (JS and/or Python) - E2E Tests - Any manual testing needed? --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
Contributor
📉 Python coverage change detectedThe Python unit test coverage has decreased by 0.0277%
✅ Coverage change is within normal range. Coverage by files
|
# Conflicts: # e2e_playwright/__snapshots__/linux/hostframe_app_test/hostframe_app-theme_message_after[chromium].png # e2e_playwright/__snapshots__/linux/hostframe_app_test/hostframe_app-theme_message_before[chromium].png # e2e_playwright/__snapshots__/linux/hostframe_app_test/hostframe_app-toolbar_items[chromium].png
kajarenc
commented
Jun 2, 2025
Contributor
Author
There was a problem hiding this comment.
This snapshot change most probably happened because I added a new toolbar button (for sending dark theme host to guest message), and the hostframe toolbar is now a couple of pixel wider
lukasmasuch
approved these changes
Jun 4, 2025
e2e_playwright/st_context_test.py
Outdated
Comment on lines
+61
to
+63
| def test_theme_type(themed_app: Page, request: pytest.FixtureRequest): | ||
| """Test that the theme.type is correctly set.""" | ||
| app_theme = request.getfixturevalue("app_theme") |
Collaborator
There was a problem hiding this comment.
nit: I think you can also just directly use app_theme: str as a fixture.
lib/streamlit/runtime/context.py
Outdated
| attribute-style access. | ||
| """ | ||
|
|
||
| type: str | None |
Collaborator
There was a problem hiding this comment.
nit: a small typing improvement might be to use Literal["dark", "light"] | None here. But maybe mypy will complain
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new theme field in the app context so that clients can detect whether the active theme is "light" or "dark".
- Added a
color_schemefield toContextInfoin the protobuf definition. - Introduced
StreamlitThemeon the Python side (usingAttributeDictionary) and exposed it viast.context.theme. - Updated the frontend (
App.tsx) to send the theme to the backend, rerun on theme changes, and augmented tests to cover the new field.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| proto/streamlit/proto/ClientState.proto | Added optional string color_scheme to ContextInfo. |
| lib/streamlit/util.py | Introduced AttributeDictionary for attribute-style dicts. |
| lib/streamlit/runtime/context.py | Added StreamlitTheme and context.theme property. |
| lib/streamlit/elements/vega_charts.py | Replaced internal AttributeDictionary import with util. |
| lib/streamlit/elements/plotly_chart.py | Ditto for AttributeDictionary. |
| lib/streamlit/elements/lib/event_utils.py | Removed deprecated AttributeDictionary file. |
| lib/streamlit/elements/deck_gl_json_chart.py | Updated import to use util’s AttributeDictionary. |
| lib/streamlit/elements/arrow.py | Refactored import and cleaned up **kwargs annotation. |
| frontend/app/src/App.tsx | Sends colorScheme, handles reruns on theme changes. |
| frontend/app/src/App.test.tsx | Added colorScheme to mocked context in tests. |
| e2e_playwright/test_assets/hostframe.html | Added sendDarkThemeMessage button for dark theme. |
| e2e_playwright/st_context_test.py | New test for Theme type: after rerun. |
| e2e_playwright/st_context.py | Writes Theme type: in the sample app. |
| e2e_playwright/hostframe_app_test.py | Incremented toolbar button count and added dark theme test. |
| e2e_playwright/hostframe_app.py | Writes Theme type: in the hostframe sample. |
Comments suppressed due to low confidence (4)
proto/streamlit/proto/ClientState.proto:30
- [nitpick] Consider whether the field name
color_schemeshould use a JSON-friendly camelCase alias (colorScheme) via protobuf options, to match the frontend naming and avoid mapping issues.
optional string color_scheme = 6;
lib/streamlit/runtime/context.py:67
- [nitpick] The
StreamlitThemedocstring largely duplicatesAttributeDictionarydocs. Consider simplifying to focus on the theme-specific fields (type) and behavior.
class StreamlitTheme(AttributeDictionary):
lib/streamlit/runtime/context.py:77
- [nitpick] Using
typeas an attribute name may be confusing (it shadows the built-in). Consider renaming toschemeorcolor_schemeon theStreamlitThemeobject for clarity.
type: str | None
frontend/app/src/App.tsx:1653
- The frontend sends
colorScheme(camelCase), but the proto field is defined ascolor_scheme(snake_case). Ensure the JSON-to-proto mapping aligns or rename to match the protobuf field to avoid the value being dropped.
colorScheme: this.getThemeColorScheme(),
Co-authored-by: Copilot <[email protected]>
sfc-gh-lmasuch
approved these changes
Jun 5, 2025
|
Hello, the documentation at https://docs.streamlit.io/develop/api-reference/caching-and-state/st.context is not up-to-date and doesn't include Ah it seems that 1.45.2 is still not released actually, so that might explain.. But it's included in 1.46.0 which was released yesterday, could you tell when do you think the docs will be updated with this? |
Collaborator
|
This is probably missed because we tagged it Thanks for letting us know @bew . |
jrieke
added
impact:users
Collaborator
|
It's updated now! Also changed the label here. |
lukasmasuch
added
change:feature
|
great thank you 👍 |
Contributor
|
I can confirm: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe your changes
Add theme field to context, it would contain only
typefield for now, "light" or "dark".GitHub Issue Link (if applicable)
Closes #5009
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.