Pills do not show selected value(s) if disabled #12388#12555
Pills do not show selected value(s) if disabled #12388#12555sfc-gh-bnisco merged 17 commits intostreamlit:developfrom
Conversation
…check opacity instead of color
🎉 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) |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where disabled pills and segmented controls would appear in faded gray colors, making it impossible to see selected values. The fix changes the disabled styling to use opacity instead of color changes, preserving the original colors while maintaining the disabled appearance.
- Replaced disabled color styling with opacity-based styling in BaseButton components
- Updated E2E tests to verify opacity instead of color absence for disabled states
- Fixed visual feedback for both st.pills and st.segmented_control components
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/lib/src/components/shared/BaseButton/styled-components.ts | Replaced disabled color styling with opacity to preserve original colors |
| e2e_playwright/st_segmented_control_test.py | Updated test to check for opacity instead of color negation |
| e2e_playwright/st_pills_test.py | Updated test to check for opacity instead of color negation |
lukasmasuch
added
change:bugfix
|
Hey @davidsjoberg1! Good catch, thanks for the contribution! I agree we should change this, but I think we need a different approach than simply adding opacity, so this is more consistent with other widgets. Importantly, the disabled pills/controls should 1) not have a hover effect and 2) be completely grayscale. (That's also what we do for all other widgets). Maybe it would already be sufficient if you use a slightly gray background for the selected pills/controls instead of the current white? |
…yledButtonGroupBaseButton to match disabled StyledPrimaryButton. Added lightGray background on active pills/segmented controls
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
|
Hello @jrieke, thank you for the feedback!
Below is a video of how the current implementation looks after_feedback.movPlease let me know if I need to make some changes, or if I missunderstood something :) |
|
Looks great! I think we should use a different color than 
|
|
I agree! I just pushed the change to fadedText05 |
jrieke
added
status:product-approved
|
Thanks for the contribution @davidsjoberg1! Can you please add Snapshot tests for this case? I see that we already have a test case for disabled We have directions in our contributing wiki: https://github.com/streamlit/streamlit/wiki/Running-e2e-tests-and-updating-snapshots. Let me know if you have questions, thank you! |
|
Hello @sfc-gh-bnisco! |
|
Thanks for checking @davidsjoberg1 , that's a really thoughtful question! The way you handled it is nice and pragmatic! Long-term we are moving our tests away from the index-based paradigm anyway, but this test suite hasn't yet been updated. We are good to leave this as-is! |
|
@davidsjoberg1 Congrats on getting this merged! 🥳 We'd love to send you some swag as a little thank you, just fill out this form and we'll get it on the way: https://docs.google.com/forms/d/e/1FAIpQLSe7cXh3H1DmrgNpVew9qViGIHX7vdEbTv5isA44_z5bgaKTKg/viewform |
|
@jrieke Nice, thank you! |
Describe your changes
Before the change, when disabling a st.pills component it turned into a faded gray color. The goal with the issue was to make still make it faded but keep the same colors as when not disabled.
In styled-components.ts for BaseButton there is a style called StyledButtonGroupBaseButton where the colors was set when a button is disabled. Instead of changing color I when disabling a button I just used opacity to make the faded look. Since StyledButtonGroupBaseButton is used in both st.pills and st.segmented_control this change will affect both of these components.
disabled=False
This is how it looks when the button is not disabled (just like before)
disabledfalse.mov
disabled=True
Before change:
before.mov
After change:
after.mov
GitHub Issue Link
Closes #12388
Testing Plan
I have made changes in e2e_playwright/st_pills_test.py and e2e_playwright/st_segmented_control_test.py.
I updated the disabled tests to expect opacity instead of negating an rgb(...) color, matching the new design.
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.