XY Grids with grid-margin-x causing horizontal scroll, flush to container and viewport

[stuartstarrs]

In the new XY Grid, there may be need to nest grids, with grid-margin-x applied to all.
The margins of the nested grid(s) go out of the bounds of the parent grid, and any grid-container that might be wrapped in.
If flush against the viewport edge, this will result in a horizontal scrollbar.

How to reproduce this bug:

1. Add a grid-x with grid-margin-x to a margin:0 and padding: 0
2. Add a nested grid-x with grid-margin-x inside
3. Resize your viewport to, say, 1150px and see the nested grid is 1180px with default gutters

What should happen:

Margins being margins aside, grid-container should probably contain everything

Codepen

https://codepen.io/stuartstarrs/pen/EXQaMB

[Moonbase59]

Out of curiosity: Did you contain a grid within a grid or is the inner grid contained within a CELL of the outer grid? (Makes a big difference, at least with padded grids.)

[kball]

@brettsmason I want to tag you into this

[brettsmason]

So this will occur whenever a margin grid hits the edge of the viewport as already identified. This is caused by the negative margin on the grid which is what allows the grid to line up correctly.

This was discussed in detail during development - there are basically 2 solutions to this. You can either have margin on left only, so no split gutters. This doesn't cause overflow. However this was deemed to not be a good idea for using with Masonry and a few other reasons.

The alternative is to add overflow-x: hidden to the body. It might also work on a grid container or the grid itself, but hiding overflow could have other unexpected results with other components etc. Personally I always use the body approach.

@kball not sure what to suggest. Can we add a rule to hide overflow on body or can you see any other issues?

[stuartstarrs]

@brettsmason you're right on the nesting, I see that. And margins are margins and nothing can be done about that.

The margin gutters are really useful though, I can see them being used to create spaced-out coloured blocks without too much markup. I'm just hoping that with this issue a consistent standardised Foundation approach can be decided (or baked in).

Agree that overflow-x: hidden on the grid container would be unwise... it would be all too easy to end up with grid-containers inside grid-containers and expanding components inside those getting cut off.

[jashwant]

If I am not wrong, bootstrap is dealing with it from the very start by setting a max-width on container > current breakpoint - (2 * ( negative margin on row )).
e.g.

@media (min-width: 1200px)
.container {
    width: 1140px; /* there's a 60px different here, so it doesn't overflow near breakpoint */
    max-width: 100%;
}
}

In case of full width ( fluid ) container, container-fluid contains padding = negative margin on row.

You cannot have row without a container or container-fluid , hence it always works.

I think same can work here.

[erredeco]

I had the same issue with the simple setup:

  <div class="grid-x grid-margin-x">
    <div class="small-4 cell">something</div>
    <div class="small-4 cell">something</div>
    <div class="small-4 cell">something</div>
  </div>

where the horizontal bar is always present, and:

<div class="grid-container">  
  <div class="grid-x grid-margin-x">
    <div class="small-4 cell">something</div>
    <div class="small-4 cell">something</div>
    <div class="small-4 cell">something</div>
  </div>
</div>

where it appears when resizing. In this case it can be avoided by adding .grid-container-padded but then it should be the standard.
BTW: It is just my opinion, yes, but I don't feel I like the new approach with the "grid-container" wrapper (too much similar to TW Bootstrap); I find that the old self-contained row was much more elegant.

My suggestion could be to add the margin (or padding) without splitting into left and right , but only to the right side and remove it for the last element. (or, even better, the Susy approach, where one margin (all margins are in %) is always -100% and the other one is calculated... I don't know if this path is doable)

[IamManchanda]

@jashwant I dont know whether this comment is needed or not
as i havent played with XY Grid too much right now (though plans to do it)

but on the container/container-fluid/section part what folks from bootstrap do is simply this

twbs/bootstrap#21211 (comment)

containers are not for 100% width ... instead fixed width as per device size ( media query ) . Just there to take a fixed and neat width ( center aligned ), specially used for written material ( content ).
container-fluid are there for a full width but also has a padding right and left of 15px each, specially used for written material ( content ) that takes up the full screen.
sections are there for 100% width of the screen. specially used in the backgrounds

.row.no-gutters is enough i guess . no need for including container, container-fluid or section just nest them around as per requirement. For eg..

1. If you are looking for a center aligned fixed width, nest the row within a container
2. If you are looking for a full screen but with a tiny left and right padding, nest the row within a container
3. If you are looking for a 100% full screen with no padding, nest the row within a section

This is no rocket science but this is what they do and this is how this should be done!

[IamManchanda]

If i am understanding this.issue and #10141 (comment) issue correctly

For both margin and padding grid we should do,

1. For fixed layouts,

<div class="grid-container-fixed"> <!-- grid container with fixed layout -->
  <div class="grid-x grid-*-*"> <!-- grid row on x-axis, with margin and/or padding grid on x or/and y axis -->
    <div class="cell small-6"></div> <!-- .cell.small-6 -->
    <div class="cell small-6"></div> <!-- .cell.small-6 -->
  </div>
</div>

2. For fluid layouts,

<div class="grid-container-fluid"> <!-- grid container with fluid layout -->
  <div class="grid-x grid-*-*"> <!-- grid row on x-axis, with margin and/or padding grid on x or/and y axis -->
    <div class="cell small-6"></div> <!-- .cell.small-6 -->
    <div class="cell small-6"></div> <!-- .cell.small-6 -->
  </div>
</div>

3. For 100% width layouts (mostly for backgrounds), its simple just dont do anything
   the below code will just work https://codepen.io/IamManchanda/pen/mwxxwj?editors=1100

<div class="grid-x grid-*-*"> <!-- grid row on x-axis, with margin and/or padding grid on x or/and y axis -->
  <div class="cell small-6"></div> <!-- .cell.small-6 -->
  <div class="cell small-6"></div> <!-- .cell.small-6 -->
</div>

---

And, secondly on #10141 (comment)
Just for padding grid (as it can handle that)

For Migrating the .row.column thing,
We should create a new class, maybe grid-padded-cell
and migrate it like this .grid-x.grid-padded-cell

[IamManchanda]

Also Poke 😉 @brettsmason

[brettsmason]

I think we are in a different position to Bootstrap as they use a padding only grid (though never used it so could be wrong!). Dealing with a margin grid has its own quirks (but ultimately quite a few plus points too).

We have a few scenarios to cover - this CodePen should showcase the common layout requirements, at least for me anyway.

Adding a simple overflow-x: hidden on the body works for the times when you dont want gutters on the sides.

@erredeco Its not possible to have the old style self contained row behavior with margin grids because they need negative margin on the grid to work. You can add the class grid-container directly to a grid-x grid-padding-x to imitate the old behavior though. Also removing padding/margin from the last item wont work if you are doing certain layouts or you dont know how many items are going to be in your grid.

@IamManchanda Your summary is pretty much spot on in terms of the use. However the grid-container-padded is almost like a BEM modifier (so would of been grid-container--padded in those circumstances).

I personally think its clear as is, but then I wrote a lot of it 😉 I think its going to be hard to change class names now though as they are out there in use. Open to suggestions though.

@kball Not sure where we go from here to progress this...

[IamManchanda]

@brettsmason on BEM

grid-container--fixed
grid-container--fluid

[brettsmason]

Why the need for fixed/fluid though? 'grid-container' isn't required if you want a full width grid, only if you want it contained (your fixed).

[erredeco]

@brettsmason see my answer to #10318 there is the reason why you can't add grid-container directly to a grid-x grid-padding-x

[erredeco]

I hope you will forgive me for being a bit disputatious, but... to have the same result:
Version 6.3:

<div class="row column">
  <h1>Title</h1>
</div>
<div class="row">
  <div class="small-4 column">Lorem Ipsum</div>
  <div class="small-4 column">Lorem Ipsum</div>
  <div class="small-4 column">Lorem Ipsum</div>
</div>

Version 6.4:

<div class="grid-container">
  <div class="grid-x  grid-padding-x">
    <div class="cell">  
      <h1>Title</h1>
    </div>
  </div>  
</div>

<div class="grid-container">
  <div class="grid-x  grid-padding-x">
    <div class="small-4 cell">Lorem Ipsum</div>
    <div class="small-4 cell">Lorem Ipsum</div>
    <div class="small-4 cell">Lorem Ipsum</div>
  </div>
</div>

Do you see why I feel somehow perplexed?
The good thing is that now you can write:

<div class="grid-container grid-container-padded">
      <h1>Title</h1>
</div>

<div class="grid-container grid-container-padded">
	<div class="grid-x  grid-margin-x">
		<div class="small-4 cell">Lorem Ipsum</div>
		<div class="small-4 cell">Lorem Ipsum</div>
		<div class="small-4 cell">Lorem Ipsum</div>
	</div>
</div>

And have margins instead of paddings, and you'll end up with a bit larger container (1200px instead of 1170)

[IamManchanda]

@erredeco This will work fine too!

<div class="grid-container">
  <div class="grid-x  grid-padding-x">
    <div class="cell">  
      <h1>Title</h1>
    </div>
  </div>  
  <div class="grid-x  grid-padding-x">
    <div class="small-4 cell">Lorem Ipsum</div>
    <div class="small-4 cell">Lorem Ipsum</div>
    <div class="small-4 cell">Lorem Ipsum</div>
  </div>
</div>

[erredeco]

@IamManchanda of course! But for my experience, usually designers want to have an effect like this https://codepen.io/erredeco/pen/NgMjVz so it's better to think of each row as a separate module that contains itself, instead of having a global wrapper :)

[IamManchanda]

I know it sucks but margin grid needs a container wrapper unfortunately

[IamManchanda]

See #9875 (comment)

[erredeco]

It is just difficult to accept that I need all this just to align the title to the container below :(

<div class="grid-container">
  <div class="grid-x  grid-padding-x">
    <div class="cell">  
      <h1>Title</h1>
    </div>
  </div>  
</div>

[IamManchanda]

WIth {Margin Grid} Unfortunately yes!!

[IamManchanda]

For me best bet would be this above ⏫ #10297 (comment)

[erredeco]

you can spare some div just putting:

<div class="grid-container grid-x  grid-padding-x">
   <div class="cell">
        <h1>Title</h1>
   </div> 
</div>

But it is a workaround...

[erredeco]

I still think that another way to go could be to use only a right-margin add a class to the last element that resets the margin. (see susy - @include span(3 last); )
Usually the situation when you don't want to add it by hand is the "gallery" when you have an unknown number of elements, but they are all identical, and I think you can calculate the correct margins with math

[erredeco]

my last 2 cents for tonight (BTW: nice discussion, I appreciate it) I am sure you have already noticed that the padding-grid and the margin-grid are misaligned (See: https://codepen.io/erredeco/pen/pwVreN)
I asked myself what happens when I get rid of the negative margins.
Answer: https://codepen.io/erredeco/pen/RgyZxm

[brettsmason]

@erredeco Sorry for the delay in replying! Agreed - great discussion and the feedback is definately valued 😄

I was a long time Susy user and am quite familiar with it. I dont think we are really in a position to make such a potentially huge change to the grid when we have just launched it, though I understand where you are coming from. I think reliance on first/last for CSS users would be too alienating for the core audience though who are used to the freedom a split gutter approach brings.

Regarding the padding/margin grid not aligning - I'd say they have different use cases and generally wouldnt be used together, but I could be completely wrong. Its still very early in the grids life cycle so feedback is still coming in slowly. The benefit of a margin grid is the margin is a "true" gutter, in that its a proper space rather than like the old padding was. I guess we could potentially add a class/way to 0 the negative margins on the grid for folks who dont want that behaviour though. What do you think?

Having said all that I'm going to look into potentially better ways me could handle this.

[erredeco]

Thank you for your feedback; IMHO there are 2 priorities

1. resolve the issue reported here (I admit I went a bit off-topic) :) and I fear that the only option would be adding a container-full or container-fluid wrapping div (sigh)
2. see if it could be possible to simplify the markup in case you have only 1 content that spans 12 cells
   <div class="grid-container standalone"> I dunno....

[jashwant]

@stuartstarrs ,

Your issue will be fixed by adding grid-container-padded class to grid-container.
https://codepen.io/jashwant/pen/owPWgd.

However, I do think that there should be a padding in grid-container by default ( without adding grid-container-class.

[brettsmason]

So I've been taking all the feedback on board and have been experimenting with possible solutions with the grid container. Take a look at this CodePen for what I'm thinking: https://codepen.io/brettsmason/pen/zzLQbm

If everyone could take a look and give me feedback that would be appreciated. Then I can put a PR together to solve this. To confirm, this would include removing the current grid-container-padded implementation.

[stuartstarrs]

@brettsmason just not clear on one thing. Are you suggesting grid-container-padded be incorporated into grid-container as its default behavior?

[brettsmason]

@stuartstarrs Yeah exactly that. This has the benefit of working with the margin grid (plus applying negative margin to padding grids by default too) so that both line up correctly as per the comment made by @erredeco.

The only other thing I'm not 100% clear on is the grid-container-full behaviour. I want to be able to have full width content with no gutters on the sides, and I think use of overflow (like in the CodePen) is going to be the only way.

What do you think of the approach?

[jashwant]

@brettsmason .

I think, overflow: hidden should be overflow-x: hidden in grid-container--full.
Also, if we are going to use BEM, there should be a way to use custom class names for grid elements.

Everything else is great. Excellent work.

[brettsmason]

@jashwant re BEM - sorry that was habbit after having discussed with @IamManchanda (Foundation 7 architecture changes). It will probably be grid-container-full (or maybe just full - not sure yet) until Foundation 7.

The overflow is a tricky one - if you look at the last demo I have set grid-margin-y too. If I only set overflow-x: hidden then there is a scrollbar for the grid - give it a try and you will see what I mean.

[IamManchanda]

Sorry going off topic, but
@brettsmason On BEM/BEMIT, Do you think this is the right time to create a new thread regarding the same for feedback and knowing people on whether they will love 😍 it or hate it ?

[brettsmason]

@IamManchanda Yep definitely a good idea. Maybe give examples of possible class naming structures.

[stuartstarrs]

@brettsmason I agree with everything you said originally and more so after seeing all these examples. Since posting this I'm using padding gutters for everything anyway (so far) and as of now I'd probably apply hidden overflow to body as per your original suggestion.

With your latest examples and the -full -fluid concepts – I'm trying to think of pros and cons.

The biggest pro is that we get to have the use case of margin-grids right up to the sides of the viewport with space around the cells (your Margin grid - fluid container) and without (Margin grid - flush container). I don't know if I'd use these, but, I'd be expecting these to "just work" if I just starting using F6.4 for the first time today. Ideally gutters probably should be margins over paddings as margins are for separating elements and paddings should be internal to elements. Which is why the I thought grid-x was so great.

The cons (con?) is the overflow hidden on the grid container. I think there's going to be lots of people placing things inside a container that need to overflow. I can image a page with rows of containers with elements with negative margins or things that are supposed to overflow not working. Or even just a Foundation Dropdown-menu tucked into the button of a container is going to have all the submenus hidden.

I'm really wondering if the grid should be kept exactly as it is and instead some fancy way of more clearly documenting this behavior could found?

I think, possibly, my vote is for grid-container to have the padding in it by default as you've done in these examples, for there to be no -fluid or -full and for people to create their own version of -full by manually removing the padding on the specific occasions they want to. Maybe there's issues with that I'm not considering.

It's all very subjective, I think, more about what appears to make sense rather than the technical issue I thought it was.

[IamManchanda]

@brettsmason @kball @ALL others
What i simply want us to fetch from bootstrap is simply this
https://codepen.io/IamManchanda/pen/rwZqjw?editors=1000

(Please resize the window for responsive)

I upvote for grid-container--full too apart from these two!

[brettsmason]

@stuartstarrs Thanks that was a really helpful response!

Completely agree with your thoughts on margin vs padding for gutters - its why we were keen to implement margin in the first place.

I also agree on the use of overflow on the container, I dont think we can realistically do it as its going to cause issues down the road like you pointed out, so I will remove this.

Therefore I'm thinking we can offer the 3 classes: grid-container, grid-container-fluid and grid-container-full (or flush?). However there would have to be a callout next to the full class to say that you need to use overflow-x: hidden on your body in order to use that class correctly.

I definately want to keep the full class, as I do use that behaviour quite a lot so this would be nice for me personally.

What do you think?

@IamManchanda I'm sorry but I really dont see the need for the fixed (bootstraps default container) compared to our base grid-container behaviour. This reminds me of the whole adaptive vs responsive style websites from years ago, but I cant remember the last time I saw adaptive 😄

[erredeco]

so, just to recap:
grid-container means fixed max-width
grid-container-fluid means width 100% but with left and right padding
grid-container-full means width 100% but with no padding, so truly 100% of space is occupied.
The last one seems to me a bit extreme scenario, could you give me an example where it should be used?

[brettsmason]

@erredeco Yep thats correct. A good example of grid-container-full is in the CodePen - a full width gallery, bleeding to the edges (I had to do this on a recent project). I'm sure there are other use cases too, but I cant think off the top of my head 😄

Do they solve your issues? You can now simply place an element within grid-container to mimic the old row column behaviour - no need to wrap it in a cell.

[jashwant]

Now, I agree that there won't be any need for container-fixed ( container in bootstrap ).

@erredeco, grid-container-full can be used in masonry layout.
Images taking all the space ( they may or may not have gutter ).

[stuartstarrs]

@brettsmason I think that would straight up allow me to use grid-margin-x instead of any padding on all occasions where I'm not applying background colours to cells and not give it a second thought. I do agree with everyone who's mentioned that even needing a container is horribly sad though. And good luck making this clear in the docs!

@erredeco the -full is the very very first thing I wanted to do with the XY Grid! Hence this issue. (That said, I wanted to use grid-margin-x and grid-padding-x in conjunction as an XY Grid holy grail, but the same issues apply.)

[IamManchanda]

Usecase for flush container if anybody wants one
https://codepen.io/IamManchanda/pen/qjMLYm

<div class="small-6 cell" style="background-color: #2695FB;"></div>
<div class="small-6 cell" style="background-color: #0079E9;"></div>

[brettsmason]

I have put together a PR to address what we have discussed: #10371
@stuartstarrs @jashwant @erredeco (and anyone else) I'd really appreciate it you were able to give the PR a test to confirm this behaviour suits everyone and is the right step forward.

[jashwant]

@brettsmason ,
PR works on all the use cases I had.

Is there any unusual side effect on overflow-x: hidden on body ?

[erredeco]

@brettsmason your codepen seems ok; I forked it and did some other tests (https://codepen.io/erredeco/pen/zzmxXo) - the "Padding grid - flush container" case was absent-
The overflow-x:hidden on body is a bit strange for me (but I understand why it is necessary)
I hope that it will not have any nasty side effects I can't see by now.
I agree that the grid-container should always have padding, without a specific class for it, as it seems it is always needed.

[erredeco]

I am not currenty able to make your PR work, sorry

[stuartstarrs]

@brettsmason I think this looks OK. I think having -full and -fluid is slightly easier to grasp and I like how paddings and margins now line up and are completely hot swappable.

So the only issue now is the possibility in some cases of having to rely on overflow-x hidden on body. I've been trying to wrack my brain for how that could be a problem and I'm not coming up with anything. The body is the viewport limit in all cases that I can think of.

[jashwant]

Also, in near future, instead of negative margins on grid, we should target :first and/or :last cell to align.
However, I agree that this should be included in Foundation 6.

[erredeco]

@jashwant the problem with and :first and :last is that it is plenty of scenarios where you don't actually know how many elements will fit in a container. It works great if you have only one row of elements or when they have all the same width.

[kball]

Going to close this based on #10371

This will go out in a 6.4.2, currently targeting next Monday.