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

remove -ve margin right #168

Open
fgfmichael opened this issue Mar 5, 2021 · 2 comments
Open

remove -ve margin right #168

fgfmichael opened this issue Mar 5, 2021 · 2 comments

Comments

@fgfmichael
Copy link

Hi People!

I just had an issue where I had a few layers of grid embeded inside of each other which were at full width. Due to how -ve margin works differently on left vs right due to just how browsers work I was stuck as I had a nasty overflow to the right of my application.

Whiles looking for a solution I came accross this blog post: https://www.rawkblog.com/2019/01/flexbox-gutters-negative-margins-solved/
(check out the Making Negative Margins Safer section)

This blog post basically explained one downside to the -ve margin solution for gutters which was my exact problem:

In a left-to-right website, left margins won’t create overflow. They’ll just push harmlessly into the empty space outside of the browser. So instead of using margins all around, we can be more restrictive:

Basically, if instead of having gutter implemented via padding left/right as 1/2 gutter width and outer margin being left right as -1 * 1/2 gutter you instead make padding left full gutter and margin right -1 * full gutter.

To test this out I created the following shims around the v6 of this library:

export const MarginLeftRow = (props) => (
  <Row 
    {...props} 
    style={{
      ...(props.style || {}),
      marginLeft: "-30px",
      marginRight: "0",
    }} 
  />
);

export const PaddingLeftCol = (props) => (
  <Col 
    {...props} 
    style={{
      ...(props.style || {}),
      paddingLeft: "30px",
      paddingRight: "0",
    }} 
  />
);

This I believe would be roughly equivilent to changing

paddingLeft: gutterWidth / 2,
(please correct me if I'm wrong).

Now I have no overflow AND everything still looks the same (so far, I'm yet to notice any issues).

My question is:
Has this ever been considered for the library? I feel like I couldn't have been the only person to discover this issue and I don't believe that the community as a whole would just be accepting of the easy answer which is to set overflow-x: hidden on the Row. If this has been concidered why did it fail? If not would this be open for implementation via PR? If so what are the quality controls I should use to ensure my changes don't break everyone else??

Cheers
Michael.

@fgfmichael
Copy link
Author

NOTE: if we do implement this change we would need to add additional configuration that allowed users to specify if the browser is left-right or right-left if my understanding of the blog post is correct, otherwise this might break on right to left websites. Would need some follow up from somebody who knew about those kinda websites.

@fgfmichael
Copy link
Author

it appears that if you set direction: rtl at the body of the document that the issue mentioned in the blog post occurs. I tested swapping out margin/padding from left to right and it fixed, so this would likely force a major release to v8 as we would need to add configuration + its effectively changing out the entire gutter system with 4 lines of code.

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

No branches or pull requests

1 participant