-
Notifications
You must be signed in to change notification settings - Fork 574
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
feat(AnchoredOverlay): allow overlay to reflow #5210
feat(AnchoredOverlay): allow overlay to reflow #5210
Conversation
🦋 Changeset detectedLatest commit: 6202c7b 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 |
👋 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 📦
|
🟢 golden-jobs completed with status |
…verlay-example-overlay-has-fixed-width
…' of github.com:primer/react into 3420-prcanchoredoverlay-example-overlay-has-fixed-width
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/349748 |
@@ -112,6 +117,7 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr | |||
alignmentOffset, | |||
anchorOffset, | |||
className, | |||
preventOverflow = true, |
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.
On by default, which means the overlay will NOT reflow unless preventOverflow={false}
is explicitly set as a prop on the component. This is of course overriden by the primer_react_overlay_overflow
FF introduced in https://github.com/primer/react/pull/5129/files#diff-228ff69d3a08b1a41b50995af899947232eb64943d74ffd7aefd6bdd80aac449R9
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.
LGTM! 🚀
Closes https://github.com/github/primer/issues/3420
Adds support for the changes introduced in #5129 for
AnchoredOverlay
to allow overlay to reflow on small viewport sizes. Also modifiesOverlay
's reflow logic to always allow the overlay to reflow when the content of the overlay exceeds the size of the viewport, independently of whichwidth
is used. This guarantees accessibility in terms of WCAG 2.2 Success Criterion 1.4.10 Reflow (Level AA)Changelog
New
preventOverflow
prop toAnchoredOverlay
's*.docs.json
fileOverlay
'spreventOverflow
prop toAnchoredOverlay
Changed
width
into consideration forOverlay
's reflow (reflow at all sizes if enabled)preventOverflow={false}
toAnchoredOverlay
's stories to allow reflowAnchoredOverlay
's stories that usehoverCard
to set the min-width on the overlay instead of thehoverCard
container (this helps with reflow)Removed
Rollout strategy
Testing & Reviewing
Go to playground and width AnchoredOverlay stories and resize the browser, notice the reflow on small viewports. Compare to production.
Merge checklist