Skip to content

Implement the secondaryAction prop and deprecate footer #5939

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

Merged
merged 24 commits into from
Apr 23, 2025

Conversation

hectahertz
Copy link
Contributor

@hectahertz hectahertz commented Apr 18, 2025

Closes https://github.com/github/primer/issues/4111

Changelog

New

Adds the secondaryAction prop to the SelectPanel component.
New prop example

Improves the dev story so that all the prop combinations and variants are easy to test.

Changed

Deprecates the footer prop. Changes the existing footer story to use the new secondaryAction prop.

Also renamed the 'Save' button to 'Apply' to be consistent with the Figma file.

Wide screen variants:

Screen.Recording.2025-04-19.at.17.02.13.mov

Narrow screen variants:

Screen.Recording.2025-04-19.at.17.03.44.mov

Removed

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

Merge checklist

@Copilot Copilot AI review requested due to automatic review settings April 18, 2025 14:07
@hectahertz hectahertz requested a review from a team as a code owner April 18, 2025 14:07
Copy link

changeset-bot bot commented Apr 18, 2025

🦋 Changeset detected

Latest commit: 3aa8790

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 integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Apr 18, 2025
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 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 implements the secondaryAction prop in the SelectPanel component while deprecating the legacy footer prop. Key changes include:

  • Adding a new secondaryAction prop to allow alternative control rendering in the footer area.
  • Deprecating the footer prop and updating its documentation.
  • Updating stories across the SelectPanel component to reflect the change and showcase new prop combinations.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
packages/react/src/SelectPanel/SelectPanel.tsx Introduces secondaryAction and adjusts footer rendering logic, including a label change from "Save" to "Apply".
packages/react/src/SelectPanel/SelectPanel.features.stories.tsx Renames and revises the story to use secondaryAction, and removes the custom sorting logic for selected items.
packages/react/src/SelectPanel/SelectPanel.dev.stories.tsx Updates demo variations and adds new controls for secondaryAction, with a fallback for unsupported modal cases.
Files not reviewed (1)
  • packages/react/src/SelectPanel/SelectPanel.module.css: Language not supported
Comments suppressed due to low confidence (2)

packages/react/src/SelectPanel/SelectPanel.tsx:621

  • The button label was changed from 'Save' to 'Apply'. Please confirm that this change is intentional and consistent with the rest of the application.
Apply

packages/react/src/SelectPanel/SelectPanel.features.stories.tsx:263

  • The sorting logic for selected items has been removed in favor of using the filteredItems order. Please verify that this change in behavior is intended.
export const WithSecondaryAction = () => {

{variant === 'anchored' ? (
<Component secondaryAction={secondaryActionElement} variant={variant} />
) : (
'Not supported'

This comment was marked as resolved.

Copy link
Contributor

github-actions bot commented Apr 18, 2025

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 102.52 KB (+0.24% 🔺)
packages/react/dist/browser.umd.js 102.88 KB (+0.28% 🔺)

@github-actions github-actions bot requested a deployment to storybook-preview-5939 April 18, 2025 15:45 Abandoned
@github-actions github-actions bot temporarily deployed to storybook-preview-5939 April 18, 2025 15:59 Inactive
@hectahertz hectahertz marked this pull request as draft April 18, 2025 19:08
Copy link
Member

@francinelucca francinelucca left a comment

Choose a reason for hiding this comment

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

Mostly LGTM!
Some change requests/questions/suggestions
Loving the new dev story ❤️ !

[ ] Let's confirm this link color:
image

for reference, this is what it looks like in deprecated SelectPanel

image

[ ] These "apply" buttons shouldn't be here (all FFs on)
image

image

[ ] cancel/apply buttons shouldn't be here
image
image

[ ] Let's get the CIs to green and make sure to run integration checks against dotcom 🙏

@hectahertz
Copy link
Contributor Author

hectahertz commented Apr 19, 2025

@emilybrick @francinelucca rewrote the logic and re-recorded the videos, it is now ready for review

@primer-integration
Copy link

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

@github-actions github-actions bot requested a deployment to storybook-preview-5939 April 23, 2025 17:35 Abandoned
Copy link
Member

@francinelucca francinelucca left a comment

Choose a reason for hiding this comment

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

@primer-integration
Copy link

🟢 golden-jobs completed with status success.

@github-actions github-actions bot added integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Apr 23, 2025
@hectahertz hectahertz added this pull request to the merge queue Apr 23, 2025
Merged via the queue into main with commit 633fd39 Apr 23, 2025
44 of 45 checks passed
@hectahertz hectahertz deleted the hectahertz/selectpanel-secondaryaction branch April 23, 2025 18:18
@primer primer bot mentioned this pull request Apr 23, 2025
hectahertz added a commit that referenced this pull request May 20, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants