-
Notifications
You must be signed in to change notification settings - Fork 6
fix(FilterPicker): sorting behavior in controlled mode #742
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 50133ed The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🚨 Bugbot Trial ExpiredYour team's Bugbot trial has expired. Please contact your team administrator to turn on the paid plan to continue using Bugbot. A team admin can activate the plan in the Cursor dashboard. |
There was a problem hiding this 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 fixes the FilterPicker sorting behavior in controlled mode by modifying how cached order is managed during popover state changes.
- Updates caching logic to clear cached order when popover closes instead of checking popover state
- Removes conditional logic that relied on
isPopoverOpenstate for cache reuse - Ensures sorted order is recomputed fresh for each popover opening session
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/fields/FilterPicker/FilterPicker.tsx | Modified cache management logic for children and items order, clearing cache on popover close |
| .changeset/gold-bats-live.md | Added changeset entry documenting the fix |
| // Reuse the cached order if we have it. We only want to compute the sorted | ||
| // order once per pop-over opening session. The cache is cleared when the | ||
| // pop-over closes so the next opening can recompute. | ||
| if (cachedChildrenOrder.current) { |
Copilot
AI
Aug 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The simplified caching logic may cause issues in controlled mode. The original code checked !isPopoverOpen to avoid using stale cache when the popover is open. Without this check, the cache could be used even when the popover is open and selections have changed, potentially showing outdated sorting.
| if (cachedChildrenOrder.current) { | |
| // Only use the cache if the popover is closed to avoid stale sorting. | |
| if (cachedChildrenOrder.current && !isPopoverOpen) { |
| if (!isPopoverOpen && cachedItemsOrder.current) { | ||
| // Reuse the cached order if we have it. We only compute the sorted array | ||
| // once when the pop-over is opened. Cache is cleared on close. | ||
| if (cachedItemsOrder.current) { |
Copilot
AI
Aug 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the children order issue, removing the !isPopoverOpen check may cause the cached items order to be used inappropriately when the popover is open and the sorting should be recomputed based on current selections.
| if (cachedItemsOrder.current) { | |
| if (cachedItemsOrder.current && !isPopoverOpen) { |
📦 NPM canary releaseDeployed canary version 0.0.0-canary-a3aca12. |
🏋️ Size limit report
Click here if you want to find out what is changed in this build |
🧪 Storybook is successfully deployed!
|
No description provided.