vitamin-web icon indicating copy to clipboard operation
vitamin-web copied to clipboard

feat(@vtmn/css): The class 'vtmn-container' doesn't allow us to set the margin according to the design system

Open AlbertNambiaparambil opened this issue 2 years ago • 7 comments

Is your request related to a problem? Please describe.

When we tried to set the margins(in accordance to the Design System Guidelines) on the landing pages created using NEMO, we discovered that the container class, vtmn-container, is a fixed width container, meaning it will have a fixed left and right margin and will not take the complete width of the viewport. Attaching some screenshots below.

Describe the solution you'd like

Can we make the container class, vtmn-container a fluid container? As in, the container will take up the complete width of the viewport and will expand or shrink whenever the viewport is resized?

Our developers have told me that if the container has the styles 'min-width' and 'max-width', it will allow it to have the flexibility needed for us to set the desired margins. Right now, the container only has 'min-width'.

Additional context The below screenshot shows a NEMO landing page (link) at the breakpoint 1440. image

The below screenshot shows the same NEMO landing page at the breakpoint 1799. image

(P.S: Just so you know, I am not a programmer. This request is based on my conversations with the developers here in Spain :) )

AlbertNambiaparambil avatar Aug 02 '22 05:08 AlbertNambiaparambil

Hi @AlbertNambiaparambil, thanks for reaching us. Just to be sure, what component are you referring to?

GaspardMathon avatar Aug 02 '22 13:08 GaspardMathon

Hi thanks @AlbertNambiaparambil for this report. @GaspardMathon I think Albert is speaking about the vtmn-container generated by Tailwind CSS. Maybe we need to update our preset.https://v2.tailwindcss.com/docs/container

lauthieb avatar Aug 02 '22 13:08 lauthieb

Hello @GaspardMathon @lauthieb ! Correct, I was referring to the class vtmn-container.

AlbertNambiaparambil avatar Aug 02 '22 15:08 AlbertNambiaparambil

FYI @AlbertNambiaparambil, @thibault-mahe will investigate as soon as possible.

@thibault-mahe feel free to add a section "Containers" inside this page: http://localhost:6006/?path=/story/guidelines-layout--page

Thanks a lot!

lauthieb avatar Aug 12 '22 08:08 lauthieb

Hi @AlbertNambiaparambil, thanks for reaching us!

The Design System Guidelines do not recommend that "the container[…] take up the complete width of the viewport". On contrary, the Breakpoints section specifies to avoid using a 100% width layout on large screens, using breakpoint's margins instead. This is what the vtmn-container class is all about : specifying given max-width on different breakpoints, with fluid margins, "to avoid having too many spaces or too long sentences on large screens."

On your 1440px wide screenshot, the container has a 1200px width (as specified in the guidelines), leaving (1440 - 1200) / 2 = 120px of fluid margin on each side. On your 1799px wide screenshot, the container still has a 1200px width since the next breakpoint is at 1800px, leaving this time (1799 - 1220) / 2 = 300px of fluid margin on each side. This approach is in line with the guidelines.

I'm closing this issue. Do not hesitate to get back to me here to discuss this subject or reopen it. I may not have fully understood the issue, especially the 2 following parts : "vtmn-container is a fixed width container, meaning it will have a fixed left and right margin" (if the container is fixed then the margins are necessarily fluid, and vice versa) and the "Right now, the container only has 'min-width’." (right now the vtmn-container has a full width and max-width at different breakpoints).

I will add an illustrated « Containers » section in the guidelines to make this clearer.

Thanks! :)

thibault-mahe avatar Aug 17 '22 15:08 thibault-mahe

Hello @thibault-mahe @lauthieb, I understand the class vtmn-container works with a specific max-width and fluid margins. However, if I understand the layout guidelines of the Design System correctly, the margins are supposed to be constant within a given breakpoint. image

Which means according to the Design System (image above),

  • A device of size 1440 should have a margin of 80px
  • A device of size 1600 should also have a margin of 80px.

Where as on this landing made using NEMO,

  • A device of size 1440 has a margin of 120px (image below) image

  • A device of size 1600 has a margin of 200px (image below) image

Looking forward to our video call on Monday to discuss this problem further, Have a great weekend, Albert

AlbertNambiaparambil avatar Aug 19 '22 15:08 AlbertNambiaparambil

Hi @AlbertNambiaparambil, thanks for your message :)

If I understand the layout guidelines of the Design System correctly, the margins are supposed to be constant within a given breakpoint.

You do understand the layout guidelines of the Design System correctly, mistake is our. I discuss this subject with the design team and they told me that the specified margins are not constant, but they are minimal margins.

It still means that your screenshots are compliant with the guidelines. But it also means that, de facto, some breakpoints cannot be in line: for instance at the 600px breakpoint, the container would be 600px wide, with 0 margin...

I reopen this issue for 2 tasks:

  • Rework the breakpoints (or the container) to be compliant all the time --> this could be a breaking change so it could take longer to be fixed
  • Clarify the guidelines: make the container width clear for each breakpoint, and make the minimal margins explicit.

I'll keep you updated on this thread. Thanks again for having pointed this out!

thibault-mahe avatar Aug 22 '22 08:08 thibault-mahe

I have added this task to the Design system board projet

GaspardMathon avatar Nov 17 '22 10:11 GaspardMathon

Hi, As this issue will be brought in the next major release of Vitamin because of breaking changes for the current version, just added this issue in comment of our existing issue https://github.com/Decathlon/vitamin-design/issues/3#issuecomment-1327207263

lauthieb avatar Nov 25 '22 09:11 lauthieb

The issue is now refered here : https://github.com/dktunited/vitamin-play/issues/35 Thanks again all !

Sabrinavigil avatar Apr 07 '23 10:04 Sabrinavigil