Skip to content

Conversation

@francinelucca
Copy link
Member

@francinelucca francinelucca commented Apr 25, 2025

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:

  • Filter value is cleared
  • Items have changed
  • Panel is reopened

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

  • Added sorting logic to SelectPanel
  • New Jest tests for sorting

Changed

  • Updated snapshots

Removed

  • Custom sorting logic from SelectPanel stories as it is no longer needed

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

SelectPanel stories

Merge checklist

Copilot AI review requested due to automatic review settings April 25, 2025 01:05
@francinelucca francinelucca requested a review from a team as a code owner April 25, 2025 01:05
@francinelucca francinelucca requested a review from joshblack April 25, 2025 01:05
@changeset-bot
Copy link

changeset-bot bot commented Apr 25, 2025

🦋 Changeset detected

Latest commit: 4948a5a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Minor

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 github-actions bot added the staff Author is a staff member label Apr 25, 2025
@github-actions github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Apr 25, 2025
@github-actions
Copy link
Contributor

👋 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!

Copy link
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 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
Copy link

Copilot AI Apr 25, 2025

Choose a reason for hiding this comment

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

[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) => {
Copy link

Copilot AI Apr 25, 2025

Choose a reason for hiding this comment

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

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.
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,
Copy link
Member Author

Choose a reason for hiding this comment

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

any issues with having a FF on by default? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Nope! Should be okay 👍

@github-actions
Copy link
Contributor

github-actions bot commented Apr 25, 2025

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 98.89 KB (+0.13% 🔺)
packages/react/dist/browser.umd.js 99.19 KB (+0.16% 🔺)

@github-actions github-actions bot requested a deployment to storybook-preview-5971 April 25, 2025 01:08 Abandoned
@primer-integration
Copy link

👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/374845

@francinelucca francinelucca added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Apr 25, 2025
@github-actions github-actions bot temporarily deployed to storybook-preview-5971 April 25, 2025 01:23 Inactive
@primer primer bot requested a review from a team as a code owner April 25, 2025 01:28
@primer primer bot requested a review from tbenning April 25, 2025 01:28
@github-actions github-actions bot added integration-tests: failing Changes in this PR cause breaking changes in gh/gh and removed update snapshots 🤖 Command that updates VRT snapshots on the pull request integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Apr 25, 2025
@github-actions github-actions bot requested a deployment to storybook-preview-5971 April 25, 2025 01:34 Abandoned
@github-actions github-actions bot requested a deployment to storybook-preview-5971 April 25, 2025 01:45 Abandoned
@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2025

👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks!

@github-actions github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label May 6, 2025
@github-actions github-actions bot removed the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label May 6, 2025
@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2025

👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks!

@github-actions github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label May 6, 2025
@github-actions github-actions bot requested a deployment to storybook-preview-5971 May 6, 2025 15:55 Abandoned
@github-actions github-actions bot temporarily deployed to storybook-preview-5971 May 6, 2025 16:10 Inactive
@francinelucca francinelucca added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label May 6, 2025
@github-actions github-actions bot added integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels May 6, 2025
@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2025

👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks!

@github-actions github-actions bot removed update snapshots 🤖 Command that updates VRT snapshots on the pull request integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels May 6, 2025
@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2025

👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks!

@github-actions github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label May 6, 2025
@github-actions github-actions bot temporarily deployed to storybook-preview-5971 May 6, 2025 20:29 Inactive
@francinelucca francinelucca added this pull request to the merge queue May 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 6, 2025
@francinelucca francinelucca added this pull request to the merge queue May 6, 2025
Merged via the queue into main with commit 56248f7 May 7, 2025
45 checks passed
@francinelucca francinelucca deleted the feat/auto-select-panel-ordering branch May 7, 2025 00:01
@primer primer bot mentioned this pull request May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm staff Author is a staff member status: review needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants