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

feat: [CFISO-1472] Implement new Pagecut style to Layout component #2747

Open
wants to merge 33 commits into
base: main
Choose a base branch
from

Conversation

stephanLeece
Copy link
Contributor

@stephanLeece stephanLeece commented May 7, 2024

Purpose of PR

Updates the Layout alpha component to the new Modern and Scalable U.I design.

Tickets:
https://contentful.atlassian.net/browse/CFISO-1472
https://contentful.atlassian.net/browse/CFISO-1520

  • New Narrow variant with max Layout.body width of 720px
  • narrow, wide variants have top border radius, box shadow, 16px horizontal margin and 12px top margin
  • Layout default variant is now wide as it is more ...widely used.

Full screen:
Screenshot 2024-05-14 at 17 22 28

Wide:
Screenshot 2024-05-14 at 17 21 51

Narrow:
Screenshot 2024-05-14 at 17 22 51

PR Checklist

  • I have read the relevant readme.md file(s)
  • All commits follow our Git commit message convention
  • Tests are added/updated/not required
  • Tests are passing
  • Storybook stories are added/updated/not required
  • Usage notes are added/updated/not required
  • Has been tested based on Contentful's browser support
  • Doesn't contain any sensitive information

Copy link

vercel bot commented May 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
forma-36 ✅ Ready (Inspect) Visit Preview May 31, 2024 11:25am

Copy link

changeset-bot bot commented May 7, 2024

⚠️ No Changeset found

Latest commit: 5fec0ae

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

github-actions bot commented May 7, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
CommonJS 134.77 KB (0%) 2.7 s (0%) 113 ms (+60.27% 🔺) 2.9 s
Module 131.19 KB (0%) 2.7 s (0%) 134 ms (+56.06% 🔺) 2.8 s

@stephanLeece stephanLeece changed the title feat: [CFISO-1472] Implement new Pagecut style Layout component feat: [CFISO-1472] Implement new Pagecut style to Layout, Header components May 7, 2024
@stephanLeece stephanLeece marked this pull request as ready for review May 7, 2024 16:39
@stephanLeece stephanLeece requested review from damann and a team as code owners May 7, 2024 16:39
@stephanLeece stephanLeece requested review from massao and dan-cf and removed request for dan-cf May 7, 2024 16:39
@damann

This comment was marked as resolved.

@stephanLeece
Copy link
Contributor Author

stephanLeece commented May 8, 2024

@stephanLeece I noticed that the last item in the breadcrumb lacks a margin. It should be the same as for the breadcrumb links.

Screenshot 2024-05-08 at 10 20 25 Two tiny details in the documentation:
  1. the example content (form) has a padding. Ideally that would be removed.
  2. The background there is white. It should be gray/100

Screenshot 2024-05-08 at 10 19 26 -36/assets/67584870/e0423f04-ad7b-4bf4-a21c-383a59ee9000">
Generally speaking, could you please also update the documentation to show the different variants: left, right, both sidebars, and full, wide, narrow?

Let me know if you have questions.

One thing regarding the background - i will add it for the example, but it's not part of the Layout component itself. So we will need to add that background to the parent component when using Layout in the web app.

Copy link
Collaborator

@denkristoffer denkristoffer left a comment

Choose a reason for hiding this comment

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

Code looks good to me, I think, but let me play around with it a bit as well 🙂

packages/components/layout/src/Layout.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@denkristoffer denkristoffer left a comment

Choose a reason for hiding this comment

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

Looks good to me but I will request changes to block release for now. Do you want to release an alpha?

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.

None yet

3 participants