fix(gutter): update mobile gutter spacing in layouts that have gutters #1829
Conversation
|
Deploy preview for pf-next ready! Built with commit 6c47de9 |
| @@ -86,6 +86,7 @@ | |||
|
|
|||
| // Gutter | |||
| --pf-global--gutter: #{$pf-global--gutter}; | |||
| --pf-global--gutter--md: #{$pf-global--gutter--md}; | |||
There was a problem hiding this comment.
In the documentation I think it would be worth noting the spacer size that the gutter uses and then how it changes on a medium breakpoint.
There was a problem hiding this comment.
You think this should be in the layouts' documents, or somewhere else?
There was a problem hiding this comment.
For example here in the Gallery Layout. "Adds space between children" is a little too broad and I think it would help if the consumer knew what size gutter was being applied.
There was a problem hiding this comment.
You may need to add it to every layout that uses the gutter
There was a problem hiding this comment.
What do you have in mind for documentation that? Not to say we shouldn't, but we don't really document spacing values currently. And if we update the value of the spacing, it would be good if we don't have to update every component/layout that uses it. I wonder if we could say it uses the globally defined gutter value and generally matches the gutters used for sections in the page layout?
There was a problem hiding this comment.
Ok yeah I get that we don't want to explicitly state the size but I think this will help to inform the user:
"Adds space between children by using the globally defined gutter value"
| @@ -134,6 +134,7 @@ $pf-global--spacer--form-element: pf-size-prem(6px) !default; | |||
|
|
|||
| // Gutter | |||
| $pf-global--gutter: $pf-global--spacer--lg !default; | |||
| $pf-global--gutter--md: $pf-global--spacer--md !default; | |||
There was a problem hiding this comment.
Are you naming this variable by the breakpoint that it relates to? IMO when i first glance at the variable it seems like its a larger version of the default variable which could be confusing for users since it really is smaller. I wonder if it should be named with sm because its a smaller version of the gutter, in the case that the variable is used elsewhere when it doesnt relate to a breakpoint? I think most of the variables in here are named based on size over breakpoint, but let me know your reasoning :)
There was a problem hiding this comment.
Not by the breakpoint - I'm using --md as the modifier to represent "medium" since that's the size spacer it maps to. I wondered about this, too. The way I see it, gutters are similar to our spacing system, but they are used in a different context. We could potentially have gutters that map 1:1 with our spacers. So I think having gutter names that match the spacer names makes sense. In this case, I'm going off the assumption that pf-global--gutter is the same as pf-global--gutter--lg, so pf-global--gutter--md is pf-global--spacer--md, pf-global--gutter--sm would map to pf-global--spacer--sm, etc. If they don't match, I think that's confusing, and if we decided to have a gutter that matched pf-global--spacer--xs, we would need to create pf-global--gutter--xxs.
There was a problem hiding this comment.
Ok great that makes sense and I like that approach
|
🎉 This PR is included in version 2.6.16 🎉 The release is available on: Your semantic-release bot 📦🚀 |
…s (#1829) * fix(gutter): update mobile gutter spacing in layouts that have gutters
fixes #1822