Consider new clearfix methods

issue persists in HEAD

attached patch changes "." to "\0020"

Bump and +1.

The problem with the current clearfix is the generated "." has font size and line height, creating an unexpected space beneath a clearfixed element under certain conditions (for example, when a clearfixed element with no bottom margin, padding, or border touches the bottom of the page).

Possible fixes include adding font-size:0; line-height:0;, or the more widely approved height:0; overflow:hidden;. In my experience, when using overflow:hidden; there is also no need to use visibility:hidden;.

If we are interested in moving farther forward, there is a compact method at http://nicolasgallagher.com/micro-clearfix-hack/ which also addresses the inconsistency where a clearfixed element's top margins collapse in modern browsers, but behaving like floated objects and do not collapse in ie6 and 7. I personally am not interested in implementing this -- many themes including my own take advantage of the margin collapse and I think most themers are more interested in clearing floats than visual consistency with soon to be irrelevant legacy browsers.

Paul Irish has adopted parts of the Gallagher clearfix above. His recommendations can be found at https://github.com/paulirish/html5-boilerplate/blob/master/css/style.css....

At the very least, let's add overflow:hidden; right away.

BTW, there is a parallel Drupal 8 discussion at #1051464: change full stop to space in .clearfix rule & improve,

#2: 961876-clearfix-1.patch queued for re-testing.

#1051464: change full stop to space in .clearfix rule & improve has been marked as duplicate. Also contained patches, including further changes; as partially outlined by @RobW in #3, and comments on this issue provide more technical insight/evidence than on the other issue.

I don't support overflow: hidden;. That would make dropdown menus, e.g. contextual links and the like impossible. What's wrong @nimbupani's fix? I think we should test and shoot for that.

The patch in #2 needs to revise the comment that credits the perishable press as a source when we make this change.

I think most themers are more interested in clearing floats than visual consistency with soon to be irrelevant legacy browsers.

Not sure I totally agree here. For most themers, I think clearfix is something of a mysterious black box - I think we' do well to make our mysterious black box as visually consistent as possible. When a margin isn't collapsed, they can see and account for that in their css. I reckon most themers (myself included) generally work that way, with the "write some css, see if it's accomplished what you want, edit, rinse, repeat" methodology... I'd rather deal with it then than later check in IE7 and find things look different.

As for soon to be irrelevant - I imaging there'll be Drupal themers out there who'll have to support IE7 for a while to come, probably throughout the D8 life cycle. Maybe I'm wrong.

Oh, and subcribe :o)

Chiming back in, but don't have my references with me. Hopefully this is all correct as I remember it.

@Jacine: I believe only changing the clearfix to a space does not fix the issue in all browsers. IIRC, Chrome will still show the extra space at the bottom of the page without overflow:hidden or font-size/line-height:0.

Overflow hidden is only applied to the generated :after element, so any dropdowns or content that moves outside the clearfixed element will still display fine. I lean toward overflow:hidden because of the inconsistencies in how browsers handle height:0 and font-size:0 -- I haven't personally found problems with font-size/line-height:0, but it feels like playing with fire to me.

@Steve: I hear what you're saying, but respectfully disagree. To expand a bit:

• Clearfix is for clearing floats. You should be able to apply it to any block level element and expect the result to be the floats are cleared and nothing else.
• Applying almost any clearfix in ie6 and ie7 causes the cleared element to display as if it has been floated (i.e., the margins no longer collapse). I would go as far as saying this behavior is really a bug, since in ie8+ and all (more) standards compliant browsers the margins act as they would without clearfix attached.
• There is consensus that ie6 support should be dropped in D8, and discussion of dropping ie7 (reasons given include the upgrade path is much easier for ie7, and it's dwindling pretty fast on its own). See #308865: Drop IE6 support in Drupal core.

So I think our choice is to either add a behavior that is not part of clearfix's explicit purpose (margins behave differently than normal, don't collapse) in the interest of consistency with browsers that are being phased out by core, or to have to write some ie6 and 7 specific css. Either way we are asking themers to compensate for something unexpected. The latter choice I believe is the better one: it looks to the future, lets themers take advantage of expected behavior in more browsers, and fits with most developer's workflow of coding for standards compliant browsers and then fixing inconsistencies in edge cases.

[Edit] To avoid confusion, when I mention overflow:hidden I'm referring to using it on the generated :after element, not the clearfixed element. Moot point now, see the Compass clearfix in #10.

I wonder why we're not using one of the Unicode separator characters?

U+2028	
	e2 80 a8	
	
	LINE SEPARATOR
U+2029	
	e2 80 a9	
	
	PARAGRAPH SEPARATOR
U+2063	⁣	e2 81 a3	⁣	⁣	INVISIBLE SEPARATOR

See http://www.utf8-chartable.de/unicode-utf8-table.pl

@RobW Ah, I didn't realize that's what you meant.

The code in patch #2 is no longer in HTML5 Boilerplate though. This is what I see there now which is adapted from Micro clearfix hack you linked in #3:

.clearfix:before,
.clearfix:after {
  content: "";
  display: table;
}
.clearfix:after {
  clear: both;
}
.clearfix {
  zoom: 1; 
}

Compass uses this, also adapted from the same Micro clearfix hack:

.clearfix {
  *zoom: 1;
}
.clearfix:after {
  content: "";
  display: table;
  clear: both;
}

I don't see either of them using overflow: hidden. The Boilerplate code prevents top-margin collapsing, and the Compass project thinks that is outside of the scope of a clearfix, and would prefer the pseudo elements not be used up unnecessarily... I don't feel strongly either way at the moment. Getting some more feedback would be good.

I wonder why we're not using one of the Unicode separator characters?

That code is out of date, so we likely won't have to worry about this, but... That is the unicode hex for a space U+0020. The U+ is replaced with a backslash so it will actually work. Here's some more info on that:

http://www.w3.org/TR/CSS2/syndata.html#escaping
http://alanhogan.com/tips/css/special-characters-in-generated-content

Tagging.

Please don't go for overflow: hidden; I will seriously stab myself if we do that.
It would completely destroy the ability to bump stuff out of their wrapper (using absolute positioning or negative margin or w/e) which would seriously hamper the ability to implement complex designs in a sane manner.

just imagine a header where you want to float a bunch of menu blocks, but you want to bump the search box out of it. Either you can't clearfix the header, or the search box can't be in there.

This issue isn't about collapsing margins, that's a whole other story, trying to fix this here would mean non-clearfixed containers might act differently from clearfixed ones, which could easily add to the confusion surrounding both clearfix and collapsing margins, prolly leading to ppl adding a clearfix just to get the collapsing margin behavior...

@Jacine: Thanks for the resources, and for referencing another voice that disagrees with enforcing non-collapsing top margins with clearfix. I think it's important for Drupal to look forward on this -- allow users to take advantage of modern browser's capabilities and fix ie6 and 7 inconsistencies separately, instead of making all browsers clear floats like older versions of ie. If a user wants all browsers to act like older ie's in this case, adding code is cleaner than overriding it.

Of the options so far, the compass clearfix certainly seems the best: it's light, simple, and avoids display and line height issues with generated content entirely.

@seutje don't worry, we're definitely not going with overflow: hidden;. I'm pretty sure @RobW was referring to this method (I misuderstood at first too).

It would be great to get some more feedback here, but I think most of us are leaning towards the Compass method (not collapsing the margins), except for maybe @stevetweeddale. Does that sound about right? Anyone want to roll the patch?

@Jacine, that is the method I was referring to. Sorry for any misunderstanding.

Ok, attached is a patch against 8.x from git. It's my first patch, let me know if it works.

I think I see what I did there... give this one a try.

+ * Zoom:1 is included for browsers that do not support the :after
+ * element, namely Internet Explorer 6 and 7. If support for these
+ * browsers is dropped in core, this can be removed.

do we really need to add that whole comment? seems like a simple /* IE6-7 */ after the zoom statement would suffice along with the link, no?

Sure, I can revise. Comments get stripped when css aggregation is turned on so I thought to err on the side of more info.

BTW, this patch applies to Drupal 7.4 (and probably 7.x), in case anyone wants to test on their 7 installation.

@RobW: thanks for the dialogue!

Giving it some more thought, I'm happy with going for the non margin-collapsing route: the issue of themers not really understanding what clearfix does and therefore how it might (or might not!) behave differently in different browsers probably shouldn't cloud things (though a docs page on 'the drupal clearfix' might not be a bad idea). I stand by my feeling that plenty of themers would probably have opted for the 'just make my life consistent' solution; but then there's gonna be plenty who feel (as I probably would now):

Clearfix is for clearing floats. You should be able to apply it to any block level element and expect the result to be the floats are cleared and nothing else.

Not that these patches needed my blessing! Looks good though.

No problem. And of course these patches need your blessing, that's how this place works.

Is visibility: hidden needed? We are not using it at Compass pie-clearfix, and it works fine for me.

@barraponto @RobW's latest patch implements the micro clearfix hack the same way that Compass does. See comment #10. There is no visibility: hidden in it. Where are you looking?

+++ b/modules/system/system.base.cssundefined
@@ -242,23 +242,18 @@ html.js .js-hide {
+ * Float clearing. ¶

@RobW, there is white space after the float clearing comment that needs to be removed. Setting to "needs work" for that.

@barraponto: the patch removes that (note the - ;))

+++ b/modules/system/system.base.cssundefined
@@ -242,23 +242,18 @@ html.js .js-hide {
+ * Float clearing. ¶

Needs remove trailing space.

+++ b/modules/system/system.base.cssundefined
@@ -242,23 +242,18 @@ html.js .js-hide {
+ * @see http://nicolasgallagher.com/micro-clearfix-hack/

we adding comment here but removed .clearfix:before. It's not same to the links content. can we add some further explanation of why we do it.

0 days to next Drupal core point release.

@barraponto, visibility:hidden was from a previous discussion, now it looks like we're trying the Gallagher/PIE clearfix.

I cited Nicholas Gallagher's post because he was the originator of the technique, but we are using a version revised through discussion on this thread and also in use at Compass. Should we cite the Compass clearfix discussion as well, instead, or add a description of the change in the patch comment?

Maybe something like:

* Float clearing. Revised to allow margin collapse in modern browsers.
* @see http://nicolasgallagher.com/micro-clearfix-hack/

What do you think? After I get some feedback I'll revise the comment, remove the white space, and post a new patch.

can't we just link to http://compass-style.org/reference/compass/utilities/general/clearfix/#m... ?

and not to be a dick, but there's a disconcerting note about FF3.5 there

@seutje: Firefox users are usually up to date. And it works in Firefox 3.6, which is two major versions old. So I guess it is safe for Drupal 8.

As for the link, it is rather better to link to a post than a thread. But we can document it on Drupal.org if you think it is needed.

The http://nicolasgallagher.com/micro-clearfix-hack method uses a :before psuedo selector also, is there a reason why this is not included here. Just subscribing really, this looks good but needs some serious testing.

@Jeff,
read #10 & #27

:before is used to keep all browsers same styling (rendering)

@Jeff: I wrote some reasons/explanation for removing the :before pseudoelement in #8, and Jacine linked to Compass's arguments in #10.

It looks like removing the :before is causing some confusion, so it should probably get a reference link in the comments and be documented on Drupal.org. Re-rolled a patch that removes white space problem and cites Gallagher's post and the Compass Github thread so we have something to test while discussion continues.

Thanks for the pointing out the right comments to look at etc. I did some pretty extensive testing with the patch in #33 and it appears consistent with the current method. All in all an interesting approach to clearfixing, a different approach that looks solid.

Firefox 3.5 is barely relevant now, certainly going to be a dead browser when D8 ships - no issue there.

Sorry but I don't really like the comments, they should explain more what this is and why it is modified, for example:

Based on the micro clearfix hack by Nicolas Gallagher, with the :before pseudo selector removed to allow margins to collapse normally.
@see http://nicolasgallagher.com/micro-clearfix-hack

Not really seeing too much point in referencing the github comment if we explain this properly.

Float clearing for IE6 and IE7.

This is a bit inaccurate, technically it's a hasLayout trigger and IMO should be an inline comment adjacent to the declaration, e.g.

.clearfix {
  zoom: 1; /* hasLayout trigger for IE */
}

Comments changed or not (not that important in the grand scheme of things) I would +1 this for RTBC, but leave that up to Jacine and sun to give their blessings. Nice work.

Thanks RobW & Jeff. :)

I have not tested this with Drupal core themes, but I have been using this code via Compass since it got committed in June without any issues in projects. So, personally I'm comfortable with using the code as is for a replacement.

However, I also agree that the comments needs work, so marking this issue as such.

removed to allow margins to collapse normally.

it prevents TOP margin collapse. (not allow it).

maybe say: ......... remove :before to allow browsers to render their default behaviors

@37, not sure what you mean - in most browsers (99%) when two margins touch (bottom to top) the lesser margin collapses in favor of the greater margin, using the :before selector prevents this, which is not normal - afaict this is why RobW and others object to using this - because its not the normal thing we're used to - the comment reflects this, short of diving into a really long winded explanation.

I am attaching the files I used for testing - there are 4 clearfix methods you can switch between - only the last one (cf4 - Gallager method) prevents margin collapse.

I did some testing with core themes, but most testing was done in the attached files. I'd be quite surprised if we got a regression out of this, although my testing of core themes was not what I would call robust cross browser testing.

I like Jeff's suggestions for the comments. I am waffling on whether it's better to call the zoom a hasLayout trigger, which someone with a certain level of css familiarity will recognize as a way to clear floats, or to call it float clearing, which someone with a certain level of css familiarity will recognize is accomplished by triggering hasLayout. To me it is a non-issue, but is one more useful for a user who doesn't recognize these connections?

@RobW zoom is a hasLayout trigger, it won't clearfix all by itself.

Well, in ie6 and 7, triggering hasLayout alone causes the box to contain/clear the floats, which is what we are talking about here. At first I was worried that labeling the zoom as a hasLayout trigger would confuse some, but since the comment is inline and both declarations fall under the heading Float Clearing, I think it works.

subscribe

I like the idea of using the pie-clearfix method from compass, using it most of the time. The patch in #33 looks good for me. (subscribing)

Would it be possible to use the method described here? http://www.quirksmode.org/css/clearing.html

Isn't as hackish as using the content-property and works pretty solid. It also prevents many problems caused by clear:both. The disadvantage is that a width has to be specified, but as clear:both basically has a similar effect in many cases this could be neglectable.

Thats the ye ole overflow method, not something suitable for very generic clearfix.

The problem I have with the methods that use clear:both is that it not only clears that float, but also interferes with floated elements outside the element the clearfix is applied to.

If the overflow method would also work with "visible" it would be great but until then I don't think we should use that.

from the article linked in #44

The trick works with three of the four overflow values: auto, hidden and scroll

Clearing with content:'whatevs' works more predictably than an overflow declaration in edge cases. For instance, add some padding and margins to what's inside the overflow:auto clearfix and you'll often end up with scrollbars. Overflow:visible will never clear floats (because what would be the point of overflow declarations then?), so that's out.

New micro/revised PIE clearfix seems to be the way to go.

@Jeff, haha, "ye olde overflow method".

EDIT: Wouldn't the overflow-method let you use the clearfix in combination with floated elements more consistently?

With overflow, you'd have to use overflow:hidden to avoid problems, which isn't an issue in most float-clearing-scenarios.

The only reliable value is "hidden", and we can't just universally HIDE overflowing content - that is not an acceptable trade-off for a generic clearfix solution. For example drop menus rely on being able to overflow their containers, and many dynamic JS effects also. Overflow hidden is a useful tool in specific instances, but not as a generic approach.

Unfortunately the link in the OP is now a 404, we should ideally have a more robust issue summary with good information for the maintainers.

Pending reworked comments (see #35) the patch in #33 is good to go.

In that case, here's your revised patch.

Excuse my hamfisted patching.

#53: revised-clearfix-96187-53.patch queued for re-testing.

reroll of patch in #53 (which was certainly broken by the removal of ie6 support)

idflood, thanks for the reroll. What exactly was I doing wrong? Git newb, thought I was following directions in the docs to the letter.

Edit AHH, I see. I guess that's what rebase is for.

Wonderful. It is exactly the same solution the Compass project uses: http://compass-style.org/reference/compass/utilities/general/clearfix/#m...

This issue could really use a summary, particularly relating to what problem we're trying to solve here, what browsers this change has been tested on, and why changing underlying markup should be considered in D7.

revised and expanded issue summary

fixed typos and clarified some points.

@webchick: revised the issue summary, should contain most of the info you're looking for. Please comment back if you're looking for anything more.

Wow, awesome! That was way more than I was looking for, but super useful information! Thanks, RobW!

The one question I have remaining is whether this is truly safe to backport to D7, as the tags indicate? It seems to me it's better to keep the old clearfix the way it was, since there are however many hundreds of themes out there right now that are probably relying on its current behaviour. And glancing through the issue I don't see much if any thorough testing on browsers to see what impact it might have on ~170K sites in the wild.

It seems totally cool to commit this to D8, but D7 is where I'm a little less sure.

I'm honestly not sure on that one. On one side, if I were creating a fresh D7 install I would want the micro clearfix. But on my current installs I've massaged the clearfix in my theme's style.css to address the font/line height issue mentioned in the summary, and if it were updated I'll have the micro clearfix plus the lines .clearfix:after {visibility: inherit; overflow: hidden;}. At a glance I don't think that would cause any terrifying design collapse, but I worry that if people have altered the current clearfix in different ways, revising the method so completely could lead to unexpected layout changes.

What is the policy on updates in core? If you trust that people will actually read the changelog, then with a "hey, heads up" warning this could be a go. I can't think of any specific common situations where changing between the two would definitely be a problem, but it is clearfix, one of the pieces of code you can guarantee that every Drupal site in existence is using.

Maybe a D7 patch included in this issue, and a D8 commit is the safest route to go.

Ha, no we definitely can't trust people to read the changelog. :) And a lot of Drupal sites find themselves in a situation where someone else developed and designed the site, got it up and running, and then taught them the basics of running "drush up" to do minor version updates. If they do that and their theme completely breaks on them in a routine maintenance activity, they're not going to be able to recover, without calling that person who built their site who is likely off developing others' sites by now.

Given that your theme is an example of one that already has a workaround for this problem in D7, I'm going to remove the backport tag and mark back to RTBC for D8. Dries or catch can evaluate this and see if it makes sense, but I think it would be a great improvement along with other changes inline with supporting better web standards.

Sounds good. Thanks for the RBTC, excited to see this committed.

I agree this is RTBC too. Thanks for all the work here @RobW, and for the re-roll @idflood!

Sorry @webchick, I didn't realize that tag was here. :) This isn't really a bug, and it shouldn't go back into D7.

Totally agree on no backport, I and many others are running improved clearfixes already with thousands of users. Great summary RobW!

Oh wow, just saw the summary. Very nice @RobW! :D

Looks great, thanks for the detailed issue summary. Committed and pushed to 8.x.

Sweet, first core patch I've worked on that was committed! How exciting.

YAY! :D Go RobW!

Of course, you know this means we have to "initiate" you now. I hope you like green jello and feathers...

one last tweak...

Changed the summary a bit after Nicolas pointed out a few errors:
- Corrected Nicolas' name (was Nicholas)
- Corrected the notion on Firefox < 3.5 (it *does* work, there's just an edge-case bug)

- Corrected Nicolas' name
- Corrected the notion on Firefox < 3.5

Thanks for the corrections.

Also, the tags could use a revision (-html5, +clearfix) as this issue has nothing to do with HTML5. Not changing myself, maybe maintainers are using that tag to group issues as part of an overall css/html modernization push, don't want to step on toes.

Here's a D7 patch for for anyone who would like to use the revised clearfix on projects right now.

Tested one of my sites with the new Micro Clearfix after discovering some major inconsistencies in IE7 and IE8 and minor inconsistencies in other browsers.

Confirming successful test on WinXP: IE7, IE8; Mac OS X: FF, Chrome, and Safari.

Thank you very much!

- Corrected the other instance of "Nicholas Gallagher"