Discuss removing 'clearfix' class from Classy

If modules are still loading CSS that floats elements, then clearfix is still useful. We don't have a good way of reusing clearfix like we would with Sass.

The attached patch removes all occurences of the .clearfix class from Classy. To find the files where it's used you can use the following grep command:

$ grep -iRl "clearfix" ./core/themes/classy
./core/themes/classy/templates/content-edit/node-edit-form.html.twig
./core/themes/classy/templates/field/field--text.html.twig
./core/themes/classy/templates/form/field-multiple-value-form.html.twig
./core/themes/classy/templates/navigation/toolbar.html.twig
./core/themes/classy/templates/views/views-exposed-form.html.twig
./core/themes/classy/templates/views/views-view-grid.html.twig

+++ b/core/themes/classy/templates/field/field--text.html.twig
@@ -4,16 +4,6 @@
- * A 'clearfix' class is added, because 'text' fields have a 'format' property
- * that allows a Text Format to be associated with the entered text, which then
- * applies filtering on output. A common use case is to align images to the left
- * or right, and without this 'clearfix' class, such aligned images may be
- * rendered outside of the 'text' field formatter's boundaries, and hence
- * overlap with other fields. By setting the 'clearfix' class on all 'text'
- * fields, we prevent that.

How does flexbox help in this use case? I agree with LewisNyman that clearfix still has a place for clearing non-layout related floated elements.

This particular instance is probably important for making floated image in rich text fields not break layouts.

@Sam152 Using flexbox doesn't require floats for aligning elements.

[...]the flexible box model provides an improvement over the block model in that it does not use floats[...]

See Using CSS flexible boxes for an explanation and A Complete Guide to Flexbox for a few examples.

Although it might be possible using flexbox, core still uses floats for aligning images in filtered text:

.align-right {
    float: right;
}

Until that is addressed, blindly removing all the clearfix classes is a regression.

Ok, I'm just trying to give some help and also to start contributing.

@Sam152 Agreed, I've proposed an initial patch which is incomplete. It identifies the .clearfix usage and removes it from the markup (without really breaking anything on a default setup).

@mortendk Yes, this is missing from the attached patch. The micro clearfix hack should be dirrectly applied on the wrapper elements.

Are any of you at the extended sprints this weekend and interested in discussing a suitable solution?

I still don't know if this is a good idea. Should we discuss it before patching?

Ok, I'm going to work on something else for now. I've seen @mortendk here at the sprints but he doesn't look very approachable :)) Just kidding, let me know if any of you are available for a discussion in the meantime. Thanks!

@mortendk that's fine, I thought myself that's not a priority for now. As a new contributor I start to work on issues that are well defined or somehow isolated. Just le me know when this is ready for work or if there's anything else where help is needed.

Update issue status for #7

Rerolled #7.

I'm not sure if this is possible as Classy is a base theme, so it can't be changed anymore. I think that these kind of changes can only happen in non base themes (neither Classy nor Stable) at this state.

Tested patch #18 woring for me clear all clearfix class for classy theme

I'm not sure we can set this to RTBC yet. Have we had a proper discussion at yet? We are adding a lot of repetitive code to maintain, is this a good idea?

Also, we need screenshots of the affected areas to show that we didn't miss anything. How does this change affect Bartik and Seven?

Setting this to a 'Plan' as we need to discuss alternative solutions first before working on patches.

This could either happen in 8.x, or it should be won't fix, but I don't see a specific reason to move it to 9.x since the class could be moved to Stable?

Discussed in #frontend. markconroy suggested that this should be moved to the contrib project. I am doing that now.