Skip to content

Add Modal variant with declarative buttons to PRC SelectPanel - #5883

Merged
francinelucca merged 27 commits into
mainfrom
hectahertz/selectpanel-modal
Apr 16, 2025
Merged

Add Modal variant with declarative buttons to PRC SelectPanel#5883
francinelucca merged 27 commits into
mainfrom
hectahertz/selectpanel-modal

Conversation

@hectahertz

@hectahertzhectahertz commented Apr 5, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is a remake of #5817 to expedite merging to main.

Closeshttps://github.com/github/primer/issues/4106

Changelog

New

Adds the modal variant to SelectPanel, complete with Radio icons.

Single select modal

Screen.Recording.2025-04-06.at.18.21.19.mov

Multi select modal

Screen.Recording.2025-04-06.at.18.23.07.mov

See #5817 for all the context.

Shortcut

We are still using AnchoredOverlay underneath. But, for variant=modal, we override its position by directly passing top, left, etc. to overlayProps going down to the Overlay

The ideal super clean solution would have been to refactor SelectPanel and conditionally use useAnchoredPosition instead of AnchoredOverlay for variant=anchored. Something @siddharthkp attempted in #5230 earlier but had to revert.

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

@hectahertz
hectahertz requested a review from a team as a code ownerApril 5, 2025 17:07
@changeset-bot

changeset-botBot commented Apr 5, 2025•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 738cb3c

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

This PR includes changesets to release 1 package
NameType
@primer/reactMinor

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

@hectahertzhectahertz mentioned this pull request Apr 5, 2025
Closed
13 tasks
@github-actions

github-actionsBot commented Apr 5, 2025

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

@github-actionsgithub-actionsBot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Apr 5, 2025
@github-actions
github-actionsBot requested a deployment to storybook-preview-5883 April 5, 2025 17:16 Abandoned
@hectahertz
hectahertzforce-pushed the hectahertz/selectpanel-modal branch from 500510a to 94ed190CompareApril 5, 2025 17:16
@github-actions
github-actionsBot requested a deployment to storybook-preview-5883 April 5, 2025 17:19 Abandoned
@github-actions
github-actionsBot requested a deployment to storybook-preview-5883 April 5, 2025 17:25 Abandoned
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5883 April 5, 2025 17:39 Inactive
@hectahertz

hectahertz commented Apr 6, 2025•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@francinelucca All comments from #5817 have been addressed here, can I please have a review?

@hectahertzhectahertz added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Apr 6, 2025
@github-actions
github-actionsBot requested a deployment to storybook-preview-5883 April 6, 2025 15:15 Abandoned
@primer
primerBot requested a review from a team as a code ownerApril 6, 2025 15:24
@primer
primerBot requested a review from tbenningApril 6, 2025 15:24
@github-actionsgithub-actionsBot removed the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Apr 6, 2025
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5883 April 6, 2025 15:28 Inactive
@github-actions
github-actionsBot requested a deployment to storybook-preview-5883 April 6, 2025 15:37 Abandoned
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5883 April 6, 2025 15:51 Inactive
@hectahertzhectahertz added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Apr 6, 2025
@github-actions
github-actionsBot requested a deployment to storybook-preview-5883 April 6, 2025 16:26 Abandoned
@github-actionsgithub-actionsBot removed the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Apr 6, 2025
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5883 April 6, 2025 16:40 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5883 April 7, 2025 16:46 Inactive
@github-actions

github-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

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

1 similar comment
@github-actions

github-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

👋 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-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

👋 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-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

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

@hectahertz@francinelucca
Update packages/react/src/SelectPanel/SelectPanel.tsx
20a2719
Co-authored-by: Marie Lucca <40550942+francinelucca@users.noreply.github.com>
@github-actions

github-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

👋 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-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

👋 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-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

👋 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-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

👋 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-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

👋 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-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

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

@primer-integration

primer-integrationBot commented Apr 16, 2025

Copy link
Copy Markdown

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

@github-actions

github-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

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

@primer-integration

primer-integrationBot commented Apr 16, 2025

Copy link
Copy Markdown

🟢 golden-jobs completed with status success.

@github-actions

github-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

👋 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-actionsBot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

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

francinelucca
francinelucca approved these changes Apr 16, 2025
View reviewed changes

@francineluccafrancinelucca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

✨

@primerprimerBot mentioned this pull request Apr 16, 2025
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@francineluccafrancineluccafrancinelucca approved these changes
@TylerJDevTylerJDevAwaiting requested review from TylerJDevTylerJDev is a code owner automatically assigned from primer/engineer-reviewers
+1 more reviewer
@emilybrickemilybrickemilybrick left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

integration-tests: passingChanges in this PR do NOT cause breaking changes in gh/ghintegration-tests: recommendedThis change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hectahertz@francinelucca@emilybrick