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

fix: fix cell widths overridden by readjusted smaller breakpoints in XY Grid #10468 #11117

Merged

Conversation

ncoden
Copy link
Contributor

@ncoden ncoden commented Apr 1, 2018

Context

With margin gutters, a cell width is defined with its gutter. This is why we have to readjust all sizes and not only .cell when applying the new gutter in a bigger breakpoint. This must include all sizes for smaller breakpoints too. Before this commit there was an optimization that prevent gutters for smaller breakpoints without a new gutter defined to be regenerated. This can seems logical since no gutter was generated for this breakpoint so there is nothing to readjust.

Bug

Because the gutter is defined with the width/height, the width/height of readjusted gutters (with a custom gutter) overrides the width/height of non-readjusted gutters (without a custom gutter) even if they are from a bigger breakpoint.

Changes:

  • Readjust gutters for all breakpoints even without new gutter defined
  • Add explainaitions

There is no impact on the bundle foundation.css size because there is no breakpoint (outside of the last one) defined without a custom gutter. There can be a minimal impact on a user CSS size if ze uses a lot of custom breakpoints.

Note: there a reason why grid system always used paddings for grid system: it does not impact the width. All troubles (and a lot of code duplication and specificity issues) we have with the XY Grid comes from that. And apart from redicing by 1 the number of nested HTML tags in a complex grid layout, I don't see a single advantage of margin gutters. @brettsmason @kball Do you have an opinion on this ?

Closes #10468

…XY Grid foundation#10468

With margin gutters, a cell width is defined with its gutter. This is why we have to readjust all sizes and not only `.cell` when applying the new gutter in a bigger breakpoint. This must include all sizes for smaller breakpoints too.

Before this commit there was an optimization that prevent gutters for smaller breakpoints without a new gutter defined to be regenerated. This can seems logical since no gutter was generated for this breakpoint so there is nothing to readjust.

However, because the gutter is defined _with the width/height_, the readjusted width/height of readjusted gutters (with a custom gutter) overrides the width/height of bigger non-readjusted (without a custom gutter) breakpoint.

Changes:
* Readjust gutters for all breakpoints even without new gutter defined
* Add explainaitions

Closes foundation#10468
Copy link
Contributor

@DanielRuf DanielRuf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@ncoden ncoden merged commit 9310c7b into foundation:develop Apr 3, 2018
@ncoden ncoden deleted the fix/xy-grid-cell-width-overridden-10468 branch April 7, 2018 17:46
ncoden added a commit to ncoden/foundation-sites that referenced this pull request Jun 16, 2018
…overridden-10468 for v6.5.0

2b5aaa7 fix: fix cell widths overridden by readjusted smaller breakpoints in XY Grid foundation#10468

Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants