streamlit
streamlit copied to clipboard
Make the running man icon match theme colors
Describe your changes
This PR replaces the static icon_running.gif image used in the StatusWidget component with a dynamic, theme-aware React component called IconRunning.
The gif was collection of icons
Using those Icons from icon library implementing this:
https://github.com/user-attachments/assets/b7f8784e-920c-4650-8b90-3961c896595e
GitHub Issue Link (if applicable)
#11371
Testing Plan
- 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.
:tada: Snyk checks have passed. No issues have been found so far.
:white_check_mark: security/snyk check is complete. No issues have been found. (View Details)
:white_check_mark: license/snyk check is complete. No issues have been found. (View Details)
Hey @TreavVasu! Awesome, thanks for your contribution! The animation in your video looks pretty slow and the icon also seems to be bigger than today – any chance we can match this to what we have today? Also, the color should be the same as the icon that opens/closes the sidebar ideally.
cc @sfc-gh-nbellante since he's also working on some changes to the running man due to the top navigation bar we're adding, just to avoid any more merge conflicts ;)
Sure @jrieke I'll try to match that and also keep the color in check
@jrieke
- Size: Same as in declared variables.
- Color: Fixed for Light theme, there is issue for dark theme would try to resolve
- Have made changes on speed based on gif value
Currently set at 200ms same as gif
Here's the updated version of video:
https://github.com/user-attachments/assets/f3c5b3b1-5a56-4fe1-9be1-72bedcdf204a
@jrieke
Size: Same as in configs
Color: Same as Navbar button that show/hides: Opacity has been set to 0.4 which was the earlier setting which makes the runnning man a bit grey in dark mode can be adjusted by modifying
Current Showcase of how it is rendering :
https://github.com/user-attachments/assets/86b5cbe6-9e8f-4f72-8cc0-ba7a0e918964
All the requirements should be met by this PR. Let me know if any other +- is required
(I'm shifting my apartment) would add the relevant fixes wherever required tomorrow
@lukasmasuch can you call copilot again?
https://github.com/user-attachments/assets/c2241151-5d73-47b9-98ff-f80ef97a8395
https://github.com/user-attachments/assets/ebb9a336-5acf-4cc2-a731-7d67fc3fd504
@lukasmasuch One more call ig this should be last
@jrieke can you also check once?
@TreavVasu there is a prettier formatting issue that needs to be resolved and it seems the PR is causing unexpected frontend errors in the browser console: https://github.com/streamlit/streamlit/actions/runs/15469352188/job/43580859020?pr=11461#step:8:6984
@lukasmasuch thanks for pointing out I'll look into it asap
@TreavVasu I just tried this out locally. In light mode, the color of the running man and the arrow icon to open the sidebar match (#83858c). But in dark mode, I'm getting different values: #9fa0a2 for the running man and #fafafa for the arrow. Am I doing something wrong?
Also, @sfc-gh-nbellante, since this PR is touching the top bar and running man icon, which you changed in the top nav bar PR, would you rather want us to hold off on merging this until after top nav is merged?
It now is set to same values that sidebar button inherits
@TreavVasu sorry for going back and forth but I just talked with our designer (cc @shamis15) and ideally we want:
- The running man icon to always be
fadedText60. - The STOP text to be
bodyText(note that we recently got rid of the RUNNING text in a PR we merged last week).
Also, @lukasmasuch Jessi said the arrow icons to collapse/expand the sidebar should always be fadedText60 but there's something in the code here, which switches it to bodyText in dark themes, not sure if that's a bug or it was intentional? You apparently last edited that part.
@TreavVasu sorry for going back and forth but I just talked with our designer (cc @shamis15) and ideally we want:
- The running man icon to always be
fadedText60.- The STOP text to be
bodyText(note that we recently got rid of the RUNNING text in a PR we merged last week).Also, @lukasmasuch Jessi said the arrow icons to collapse/expand the sidebar should always be
fadedText60but there's something in the code here, which switches it tobodyTextin dark themes, not sure if that's a bug or it was intentional? You apparently last edited that part.
Okay, no issues would love to do it properly and ensure the PR is good in every way, That aside: The update I need to make is just to set it at fadedText60 for any case and I need to make it so for both themes light and dark?
I'll update the part about recent elimination of 'Running' Text from dev branch (if it's already not updated )
TLdr: Work Items for me
- Update the color to be just fadedText60 irrespective of theme
- Ensure the latest merged changes are included in here
Correct, fadedText60 for the running man icon and bodyText for the STOP text, no matter which theme is active. The deletion of RUNNING is already on develop!
Hi @jrieke
I have added those changes, do let me know if it's suitable (I can add video of it running here too )
Also do unit test require to be in this PR too? I have yet to update old ones