Select all or select all matches in st.multiselect (plus drop down styling)#13015
Select all or select all matches in st.multiselect (plus drop down styling)#13015sfc-gh-dmatthews wants to merge 59 commits intodevelopfrom
st.multiselect (plus drop down styling)#13015Conversation
✅ PR preview is ready!
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
frontend/lib/src/components/widgets/Multiselect/Multiselect.test.tsx
Outdated
Show resolved
Hide resolved
|
This generally feels good but few comments:
|
📉 Frontend coverage change detectedThe frontend unit test (vitest) coverage has decreased by 0.0000%
✅ Coverage change is within normal range. |
| height: "auto !important", | ||
| }, | ||
|
|
||
| // Apply highlight effect and padding to the first div inside the li |
There was a problem hiding this comment.
Is this styling meant to be for the select all?
There was a problem hiding this comment.
The highlight effect can apply to any item, including the "select all" and "add" inserted elements. All of the items exist as divs in an li wrapper.
frontend/lib/src/components/widgets/Multiselect/Multiselect.tsx
Outdated
Show resolved
Hide resolved
frontend/lib/src/components/widgets/Multiselect/Multiselect.tsx
Outdated
Show resolved
Hide resolved
| expect(options.length).toBe(2) | ||
| expect(options[0]).toHaveTextContent("b") | ||
| expect(options[1]).toHaveTextContent("c") | ||
| // Should have "Select all" + "b" + "c" = 3 total |
There was a problem hiding this comment.
question: I think these selectors (including the ones above) all need to be updated because options[0] is Select all. Utilize getByText("b") instead, since that's more resilient to these kinds of changes where it reorders items.
frontend/lib/src/components/widgets/DataFrame/menus/ColumnMenu.tsx
Outdated
Show resolved
Hide resolved
frontend/lib/src/components/widgets/DataFrame/menus/ColumnVisibilityMenu.tsx
Outdated
Show resolved
Hide resolved
frontend/lib/src/components/widgets/DataFrame/menus/FormattingMenu.tsx
Outdated
Show resolved
Hide resolved
| // chatInput, full, and maxCheckboxRadius are not impacted by this config. | ||
| export const radii = { | ||
| md: "0.25rem", | ||
| // This is a temporary name prior to refactoring md to sm, which will cause merge conflicts elsewhere. |
There was a problem hiding this comment.
question: When is this refactoring planned to happen?
There was a problem hiding this comment.
I would have to defer to the engineers here. I'd want everyone to have a heads up on the shift of meaning (md -> sm and md -> new). Since it would have to apply everywhere used, it would basically impact everyone, so I just didn't want to touch that in this PR.
There was a problem hiding this comment.
I'm a little worried about introducing this specific radii, especially given the potential blast radius and given that it's temporary without a specific work item attached to it.
Can we use an existing radii in the meantime?
@mayagbarnes - Tagging you as the theming expert. Any thoughts on what we should do here?
There was a problem hiding this comment.
There wasn't the correct radii between medium and default, so that why the new one. We previously had medium, default, xlarge, so I could call this one "large", but it is smaller than default, so that didn't seem right.
There was a problem hiding this comment.
It's literally halfway between, so the other option is to calculate the average on the fly.
frontend/lib/src/components/shared/Dropdown/VirtualDropdown.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I was fighting this one. I'll try again.
There was a problem hiding this comment.
Alright, I shifted the placeholder by one pixel. Here's the tradeoff that's happening:
To keep the left margin the same as the top and bottom margin of selected items in st.selectbox the padding was reduced by one pixel, I think from border impacts. I had shifted the placeholder text to also align with the text in the dropdown, which had the highlight effect align with the selected background of items in the widget.
So, I've unshifted the placedholder text, but we might need to revisit with Jess and see if we want to shift everything so it all aligns, while maintaining a uniform margin (top, bottom, left) on that first selected item in the multiselect.
Also, looking really close, it looks like there is an inset vs outset border, so the dropdown border itself is a little off.
There was a problem hiding this comment.
I think I'm still seeing this cause unexpected diffs. See how the placeholder text has changed from old to new:
Kapture.2026-02-02.at.09.27.57.mp4
|
If it's too messy, we can revert the drop down width adjustment and little shifts to deal adding the border in dark mode. It's just a pixel, but you can kind of see that it's off by looking at the first item, scrolled all the way up. cc @sfc-gh-jshamis |
st.multiselect (plus drop down styling)
| // chatInput, full, and maxCheckboxRadius are not impacted by this config. | ||
| export const radii = { | ||
| md: "0.25rem", | ||
| // TODO: Rename md -> sm, and md2 -> md |
There was a problem hiding this comment.
@sfc-gh-jshamis said yes to this proposed renaming plan.
|
Moved to Graphite stack starting with #13794 |





Describe your changes
The PR introduces new functionality for
st.multiselectto "Select all" or "Select all matches." Because a separator line was introduced between the original options and the new select-all options, a new style was introduced for all dropdowns, so this also unifies styling changes across multiple widgets for consistency.accept_new_optionsis true for selectbox or multiselect, the "Add" option at the bottom has a separator line above it.st.multiselectst.selectboxst.navigationin top modest.time_inputst.datetime_inputst.popoverst.date_inputst.time_inputst.color_pickerScreenshot or video (only for visual changes)
GitHub Issue Link (if applicable)
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.