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

Using margin instead of padding for item (column) gutter #42

Open
mmjaeger opened this issue Jun 1, 2016 · 16 comments
Open

Using margin instead of padding for item (column) gutter #42

mmjaeger opened this issue Jun 1, 2016 · 16 comments

Comments

@mmjaeger
Copy link

mmjaeger commented Jun 1, 2016

Hello
I was wondering how difficult it would be to use margin-left/margin-right for the column gutter? would be nice if there would be an option to choose either one? is it possible to have a collapsed grid with no gutter at all?

@fspoettel
Copy link
Contributor

You can use @include g(full) for a grid without gutters. Regarding the margins, the current system of negative margin-left and positive padding-left was written for the fallback-grid present prior to v3.x. It might be a good idea to get rid of that approach and base the grid solely on margin-left. I will update this from a PR when I arrive at something

@mmjaeger
Copy link
Author

mmjaeger commented Jun 2, 2016

Sounds good - thanks

@fspoettel
Copy link
Contributor

fspoettel commented Jun 2, 2016

So the problem with this is that margin gets added to the outer-width of a box and thereby making it impossible to declare width: 50% like it is done now without breaking the grid.

The solution to this is using calc to subtract the margin from the width dynamically, e.g width: calc(50% - 1.5rem) . This is something that was not possible with the fallback-grid as it is only supported in Android 4.4+ and the fallback-grid supported browsers all the way back to IE8. But since flexbox rocks about the same browser support as calc and this solves a couple of painpoints of the grid system, I will add this in v4.0. It requires some significant rewrites though. @renatocarvalho are you still using hagrid? what is your opinion on this?

Thanks for bringing it up @mmjaeger. if you want to help, the code is pretty well documented via SASSDoc and Stylelint. Feel free to send a PR anytime :)

@fspoettel fspoettel added this to the v4.0 milestone Jun 2, 2016
@mmjaeger
Copy link
Author

any progress on the margin gutter yet

@fspoettel
Copy link
Contributor

not yet, i have a lot of other stuff on my plate and the change as a whole is non-trivial. if you want to take a stab at it, feel free to do so. that being said, i will probably get to it soon :)

@mmjaeger
Copy link
Author

Looking forward - by the way, how would I set up a collapsed grid (no gutter)

@fspoettel
Copy link
Contributor

fspoettel commented Jul 22, 2016

Assuming you have this markup (and no custom $hagrid-item-selector set, it defaults to > * inside the selector you called @include g on):

<section class="items">
  <article class="item">Item 1</article>
  <article class="item">Item 2</article>
  <article class="item">Item 3</article>
</section>

You would get a grid based on thirds without gutters like this:

.items {
  @include g(full);
}

.item {
  @include i(1/3);
}

@mmjaeger
Copy link
Author

mmjaeger commented Jul 22, 2016

I think I got a little further with the margin gutter - I set a global var
like $hagrid-has-gutter: false !global;

in the functions.scss file I changed the code to:

// * If a general width is passed in, use it
      @if type-of($width) == number {
        @if $hagrid-has-gutter {
          #{$prop}: calc( #{calculate-width($width)} -
#{$hagrid-has-gutter};
        } @else {
            #{$prop}: calculate-width($width);
        }

        $width-detected: true;

      }

the $hagrid-has-gutter var gets set in the modifier.scss file:
@if $is-not-spacing == true {
    $hagrid-has-gutter: $gutter !global;
    margin-right: halve(-$gutter);
    margin-left: halve(-$gutter);

    #{$hagrid-item-selector} {

        margin-right: halve($gutter);
        margin-left: halve($gutter);

    }

  }

seems to work - have not check push and pull but I will later - it would be
nice though if we could choose between padding gutter and margin gutter and
if we use margin gutter, being able to add some padding to the item content.

On Fri, Jul 22, 2016 at 6:56 AM, Felix Spöttel notifications@github.com
wrote:

Assuming you have this markup (and no custom $hagrid-item-selector set):

Item 1 Item 2 Item 3

You would get a grid based on thirds without gutters like this:

.items {
@include g(full);
}
.item {
@include i(1/3);
}


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#42 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABe4lBEEDyZKZ55ij8ZQXrxX5lzaJHXIks5qYMv4gaJpZM4IsD_6
.

Marco M. Jaeger
http://net4visions.com

@fspoettel
Copy link
Contributor

nice work @mmjaeger, I will take a shot at it this weekend and link you in the PR in case you want to review it. The only open question is how to use the $hagrid-gutters-map with the change. Modifiers will no longer work for setting gutters because the grid-calc function called on the item will need to have information on the gutter-size to be able to set calc. One solution could be to add support for optional gutters per breakpoint, meaning that one could do @include i(1/3, 2/3 md narrow, 1/4 lg wide)). If none is set default will be used. The full-modifier could be renamed to collapse to further clear up the separation. I will add a hagrid-gutter-type variable for padding and margin as the new structure allows to interchange them.

@mmjaeger
Copy link
Author

mmjaeger commented Jul 22, 2016

I think I got the different gutter sizes working with grid-calc. how about
adding some vertical gutter to the columns like margin-bottom: $gutter; if
you let me know your email address I can send you the updated files.

@mmjaeger
Copy link
Author

Played a little bit with the code - the difficulty actually is where to set the global variable for the gutter and where to reset it.

@fspoettel
Copy link
Contributor

fspoettel commented Jul 22, 2016

Thanks for the input! I'm leaning towards solving this via the item mixin as it would enable fine-grained configuration (setting wide gutters for big screens only), does not require another config var set and is consistent with the rest of the library's api. The downside is that it would require another layer of string-parsing (@i() already looks for fractions, units, auto and breakpoints.
I'm curious how you solved the problem though and would really appreciate a pull request with your changes.

@mmjaeger
Copy link
Author

mmjaeger commented Jul 22, 2016

I'm actually not familiar with github - I know it's embarrassing but I'll
give it a try.

as to the gutters I actually like that you're applying the gutter to the
items when setting the container - if you put the gutter on the item mixin
how do you make sure that the compensation on the row matches

@fspoettel fspoettel mentioned this issue Jul 28, 2016
3 tasks
@renatocarvalho
Copy link
Member

renatocarvalho commented Jan 6, 2017

@renatocarvalho are you still using hagrid? what is your opinion on this?

We use Hagrid on all projects here ❤️ 🙂.

I'm ok with going forward and removing support for old browsers. Developers who need to support older browsers could still use an older version of Hagrid, right?

@fspoettel
Copy link
Contributor

@renatocarvalho that is very nice to hear, glad that it is useful! yes, old versions will be available via npm/yarn. I will set aside some time to make v4.0 happen soon, been pretty swamped with work lately

@renatocarvalho
Copy link
Member

Let me know if you need any help. Even on the landing page. I think Hagrid deserves to be known by more people; it's really awesome!

@fspoettel fspoettel removed their assignment Feb 9, 2022
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

3 participants