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 component / Layout beta + storybook #1737

Merged
merged 56 commits into from
Dec 21, 2021
Merged

PageLayout component / Layout beta + storybook #1737

merged 56 commits into from
Dec 21, 2021

Conversation

vdepizzol
Copy link
Contributor

@vdepizzol vdepizzol commented Nov 2, 2021

This PR adds the PageLayout component to Primer CSS as part of https://github.com/github/primer/issues/374, to be used alongside its Primer ViewComponent counterpart in primer/view_components#937.

This PR also adds storybook stories to the existing Layout and new PageLayout components.

PageLayout component

(previously known as "Layout beta")

PageLayout is responsible to determine the arrangement of the main regions that compose a page. This means anything after the global and local headers (i.e. repo or org headers), and anything before the global footer.

While the Layout component only handles columns meant to be placed inside a container wrapper, PageLayout controls the page spacings, supports header and footer regions, provides different styles of sidebars, and handles responsive strategies.

Test it in Storybook

Component usage

When consuming PageLayout via Primer ViewComponents, two components will be available, based on this CSS implementation:

  • <PageLayout /> - This is the classic™ GitHub layout style. By default it's a centered xl container with a pane region on the right.
  • <SplitPageLayout /> - This is a new "split" style layout, useful for hierarchical experiences where changes in the pane region are reflected in the content region.

Responsive variants

There are two ways PageLayout may appear on smaller viewports, md breakpoint and below:

  • stackRegions: presents the columns in a vertical flow, with pane and content vertically arranged.
  • separateRegions: presents pane and content regions as different pages on smaller viewports. This is useful for list/detail patterns, like the Settings example below. <SplitPageLayout /> only supports this variant.

Storybook files

PageLayout vs Layout components

For now we'll continue having Layout as a generic two-column layout component. My plan is to update Layout to continue supporting nested applications in 3-column pages, and rename it to a more specific ContentPaneLayout.

Examples/applications

Settings

  • <SplitPageLayout /> application
  • Example of separateRegions behavior on smaller viewports
  • content with an md width gets centered to the viewport whenever there's enough space.
  • Note: example is showing wrong active item
  • Storybook link
PageLayout-RepoSettings.mov

Discussions

  • Support for 3-column layout using a nested Layout component. The third column stacks vertically on a lg breakpoint before the other columns.
  • Note: The responsive header behavior in this example is not addressed by this PR.
  • Storybook link
PageLayout-Discussions.mov

Issue detail

  • Basic two-column application with stackRegions default responsive variant.
  • Use of filled divider on responsive variant
  • Storybook link
PageLayout-IssueDetail.mov

CSS implementation

I've had to disable some stylelint checks, particularly max-nesting-depth and selector-max-specificity due to the component granularity.

I've also disabled no-duplicate-selectors for code readability, with groupings by @media queries when addressing responsive variants, instead of grouping by selectors.

separateRegions-specific javascript

This component requires javascript to support anchor navigation when responsive variant is set to separateRegions. This only applies to small viewports — presentation in scenarios without JS is not compromised on larger viewports.

It's unclear where this javascript will ultimately live, whether in https://github.com/primer/behaviors or inside github/github proper. For now, a behavior reference can be found at:

https://github.com/primer/css/blob/layout-stories/docs/src/stories/helpers/pageLayoutBehavior.jsx

References

@changeset-bot
Copy link

changeset-bot bot commented Nov 2, 2021

🦋 Changeset detected

Latest commit: 0f5f357

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/css Minor

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

this is a reminder that all the abstraction can happen in the viewComponent;
there's no need to flood the css with constricted behaviors
Changelog:

### CSS classes
- `PageLayout--variant-stackRegions` → `PageLayout--responsive-stackRegions`
- `PageLayout--variant-separateRegions` → `PageLayout--responsive-separateRegions`
- `PageLayout--variant-stackRegions-panePos-*` →`PageLayout--responsive-panePos-*`
- `PageLayout--variant-separateRegions-primary-*` →`PageLayout--responsive-primary-*`
- `PageLayout-region--hasDivider-*` → `PageLayout-region--dividerNarrow-*`

### Prop names
- `responsivePrimaryRegion` → `primaryRegion`
- `paneResponsivePosition` → `panePositionNarrow`
- `paneResponsiveDivider` → `paneDividerNarrow`
- `headerResponsiveDivider`→ `headerDividerNarrow`
- `footerResponsiveDivider`→ `footerDividerNarrow`

### Args
- `*DividerNarrow` props have new `inherit` value by default
Copy link
Contributor

@langermank langermank left a comment

Choose a reason for hiding this comment

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

Looks great @vdepizzol! I read through the props again in Storybook and it all feels clear to me ☺️

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

Successfully merging this pull request may close these issues.

6 participants