Skip to content
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

PageLayout: Prevent content region from wrapping on wide viewports #1846

Merged
merged 9 commits into from Feb 9, 2022

Conversation

colebemis
Copy link
Contributor

@colebemis colebemis commented Feb 8, 2022

Problem

If the contents of PageLayout.Content are too wide, PageLayout.Content wraps onto a different line than PageLayout.Pane.

Solution

This PR fixes this issue with a combination of flex-grow: 1 and flex-basis: 0% to ensure that PageLayout.Content never wraps on wide viewports.

Screenshots

Before After
CleanShot 2022-02-08 at 14 57 24 CleanShot 2022-02-08 at 14 56 22

Browsers

  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari

Part of https://github.com/github/primer/issues/565

@changeset-bot
Copy link

changeset-bot bot commented Feb 8, 2022

🦋 Changeset detected

Latest commit: bbdfc2d

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 Patch

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
Contributor

github-actions bot commented Feb 8, 2022

size-limit report 📦

Path Size
dist/browser.esm.js 62.5 KB (0%)
dist/browser.umd.js 62.88 KB (0%)

@colebemis
Copy link
Contributor Author

🚫 Looks like this broke the other stories. Investigating...

@colebemis
Copy link
Contributor Author

Fixed with flex-basis

// Set flex-basis to 0% to allow flex-grow to control the width of the content region.
// Without this, the content region could wrap onto a different line
// than the pane region on wide viewports if its contents are too wide.
flexBasis: '0%',
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nitpick: we could just say flexBasis: 0. We don't need the unit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants