feat(SelectPanel): display selected items at the top under FF #5971
Conversation
🦋 Changeset detectedLatest commit: b3202c9 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 |
github-actions
bot
added
the
integration-tests: recommended
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces functionality to automatically sort options in the SelectPanel so that selected items are displayed first when a feature flag is enabled. It adds a new prop (showSelectedOptionsFirst), implements sorting logic within the component’s memoized items rendering, and updates tests and stories to remove obsolete custom sorting.
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/react/src/SelectPanel/SelectPanel.tsx | Adds a new prop and state to control sorting, implements new sorting logic for selected items, and updates lifecycle handling with resetSort. |
| packages/react/src/SelectPanel/SelectPanel.test.tsx | Introduces new Jest tests to verify the behavior of selected items being rendered on top under different flag conditions. |
| packages/react/src/SelectPanel/SelectPanel.stories.tsx, SelectPanel.features.stories.tsx, SelectPanel.examples.stories.tsx | Removes deprecated custom sorting logic from stories to use the built-in sorting in the component. |
| packages/react/src/FeatureFlags/DefaultFeatureFlags.ts | Enables the new feature flag (primer_react_select_panel_order_selected_at_top) by default. |
Files not reviewed (1)
- packages/react/src/SelectPanel/SelectPanel.docs.json: Language not supported
| }), | ||
| ) | ||
|
|
||
| // order selected items first |
There was a problem hiding this comment.
[nitpick] The inline comment for itemB's selection check references 'itemA' instead of 'itemB'; please update the comment to clarify that the check is for itemB.
Copilot uses AI. Check for mistakes.
| }, | ||
| } as ItemProps | ||
| }) | ||
| .sort((itemA, itemB) => { |
There was a problem hiding this comment.
Consider adding a secondary sort criteria (for example, by a stable property like the item's original index or text) when both items are equally selected/unselected to ensure a deterministic order.
Copilot uses AI. Check for mistakes.
| @@ -8,4 +8,5 @@ export const DefaultFeatureFlags = FeatureFlagScope.create({ | |||
| primer_react_overlay_overflow: false, | |||
| primer_react_segmented_control_tooltip: false, | |||
| primer_react_select_panel_fullscreen_on_narrow: false, | |||
| primer_react_select_panel_order_selected_at_top: true, | |||
There was a problem hiding this comment.
any issues with having a FF on by default? 🤔
size-limit report 📦
|
|
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/374845 |
github-actions
bot
added
integration-tests: failing
|
🟢 golden-jobs completed with status |
github-actions
bot
added
integration-tests: passing
Closes https://github.com/github/primer/issues/2409
Adds logic to SelectPanel to automatically sort items, displaying the selected items first. The sorted list gets rearranged on:
Also adds capabilities for custom sorting, providing a sorting key, sort by ascending or descending and opt-out of "selected on top" functionality.
by default, selected items will be displayed at the top when primer_react_select_panel_order_selected_at_top FF is on
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
SelectPanel stories
Merge checklist