Skip to content

Overlay: Add min-width to Overlay container - #5129

Merged
TylerJDev merged 20 commits into
mainfrom
tylerjdev/overlay-reflow
Oct 31, 2024
Merged

Overlay: Add min-width to Overlay container#5129
TylerJDev merged 20 commits into
mainfrom
tylerjdev/overlay-reflow

Conversation

@TylerJDev

@TylerJDevTylerJDev commented Oct 15, 2024

Copy link
Copy Markdown
Member

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

Adds a prop that enables "reflow", making the Overlay responsive when the screen width reaches a breakpoint of 320px.

Example: Memex Nested Overlays Story

Draft note: The prop is added so that this doesn't affect implementations that utilize Overlay such as ActionMenu and others.

Example video

Screen.Recording.2024-10-28.at.2.34.39.PM.mov

Video description: Video showcases the responsiveness of the "Memex Nested Overlays" story. I adjust the width of the viewport to demonstrate the reflow behavior of the Overlay component.

Changelog

Changed

  • Adds min-width to Overlay container

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

@changeset-bot

changeset-botBot commented Oct 15, 2024

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 2f7a92c

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

@github-actions

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 Oct 15, 2024
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5129 October 15, 2024 19:20 Inactive
@github-actions

github-actionsBot commented Oct 15, 2024

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
packages/react/dist/browser.esm.js97.75 KB (+0.09% 🔺)
packages/react/dist/browser.umd.js98.05 KB (+0.04% 🔺)

@primer-integration

primer-integrationBot commented Oct 28, 2024

Copy link
Copy Markdown

🟢 golden-jobs completed with status success.

} as React.CSSProperties
}
data-reflow-container={reflowSize && reflow ? true : undefined}
data-reflow-container={overflow || (reflowSize && !preventOverflow) ? true : undefined}

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.

I'm confused about this logic, wouldn't the second part be overriding the FF? or is that the intent here?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

For this, I was thinking of adding the prop outside of the FF, but putting the opt-out factor inside the feature flag. So basically someone could use the prop today, but they'd have to pass it themselves.

if overflow is true, that means that the FF is enabled and a consumer would have to opt-out of it via preventOverflow=true. Otherwise, if the FF is not enabled, they must pass in preventOverflow="false" in order to get the reflow style.

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.

ahh I see, thanks for explaining that!

@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.

🚀

@github-actions
github-actionsBottemporarily deployed to storybook-preview-5129 October 31, 2024 19:21 Inactive
@TylerJDev
TylerJDev added this pull request to the merge queueOct 31, 2024
Merged via the queue into main with commit e27decdOct 31, 2024
@TylerJDev
TylerJDev deleted the tylerjdev/overlay-reflow branch October 31, 2024 19:40
@primerprimerBot mentioned this pull request Oct 31, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: recommendedThis change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@TylerJDev@mperrotti@francinelucca