-
Notifications
You must be signed in to change notification settings - Fork 621
Fix SelectPanel close button alignment in narrow viewports #6327
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: d764b63 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this 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 fixes a visual alignment issue in the SelectPanel component where the close button rendered by AnchoredOverlay was not properly aligned with the header text in narrow viewports when the primer_react_select_panel_fullscreen_on_narrow
feature flag is enabled.
- Adds CSS rule to position the close button with proper top offset in narrow viewports
- Updates Storybook example to include subtitle for better testing coverage
- Includes temporary workaround documentation with TODO for future cleanup
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/react/src/SelectPanel/SelectPanel.module.css | Adds CSS rule to align close button positioning in narrow viewports for anchor variant |
packages/react/src/SelectPanel/SelectPanel.features.stories.tsx | Adds subtitle to Storybook example for testing the alignment fix |
.changeset/silver-melons-relate.md | Documents the change as a major version bump |
👋 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! |
size-limit report 📦
|
👋 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! |
👋 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! |
b4d63a2
to
d764b63
Compare
👋 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! |
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/391382 |
🟢 golden-jobs completed with status |
Hi 👋 |
@pksjce Good question! We currently have two close button implementations:
We could consolidate these now, but it's safer to wait until the This fix just addresses the alignment issue with the AnchoredOverlay button for now. The feature flags removal is planned for a few weeks from now, once the flag is removed, we can pick one approach and clean this up properly. |
Great! Would be nice to have a tracking ticket where you can put in this context and pick it back up when everything is ready? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to test it and moving the title element with margin seems to fix the issue. 👍
Closes #5446
🐛 Problem
When
primer_react_select_panel_fullscreen_on_narrow
feature flag is enabled, the close button (X) rendered byAnchoredOverlay
is misaligned with theSelectPanel
header text in narrow viewports. The close button (32px tall, centered) doesn't align with the h1 header text (21pxline-height
,flex-start
aligned).Added CSS rule to adjust the title margin for anchored variant SelectPanels in narrow viewports, following the same pattern used by the
modal
variant:🔮 Future Work
This is a temporary fix for the dual close buttons. After
primer_react_select_panel_fullscreen_on_narrow
feature flag is removed, we should:AnchoredOverlay
or close button inSelectPanel
wrapper)Rollout strategy
Testing & Reviewing
primer_react_select_panel_fullscreen_on_narrow
enabled.Merge checklist