Skip to content

Introduce Save and close on SelectPanel #5956

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 17 commits into from
Apr 23, 2025

Conversation

hectahertz
Copy link
Contributor

@hectahertz hectahertz commented Apr 22, 2025

NOTE: Built on top of #5939

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

Changelog

Changed

Changes the Save button in the specific scenario described in the issue to be Save and close.

After discussing with @emilybrick and @francinelucca we resolved this was the easiest approach that is user friendly and doesn't complicate the existing logic further.

This specific case should disappear anyway as soon as the onCancel prop is required on the next breaking release.

Screenshot 2025-04-23 at 22 01 01 Screenshot 2025-04-23 at 22 00 54

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 22, 2025 18:24
@hectahertz hectahertz requested review from a team as code owners April 22, 2025 18:24
Copy link

changeset-bot bot commented Apr 22, 2025

🦋 Changeset detected

Latest commit: 876aaa0

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 Patch

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

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 pull request introduces a "Save and close" functionality on the SelectPanel by deprecating the footer prop and adding the secondaryAction prop. Key changes include:

  • Replacing the footer prop with secondaryAction and deprecating the former.
  • Updating button rendering logic to support the new "Save and close" experience.
  • Adjusting test IDs and related story names to reflect the new changes.

Reviewed Changes

Copilot reviewed 27 out of 29 changed files in this pull request and generated no comments.

Show a summary per file
File Description
script/generate-e2e-tests.js Updated test ID for the SelectPanel secondary action.
packages/react/src/SelectPanel/SelectPanel.tsx Added secondaryAction prop with related UI logic.
packages/react/src/SelectPanel/SelectPanel.features.stories.tsx Renamed story from WithFooter to WithSecondaryAction.
packages/react/src/SelectPanel/SelectPanel.dev.stories.tsx Updated demonstration of the new secondaryAction usage.
e2e/components/SelectPanel.test.ts Adjusted E2E test for new test ID.
.changeset/*.md Updated changesets reflecting the new behavior.
Files not reviewed (2)
  • packages/react/src/SelectPanel/SelectPanel.docs.json: Language not supported
  • packages/react/src/SelectPanel/SelectPanel.module.css: Language not supported
Comments suppressed due to low confidence (1)

script/generate-e2e-tests.js:1125

  • Test id updated to 'with-secondary-action' while the display name remains 'With Footer'. Consider updating the display name to 'With Secondary Action' to ensure consistency.
id: 'components-selectpanel-features--with-secondary-action',

@hectahertz hectahertz requested review from emilybrick and removed request for maximedegreve April 22, 2025 18:27
@hectahertz hectahertz force-pushed the hectahertz/selectpanel-saveandclose2 branch from 9972ffb to 2a3cf4c Compare April 23, 2025 15:05
@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 23, 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!

@hectahertz hectahertz force-pushed the hectahertz/selectpanel-saveandclose2 branch from 2a3cf4c to b43cee3 Compare April 23, 2025 15:06
Copy link
Contributor

github-actions bot commented Apr 23, 2025

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 102.58 KB (+0.06% 🔺)
packages/react/dist/browser.umd.js 102.84 KB (-0.05% 🔽)

@hectahertz hectahertz force-pushed the hectahertz/selectpanel-saveandclose2 branch from 1a0ea9d to 876aaa0 Compare April 23, 2025 19:55
Copy link
Contributor

Uh oh! @hectahertz, at least one image you shared is missing helpful alt text. Check your pull request body to fix the following violations:

  • Images should have meaningful alternative text (alt text) at line 19
  • Images should have meaningful alternative text (alt text) at line 20

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

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.

❇️

@francinelucca francinelucca added this pull request to the merge queue Apr 23, 2025
Merged via the queue into main with commit 5cb35f3 Apr 23, 2025
46 checks passed
@francinelucca francinelucca deleted the hectahertz/selectpanel-saveandclose2 branch April 23, 2025 20:37
@primer primer bot mentioned this pull request Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants