Fluid centered container. #286
Conversation
| max-width: $max-width - $gutter; | ||
| width: $max-width - $gutter; | ||
| } @else { | ||
| max-width: calc(100% - #{$gutter}); |
There was a problem hiding this comment.
Actually I believe the side "margin" values are different from the gutter values. These are the numbers I got from Figma (under Visual Guidelines -> Responsive Notes). John confirmed the numbers are for margin on each side so total subtraction would be below numbers x 2:
XS = 20px
SM = 30px
MD = 50px
LG = 80px
XL = 100px
XXL = max 1500px (doesn't use fluid margin since max-width is fixed to 1500px)
Looks like we might need another variable files also.
There was a problem hiding this comment.
Ok, I have adjusted the modular margin variable to match the margins provided here and updated the mixin to reflect that. Please have another review.
| // 'md': 50px, | ||
| // 'lg': 80px, | ||
| // 'xl': 100px, | ||
| // '2xl': 150px, | ||
| // ) !default; | ||
| // | ||
| // Styleguide: Variables.Core.modular-spacing | ||
| // | ||
| $modular-spacing: ( |
There was a problem hiding this comment.
The centered container is working great now. There's just one thing - I believe "margin" is a new variable that's different from and not related to the "modular spacing" variable. Added screenshot from Figma with the "modular spacing" values. Perhaps create a new variable for the margins?
There was a problem hiding this comment.
So the margins are only for the outside edge of a container to the browser window? We have three spacing models?
Do we have rules on when to apply each documented somewhere? I am confused by this.
There was a problem hiding this comment.
Yup, that's my understanding from talking to John. The margins are completely on their own and aren't really used anywhere else. If I remember correctly, John and Kerri looked at some sites and adjusted the margin numbers until things looked the best for each break point.
I do agree that having 3 different spacing models can be confusing. Perhaps we could at least rethink the gutters in terms of modular spacing, eg, a XS gutter would be modular-scale(1) or something. Talk more in the Decanter meeting?
There was a problem hiding this comment.
As for when to apply each - I don't think that's documented, but here's my understanding of it (from talking to John and working on homesite):
- margins - only for outside edge of centered container to browser window
- gutters - grid gutters, so naturally also become spacing between items that follows the "x number per row" model eg, cards.
- modular spacing - a unitless spacing system used for padding/margins within and between some items, eg, for things like vertical rhythm, card padding etc.
However, I believe that we can rewrite 1 and 2 in terms of 3.
There was a problem hiding this comment.
Ok, this will make a good topic for Wednesday. I have refactored to put the screen edge margins into their own file. Generally, I am ok with these three spacing rules as long as we document them well.
There was a problem hiding this comment.
Awesome - thanks, and I agree, we need to document these rules.
|
Perfecto 😸 Thanks! |
|
Thank you two for the discussion and thorough review. I look forward to talking about this more, and working towards better documentation. This meets the design specs. |
|
I'll create a new ticket to add the figma documentation about the spacing rules to the decanter site. |
|
@sherakama @yvonnetangsu do we want to hold off on merging until Wednesday's broader discussion, or are we OK proceeding with a merge now? |
I vote yes on merging now. @sherakama ? |
|
Yeah, we can merge and talk about the documentation/simplification on Wednesday. Go ahead and pull the trigger. |
|
Trigger being pulled. |
|
Good work, team! |
READY FOR REVIEW
Summary
Needed By (Date)
Urgency
Steps to Test
Associated Issues and/or People
See Also