SelectPanel: Add variant=modal #5817

siddharthkp · 2025-03-24T22:58:46Z

Note

Current status: main is failing integration tests, so need to run integration tests again once that is cleared up

Note: This PR does not include radio icons for single selection in a modal

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

Features:

Adding variant=modal does 3 things

Changes the position to be in the center of the screen
Makes onCancel a required prop
Adds a backdrop, when you click the backdrop (click-outside), we call onCancel

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 I did attempt in #5230 earlier but had to revert.

This shortcut is easier to implement and touches less surface area.

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

Added/updated tests
Added/updated documentation
Added/updated previews (Storybook)
Changes are SSR compatible
Tested in Chrome
Tested in Firefox
Tested in Safari
Tested in Edge
(GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

changeset-bot · 2025-03-24T22:58:50Z

🦋 Changeset detected

Latest commit: 973ebab

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 · 2025-03-24T22:59:20Z

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

packages/react/src/Overlay/Overlay.tsx

packages/react/src/SelectPanel/SelectPanel.tsx

siddharthkp · 2025-03-26T22:07:14Z

packages/react/src/Overlay/Overlay.docs.json

@@ -143,6 +143,13 @@
    {
      "name": "sx",
      "type": "SystemStyleObject"
+    },
+    {
+      "name": "data-responsive",


Note for reviewer: I'm basically opening up an API to make Overlay responsive (fullscreen on narrow screens)

Should we call this responsiveVariant instead to make it feel more official? Or keep it subtle for now?

It only supports one value "fullscreen" and Overlay does not have variant so it does follow our object variant pattern 🤔 { narrow: 'fullscreen', regular: 'auto', wide: 'auto' } feels a bit odd.

Especially because Overlay is usually used by another component that passes it position props like top and left like AnchoredOverlay

I don't feel strongly either way, I don't see why not make it public though 👀

siddharthkp · 2025-03-26T22:08:14Z

packages/react/src/Overlay/Overlay.tsx

@@ -103,15 +103,6 @@ const StyledOverlay = toggleStyledComponent(
      max-width: calc(100vw - 2rem);
    }

-    &:where([data-variant='fullscreen']) {


Note for reviewer: Overlay's css module is in GA, so we don't need this css in sx anymore

I think let's keep this for now just in case, in theory the ga FF could still be turned off at any moment? The team can remove this when they remove the FF entirely from this component 🙏

I thought we weren't adding adding css for new features? Based on #5761 (comment)

I recently added this css in #5759, so thought I'll clean it up as well. yay/nay?

yeah I think the issue here is the styles won't be added at all because if FF is not enabled the Overlay classname will not be added so these styles will never make it in (why you need that FF on that other test), the correct way of adding new features without sx styling is to always have it go to css modules, for example if you created a separate class and added it to both (enabled, not enabled) implementations

github-actions · 2025-03-26T22:10:18Z

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	105.49 KB (+0.14% 🔺)
packages/react/dist/browser.umd.js	105.82 KB (+0.13% 🔺)

siddharthkp · 2025-03-27T19:53:28Z

packages/react/src/Overlay/Overlay.module.css

-    margin: 0;
-    border-radius: unset;
+  &:where([data-responsive='fullscreen']) {
+    @media screen and (--viewportRange-narrow) {


Note for reviewer: I changed full screen to only be a responsive thing instead of variant you can apply whenever. data-variant → data-responsive

packages/react/src/SelectPanel/SelectPanel.tsx

siddharthkp · 2025-03-27T19:57:32Z

packages/react/src/SelectPanel/SelectPanel.tsx

@@ -459,6 +464,16 @@ export function SelectPanel({
          role: 'dialog',
          'aria-labelledby': titleId,
          'aria-describedby': subtitle ? subtitleId : undefined,
+          ...(variant === 'modal'
+            ? {
+                /* override AnchoredOverlay position */


Note for reviewer: This is where all the shame lies. We are still using an AnchoredOverlay, but then we override it's position by directly passing top, left, etc to overlayProps going down

I'd rather us have branching on the container component instead of doing this, as it semantically is much harder to understand. I think it's fine for now, but definitely worth improving.

^ @hectahertz do we have an idea on the effort to do branching off the get go? I agree with you and we do have a bit of leeway time-wise with this, wondering if we can get it in 👀

primer-integration · 2025-03-28T21:10:21Z

🔴 golden-jobs completed with status failure.

hectahertz · 2025-04-03T15:34:46Z

This should be complete now, refer to my commit history for all the changes and features added.

Visuals and behavior for the radio buttons on the single select modal variant:

Screen.Recording.2025-04-03.at.17.24.37.mov

francinelucca

Some small suggestion/requests on the codebase. I wasn't able to test the UI since there's not a deployment available yet. I think you need to solve the merge conflicts @hectahertz

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

packages/react/src/SelectPanel/SelectPanel.tsx

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

hectahertz · 2025-04-05T17:07:21Z

Continued in #5883

wip

fbd625c

siddharthkp self-assigned this Mar 24, 2025

github-actions bot added staff Author is a staff member integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Mar 24, 2025

siddharthkp deployed to github-pages March 24, 2025 23:12 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-5817 March 24, 2025 23:12 Inactive

siddharthkp commented Mar 25, 2025

View reviewed changes

packages/react/src/Overlay/Overlay.tsx Outdated Show resolved Hide resolved

siddharthkp commented Mar 25, 2025

View reviewed changes

packages/react/src/SelectPanel/SelectPanel.tsx Outdated Show resolved Hide resolved

call onCancel before OnOpenChange for modal + click outside

1b75223

siddharthkp deployed to github-pages March 26, 2025 21:29 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-5817 March 26, 2025 21:30 Inactive

data-variant → data-responsive

cc94b85

siddharthkp commented Mar 26, 2025

View reviewed changes

disable anchor animation

be1a38a

github-actions bot temporarily deployed to storybook-preview-5817 March 26, 2025 22:20 Inactive

update snapshot

9e3f6de

github-actions bot requested a deployment to storybook-preview-5817 March 26, 2025 22:25 Abandoned

siddharthkp deployed to github-pages March 26, 2025 22:38 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-5817 March 26, 2025 22:39 Inactive

siddharthkp added 2 commits March 27, 2025 20:46

changeset

ddd5d7d

add e2e test

1f73c86

siddharthkp added the update snapshots label Mar 27, 2025

siddharthkp commented Mar 27, 2025

View reviewed changes

packages/react/src/SelectPanel/SelectPanel.tsx Outdated Show resolved Hide resolved

siddharthkp commented Mar 27, 2025

View reviewed changes

test(vrt): update snapshots

0cd98d7

siddharthkp deployed to github-pages March 28, 2025 21:07 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-5817 March 28, 2025 21:07 Inactive

siddharthkp added the update snapshots label Mar 28, 2025

siddharthkp marked this pull request as ready for review March 28, 2025 21:50

siddharthkp requested review from a team as code owners March 28, 2025 21:50

siddharthkp requested review from maximedegreve and TylerJDev March 28, 2025 21:50

test(vrt): update snapshots

d9640e3

github-actions bot removed the update snapshots label Mar 28, 2025

primer bot deployed to github-pages March 28, 2025 22:13 View deployment

github-actions bot deployed to storybook-preview-5817 March 28, 2025 22:13 View deployment

Center the panel

2191153

hectahertz added the update snapshots label Apr 3, 2025

hectahertz added 7 commits April 3, 2025 15:20

Clean up showCancelSaveButtons logic

6e3c5bf

Show the close icon on modals

3c54302

Cancel and X buttons don't need onCancel function

174401f

Add AsSingleSelectModal story

583dd69

Single select modal selection logic

b4928a3

Realign close button

e906b5b

Simplify AsMultiSelectModal story

a76b712

Render radio icons on single select modal select panel

973ebab

hectahertz force-pushed the selectpanel-modal branch from 99baabd to 973ebab Compare April 3, 2025 15:45

francinelucca requested a review from emilybrick April 4, 2025 02:16

francinelucca reviewed Apr 4, 2025

View reviewed changes

hectahertz mentioned this pull request Apr 5, 2025

Add Modal variant with declarative buttons to PRC SelectPanel #5883

Open

13 tasks

hectahertz closed this Apr 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SelectPanel: Add variant=modal #5817

SelectPanel: Add variant=modal #5817

siddharthkp commented Mar 24, 2025 •

edited

Loading

changeset-bot bot commented Mar 24, 2025 •

edited

Loading

github-actions bot commented Mar 24, 2025

siddharthkp Mar 26, 2025 •

edited

Loading

francinelucca Mar 28, 2025

siddharthkp Mar 26, 2025 •

edited

Loading

francinelucca Mar 28, 2025

siddharthkp Mar 28, 2025

francinelucca Mar 28, 2025

github-actions bot commented Mar 26, 2025 •

edited

Loading

siddharthkp Mar 27, 2025 •

edited

Loading

siddharthkp Mar 27, 2025

francinelucca Mar 28, 2025

hectahertz Apr 3, 2025 •

edited

Loading

francinelucca Apr 3, 2025

primer-integration bot commented Mar 28, 2025

hectahertz commented Apr 3, 2025 •

edited

Loading

francinelucca left a comment

hectahertz commented Apr 5, 2025

SelectPanel: Add variant=modal #5817

SelectPanel: Add variant=modal #5817

Conversation

siddharthkp commented Mar 24, 2025 • edited Loading

Features:

Shortcut

Rollout strategy

Testing & Reviewing

Merge checklist

changeset-bot bot commented Mar 24, 2025 • edited Loading

🦋 Changeset detected

github-actions bot commented Mar 24, 2025

siddharthkp Mar 26, 2025 • edited Loading

Choose a reason for hiding this comment

francinelucca Mar 28, 2025

Choose a reason for hiding this comment

siddharthkp Mar 26, 2025 • edited Loading

Choose a reason for hiding this comment

francinelucca Mar 28, 2025

Choose a reason for hiding this comment

siddharthkp Mar 28, 2025

Choose a reason for hiding this comment

francinelucca Mar 28, 2025

Choose a reason for hiding this comment

github-actions bot commented Mar 26, 2025 • edited Loading

size-limit report 📦

siddharthkp Mar 27, 2025 • edited Loading

Choose a reason for hiding this comment

siddharthkp Mar 27, 2025

Choose a reason for hiding this comment

francinelucca Mar 28, 2025

Choose a reason for hiding this comment

hectahertz Apr 3, 2025 • edited Loading

Choose a reason for hiding this comment

francinelucca Apr 3, 2025

Choose a reason for hiding this comment

primer-integration bot commented Mar 28, 2025

hectahertz commented Apr 3, 2025 • edited Loading

francinelucca left a comment

Choose a reason for hiding this comment

hectahertz commented Apr 5, 2025

siddharthkp commented Mar 24, 2025 •

edited

Loading

changeset-bot bot commented Mar 24, 2025 •

edited

Loading

siddharthkp Mar 26, 2025 •

edited

Loading

siddharthkp Mar 26, 2025 •

edited

Loading

github-actions bot commented Mar 26, 2025 •

edited

Loading

siddharthkp Mar 27, 2025 •

edited

Loading

hectahertz Apr 3, 2025 •

edited

Loading

hectahertz commented Apr 3, 2025 •

edited

Loading