contentful / forma-36

A design system by Contentful

https://f36.contentful.com

MIT License

331 stars 81 forks source link

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

Closed stephanLeece closed 2 months ago

stephanLeece commented 4 months ago

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

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

vercel[bot] commented 4 months ago

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

changeset-bot[bot] commented 4 months ago

⚠️ No Changeset found

Latest commit: 5fec0ae9bc75430f2025a42be5050b2b54f97e10

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

github-actions[bot] commented 4 months ago

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

damann commented 4 months ago

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

Two tiny details in the documentation:

the example content (form) has a padding. Ideally that would be removed.
The background there is white. It should be gray/100

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

stephanLeece commented 4 months ago

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

Two tiny details in the documentation:

the example content (form) has a padding. Ideally that would be removed.

The background there is white. It should be gray/100

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

stephanLeece commented 4 months ago

@damann I've added examples with different sub components (Header, Sidebar, Both Sidebars)

I'm not sure i will be able to demonstrate what fullscreen / wide / narrow look like as the space to show the component in the example page is already at 628px. There's no space to render a wrapper with a background colour either.

I have demonstrated these in the storybook story though.

stephanLeece commented 4 months ago

I think we are missing the spacing distribution / handling of the different elements mentioned in the ticket for the narrow variant.

I have the fixed 720px width set on the layout body for narrow variant. Spacing should be handled automatically. Is there something else missing?

Screenshot 2024-05-10 at 11 36 50

denkristoffer commented 2 months ago

I'm closing this for the next branch: https://github.com/contentful/forma-36/tree/next, let's keep future work there.