Skip to content

[fix] Share more code between Selectbox and Multiselect to align behavior#12330

Merged
sfc-gh-bnisco merged 7 commits intodevelopfrom
bnisco/fix-multiselect
Aug 28, 2025
Merged

[fix] Share more code between Selectbox and Multiselect to align behavior#12330
sfc-gh-bnisco merged 7 commits intodevelopfrom
bnisco/fix-multiselect

Conversation

@sfc-gh-bnisco
Copy link
Copy Markdown
Collaborator

@sfc-gh-bnisco sfc-gh-bnisco commented Aug 26, 2025

Describe your changes

  • We previously fixed a bug in st.selectbox ([fix] Selectbox with accept_new_options on mobile shows keyboard #12219), but not in st.multiselect. Ultimately, this is because the code is effectively duplicated between the 2, despite much of the behavior needing to be the same.
  • This PR moves the shared logic into a shared place, thus reducing duplication and fixing the bug in both places as expected.

GitHub Issue Link (if applicable)

Testing Plan

  • ✅ Updated unit tests

Contribution License Agreement

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

@sfc-gh-bnisco sfc-gh-bnisco added security-assessment-completed change:bugfix PR contains bug fix implementation change:refactor PR contains code refactoring without behavior change impact:users PR changes affect end users labels Aug 26, 2025
@sfc-gh-bnisco sfc-gh-bnisco requested a review from Copilot August 26, 2025 17:37
@snyk-io
Copy link
Copy Markdown
Contributor

snyk-io bot commented Aug 26, 2025

🎉 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)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 26, 2025

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-12330/streamlit-1.48.1-py3-none-any.whl
🕹️ Preview app pr-12330.streamlit.app (☁️ Deploy here if not accessible)

This comment was marked as outdated.

@sfc-gh-bnisco sfc-gh-bnisco removed the change:refactor PR contains code refactoring without behavior change label Aug 26, 2025
@sfc-gh-bnisco sfc-gh-bnisco changed the title [refactor] Share more code between Selectbox and Multiselect to align behavior [fix] Share more code between Selectbox and Multiselect to align behavior Aug 26, 2025
@sfc-gh-bnisco sfc-gh-bnisco requested a review from Copilot August 27, 2025 18:54
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR consolidates shared logic between Selectbox and Multiselect components to fix a bug and reduce code duplication. The refactoring moves common filtering, validation, and state management logic into shared utilities while maintaining the same functionality for both components.

Key Changes

  • Extracts shared filtering and option management logic into useSelectCommon hook and fuzzyFilterSelectOptions utility
  • Refactors both Selectbox and Multiselect to use the shared components
  • Adds comprehensive test coverage for the new shared utilities

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
frontend/lib/src/util/fuzzyFilterSelectOptions.ts New utility function for fuzzy filtering select options with sorting
frontend/lib/src/util/fuzzyFilterSelectOptions.test.ts Unit tests for the fuzzy filtering functionality
frontend/lib/src/hooks/useSelectCommon.ts New hook that encapsulates shared logic between Selectbox and Multiselect
frontend/lib/src/components/widgets/Multiselect/Multiselect.tsx Refactored to use shared utilities, removing duplicated code
frontend/lib/src/components/widgets/Multiselect/Multiselect.test.tsx Added mobile-specific tests for the refactored component
frontend/lib/src/components/shared/Dropdown/Selectbox.tsx Refactored to use shared utilities, removing duplicated filtering logic
frontend/lib/src/components/shared/Dropdown/Selectbox.test.tsx Removed test that was moved to the shared utility test file

@sfc-gh-bnisco sfc-gh-bnisco marked this pull request as ready for review August 27, 2025 20:27
@sfc-gh-bnisco sfc-gh-bnisco force-pushed the bnisco/fix-multiselect branch from 2b48d56 to 3b0c689 Compare August 28, 2025 16:02
Copy link
Copy Markdown
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.

LGTM 👍

@sfc-gh-bnisco sfc-gh-bnisco merged commit 046409b into develop Aug 28, 2025
36 checks passed
@sfc-gh-bnisco sfc-gh-bnisco deleted the bnisco/fix-multiselect branch August 28, 2025 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:bugfix PR contains bug fix implementation impact:users PR changes affect end users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants