feat: add displayInViewport option to SelectPanel#7699
Conversation
🦋 Changeset detectedLatest commit: fd3aa8c 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.
Pull request overview
Adds a new displayInViewport option to SelectPanel so consumers can opt into AnchoredOverlay’s “fit within viewport when possible” positioning behavior (useful for constrained layouts like dialogs).
Changes:
- Extend
SelectPanelPropsto includedisplayInViewportand forward it toAnchoredOverlay. - Document the new prop in
SelectPanel.docs.json. - Add a changeset for a minor
@primer/reactrelease noting the new prop.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/react/src/SelectPanel/SelectPanel.tsx | Plumbs displayInViewport through SelectPanel’s public API into the underlying AnchoredOverlay. |
| packages/react/src/SelectPanel/SelectPanel.docs.json | Adds documentation entry for the new displayInViewport prop. |
| .changeset/select-panel-display-in-viewport.md | Declares a minor release for the new SelectPanel prop. |
| pinPosition={!height} | ||
| className={classes.Overlay} | ||
| displayCloseButton={showXCloseIcon} | ||
| closeButtonProps={closeButtonProps} | ||
| displayInViewport={displayInViewport} |
There was a problem hiding this comment.
New displayInViewport behavior is being surfaced on SelectPanel, but there’s no unit test asserting the prop is forwarded to AnchoredOverlay / getAnchoredPosition. Since SelectPanel already has extensive tests, please add a targeted test that renders SelectPanel with displayInViewport={true} and verifies the anchored positioning logic receives displayInViewport: true (e.g., by mocking @primer/behaviors like ActionMenu does).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@francinelucca I've opened a new pull request, #7702, to work on those changes. Once the pull request is ready, I'll request review from you. |
…nel (#7702) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: francinelucca <40550942+francinelucca@users.noreply.github.com>
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/16977 |
|
Integration test results from github/github-ui:
|
|
Integration test results from github/github-ui:
|
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Towards https://github.com/github/issues/issues/20714
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist