Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XY Grid uncollapse classes #10302

Closed
marvinhuebner opened this issue Jun 30, 2017 · 19 comments
Closed

XY Grid uncollapse classes #10302

marvinhuebner opened this issue Jun 30, 2017 · 19 comments

Comments

@marvinhuebner
Copy link

With the new XY-Grid it is not possible to uncollapse a grid at a certain breakpoint.

If i want a collapse my grid on small and a uncollapse my grid on medium i have no possibility (unless i write my own scss for this) to uncollapse the grid.

Would be really helpful to have these utility classes available.

When we could do something like this:

<div class="grid-x grid-padding-x small-padding-collapse medium-padding-uncollapse">
  <nav>
    ...
  </nav>
</div>
@kball
Copy link
Contributor

kball commented Jun 30, 2017

cc @brettsmason

@IamManchanda
Copy link
Contributor

Just a note. If you thinking on doing this, we need to refactor the responsive code with binding into a variable just like how i did with prototyping and flexbox utilities!

@brettsmason
Copy link
Contributor

@kball I think this needs to be a Zurb decision if you want this included or not, as I think you know my view on collapsing/uncollapsing at every breakpoint 😄

It adds a lot of bloat, and If you really need to do stuff like that then mixins should be the way to go. Just my personal opinion though and its felt that it should be included then fair enough.

@marvinhuebner
Copy link
Author

It adds a lot of bloat, and If you really need to do stuff like that then mixins should be the way to go. Just my personal opinion though and its felt that it should be included then fair enough.

The extra bloat is a good point.

As i mentioned above this could easily achieved with some own css at the actual use point there it's needed.

But I'm a little bit confused, why there are $breakpoint-padding/margin-collapse classes available and the uncollapse classes not? (Beside the extra bloat)

@brettsmason
Copy link
Contributor

@marvinhuebner Sorry for the late reply on this it slipped off my radar!

Basically through the development of XY grid we came to the conclusion that the base grid should have no gutters (margin/padding) and the user could then make a choice what they wanted to include. You then also have the choice to collapse that added margin/padding if you choose to. We didn't think adding ways for users to add/remove padding at every breakpoint would be an efficient use of the code (easy with padding but margin it gets very heavy!).

@kball What do you think on this? I still find it very hard to justify the inclusion of responsive collapse/uncollapse classes due to the code bloat.

@IamManchanda
Copy link
Contributor

If we can do this below... I dont think it as a big code bloat!

$responsive-collapse-breakpoints: false;

@marvinhuebner
Copy link
Author

@kball What do you think on this? I still find it very hard to justify the inclusion of responsive collapse/uncollapse classes due to the code bloat.

I fully understand the code bloat view. Originally i opened this issue because i was confused about the feature for collapse cells but not uncollapse cells.

The idea from @IamManchanda could be a good approach, but i don't know exactly how this could work for the compiled version.

But for somebody who does not use the sass version, he has always the code bloat. On the other hand the question could be if someone who uses a non customize version from foundation does pay attention if the css file is 1kb larger or not. But thats only my personal point of view.

@brettsmason
Copy link
Contributor

@marvinhuebner We wouldn't actually need a new variable to include/exclude it, as we use this for the main class generating mixin at the moment: https://github.com/zurb/foundation-sites/blob/develop/scss/xy-grid/_classes.scss#L416 - a new variable could be added so it could easily be excluded.

@kball and I did quite a lot of work on getting the file size down during development, as the margin based grid generates a hell of a lot of extra code (mainly the responsive gutters 😢). I'm pretty sure generating a uncollapse/collapse version at each breakpoint would add a large chunk to the generated CSS. If we were only talking a few kb then I'd have no issue adding it at all, but I think we are likely to be in the 10s of kbs (would need to test though).

Saying all that I'd be up for adding it if there's people that want it and the file size increase isn't too ridiculous.

@Luckyfella73
Copy link

Hi,

using the flex-grid I tried small-collapse and medium-uncollapse without success. The class .medium-uncollapse is not generated (scss). This discussion here is about XY-Grid - but does it mean that the flex-grid as well has no breakpoint-uncollapse classes? In my generated css file I only find the breakpoint-collapse classes but no uncollapse classes.

The docs for flexgrid states that you can use for example small-collapse and medium-uncollapse in a row - if that is not the case the docs should be corrected.

Im using foundation 6.4.0 btw.

@brettsmason
Copy link
Contributor

@Luckyfella73 How are you using Foundation? Downloaded it via the customizer/Sass/Zurb stack etc?
Basically XY grid is the default with 6.4, so if you want to use the old flex grid you need to first enable it and disable the XY grid. If you can let me know how you are using it I can help advise.

@Luckyfella73
Copy link

@brettsmason I use SASS version of Foundation as a Node module and webpack to compile everything. Here are all foundation includes I have active:
@include foundation-global-styles; @include foundation-flex-grid; @include foundation-flex-classes; @include foundation-visibility-classes; @include foundation-float-classes;
all others are commented out.

I created a custom css class that helps me to set padding for the column a breakpoint small but of course it would be nicer to just use the classes
small-collapse and medium-uncollapse in the row (div)

All other flex-grid functions/classes work fine so far, it's just the collapse problem I have at the moment.

@brettsmason
Copy link
Contributor

@Luckyfella73 I just tried with a fresh version of 6.4.1 using NPM with @include foundation-flex-grid and I get all of the correct classes as I should. Not sure what to suggest without more details...

@Luckyfella73
Copy link

@brettsmason Thank you for taking your time, I really appreciate it! I Updated my node foundation to 6.4.1 but still get no class generated with the name of .medium-uncollapsed in the final css file. Are you sure you get it in the final css that is compiled with sass? If you get that class something in my webpack stuff has to produce that problem I guess. But anyhow thank you for trying to help.
Could you check if I do include all needed foundation modules?

@include foundation-global-styles;
@include foundation-flex-grid;
@include foundation-flex-classes;
@include foundation-visibility-classes;
@include foundation-float-classes;

@brettsmason
Copy link
Contributor

@Luckyfella73 I just cloned a fresh copy of the Foundation Zurb Stack to test this.
I actually had the same problem so have dug into it some more. The medium/large collapse classes are receiving the small prefix. I've tracked it down to this commit (annoyingly my commit!).

Removing this seems to fix the class generation. @kball can you remember if this was ever used in the end for anything? I know it was to do with xy-cell but cant remember details why. We need to get this in the 6.4.2 version too.

@JarvisH
Copy link

JarvisH commented Jul 31, 2017

I also would like some option to control collapsing of columns based on breakpoints.

What about adding breakpoint "-only-" rules instead of" uncollapse" rules and using those rules with media-queries? E.g. 'small-only-margin-collapse'.

We could then switch between the rules depending on requirements which would even save characters in our html code, e.g: "small-only-margin-collapse" instead of "small-margin-collapse medium-margin-uncollapse"

I just did a quick test, and the difference is ~10kb uncompressed in the generated css.

@Proper-Job
Copy link
Contributor

I would like to see this feature for the simple reason that it was possible with FlexGrid.

@colin-marshall
Copy link
Contributor

This is a feature request and we aren't adding anything to Foundation 6, however there seems to be a decent amount of people that want this.

@ncoden is this worth looking into for F6 or should we close and revisit for F7?

@ncoden
Copy link
Contributor

ncoden commented Mar 9, 2018

The issue pointed out by @brettsmason above was fixed in #10491

@colin-marshall
Copy link
Contributor

@ncoden is that the same as what is being requested in the first post?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants