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

[Grid] Uncentering columns breaks their float #9952

Closed
andycochran opened this issue Apr 13, 2017 · 8 comments
Closed

[Grid] Uncentering columns breaks their float #9952

andycochran opened this issue Apr 13, 2017 · 8 comments

Comments

@andycochran
Copy link
Contributor

How to replicate:

  • At the medium breakpoint, two columns that total > 12 and are both centered
  • At the large breakpoint, these columns total ≤ 12 and are both uncentered
  • The .large-uncentered class does not allow columns to float, as the .medium-centered class still applies float:none; and clear:both;

https://codepen.io/andycochran/pen/mmyyzj

@JeremyEnglert
Copy link

Here is a super rough way of fixing this, although, we may be able to make this a bit more generic so it applies to all of these types of instances. I'll give it a shot if we think this is a good path to go down.

@include breakpoint(large) {
	.medium-centered {
		&.large-uncentered {
			float: left;
			clear: none;
			&:last-child:not(:first-child) {
				float: right;
				clear: none;
			}
		}
	}
}

@JeremyEnglert
Copy link

This is probably a bit better.

.small-centered {
	@include breakpoint(medium) {
		&.medium-uncentered {
			float: left;
			clear: none;
			&:last-child:not(:first-child) {
				float: right;
				clear: none;
			}
		}
	}
	@include breakpoint(large) {
		&.large-uncentered {
			float: left;
			clear: none;
			&:last-child:not(:first-child) {
				float: right;
				clear: none;
			}
		}
	}	
}

.medium-centered {
	@include breakpoint(large) {
		&.large-uncentered {
			float: left;
			clear: none;
			&:last-child:not(:first-child) {
				float: right;
				clear: none;
			}
		}
	}
}

@kball
Copy link
Contributor

kball commented Apr 18, 2017

Sigh... feels like we're going down the specificity rabbit hole... @ncoden do you have any better ideas for how to address this?

@andycochran
Copy link
Contributor Author

specificity rabbit hole

^ Yup. I'm wondering if we should just live with this and be better with specificity in v7.

@ncoden
Copy link
Contributor

ncoden commented Mar 28, 2018

Here's what's funny with specificity. You're never resolving problems, just moving them elsewhere, deeper, making them even more difficult to resolve.

Take a look at #8990.

We had a trouble with float: none being overriden by something else, we added :last-child... to increase the speficity, and now we have to deal with it there.

We used :last-child:not(:first-child) at first on .column to make the last column aligned to the right (or the opposite direction). See 6ba98e9 $grid-column-align-edge. And we should not have. You want the last column to be pulled to the right ? Use float-right on it, no need for a global setting for that.

This is the best solution according to me to avoid all little "specificity bugs" everywhere in usage cases. Keep the specificity low and manually apply classes on HTML tags to modify their behavior instead of relying and global settings (like here) or their parent.

So let's deprecate $grid-column-align-edge and recommand the usage of .float-right instead. What do you think @andycochran @SassNinja ?

@andycochran
Copy link
Contributor Author

@ncoden That plan works for me.

@ncoden ncoden self-assigned this Mar 28, 2018
@ncoden
Copy link
Contributor

ncoden commented Mar 30, 2018

Hmmm... $grid-column-align-edge is true by default. Removing it can be a breaking change.
See https://foundation.zurb.com/sites/docs/grid.html#incomplete-rows

In order to work around browsers' different rounding behaviors, Foundation will float the last column in a row to the right so the edge aligns. (...)

So this is not only about incomplete grid alignement, but also to "hide" in the gutter some browsers' precision bug.

I found f014bcc that added back this behavior but I don't understand the meaning of the commit message and there is no previous commits.

I guess this restore a Grid behavior from Foundation 5 but I'm not sure. I have to find which browsers handle grid precisions differently (IE & Edge maybe, this seems related to #10808 and #10978 could interfere too) to know if we can (or not) remove this behavior.

@ncoden
Copy link
Contributor

ncoden commented Mar 31, 2018

Here are the results of my research so far. This precision bug that makes rows in a column not taking the full width (like 99.98%) and being aligned on the right have two causes:

  • We use the Sass maximum 5-digit precision, and this is not enough. Above 1200px the bug become visible, some rows width are 1px less.
  • On IE and Edge < 14, all values are truncated at 2 digits after decimal. So it's like if we had a 2-digit precision and the bug appear far more often. See XY-Grid Container doesn't fit next to each other in Edge #10808

I checked how Boostrap works to try to understand how important this is. BS 4 is based on flex grid only (so in most case a row will take all the availaible space), and BS does not resolve this bug (it can be reproduced in their documentation).

So now I understand why this "feature" is enabled by default on all columns. In complete rows this is actually a fix. So I don't like to say that but we may have to keep this behavior and go for the specificity fix....

ncoden added a commit to ncoden/foundation-sites that referenced this issue Mar 31, 2018
@ncoden ncoden added the PR open label Mar 31, 2018
ncoden added a commit that referenced this issue Apr 5, 2018
…column-9952

fix: fix uncentered last column on Float Grid #9952
ncoden added a commit to ncoden/foundation-sites that referenced this issue Jun 16, 2018
…ed-last-column-9952 for v6.5.0

50e7dab fix: fix uncenter for last column in Float Grid foundation#9952
fa07c03 refactor: add "default" position to `grid-column-positioning` to handle float position
9429539 tests: add visual test for Float Grid uncentered columns
a8f850e fix: fix last column uncentering with edge align disabled in Float Grid

Signed-off-by: Nicolas Coden <[email protected]>
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

4 participants