FE-681: add multiselect to ds - #9505
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview A shared Supporting tweaks include clear-button focus visibility, multi-select list spacing, new Ladle stories, and exporting Reviewed by Cursor Bugbot for commit 04bb78e. Bugbot is set up for automated code reviews on this repo. Configure here. |
88cfe85 to
9c6f30c
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 04bb78e. Configure here.
lunelson
left a comment
There was a problem hiding this comment.
I'm wondering only if the value should be sorted in the same order as the options. It's possible to create different values which are logically identical but will appear different depending on the order in which they were clicked
Its not a bad idea, and I almost went ahead and implemented it, but after thinking about it I think it's better not to have this. In practice it's not really an issue - it won't create any issues with memoization, since in order to change the order, a user would need to unselect and re-select options which would already trigger onChange with the inbetween states and therefore break any memoization in the interim. And in cases where the order is important the consuming developer is likely already sorting, and in practice its often not a straight alphabetical sort, so we would be double sorting in some scenarios. But the actual reasons I decided it's better not to implement it are |

🌟 What is the purpose of this PR?
Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR: