The CSS attached when viewing field collection entities is rather aggressive. It seems unlikely that any theme will want to leave that CSS intact. It also seems to me that simply allowing the markup to inherit styles looks more or less fine for a general use case. In addition, the clearfix classes added in markup require module overrides if you want to float something next to a field collection view. As far as I can tell the clearfix classes are only added to ensure it looks correct with the css. This makes proper overriding difficult -- you have to override the css AND potentially form alter or render alter the output to remove the clearfix classes.
I'd propose removing all the css and clearfix classes and letting themes handle the CSS however they see fit. I'm happy to create a patch if there's agreement. It would completely remove the field_collection_view.css and the clearfix classes from the rendering code.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | field_collection-css-1249894-4.patch | 3.52 KB | fearlsgroove |
| #4 | field-collection-clearfix.png | 82.92 KB | fearlsgroove |
| #4 | Field collection clearfix logged in.png | 85.83 KB | fearlsgroove |
| #4 | field-collection-multiple-fixed.png | 118.3 KB | fearlsgroove |
| #4 | field collection multiple fixed logged out.png | 129.42 KB | fearlsgroove |
Comments
Comment #1
fagoI'm happy with whatever works for themers. However, the default look should be fine too.
Comment #2
tim.plunkettI understand the desire to remove the clearfix, but in the absence of a
hook_field_formatter_view_alter()(cited as not an option for performance reasons), I don't know of a better way to include it. It would surely be irresponsible for field_collection to redefine the behavior of clearfix.However, a step in the right direction would be following in the footsteps of core's CSS Cleanup, and rename the file. This will make it easier to do targeted overrides in the theme layer.
Comment #3
fagothanks, committed.
Comment #4
fearlsgroove commentedSee the images for an example of why clearfix needs to go. The "Float test" field is just a normal field, floated right with css. Clearfix has a downside -- you can't float items next to it. You can add the effects of clearfix in with css, but it's almost impossible to remove with css, which means putting clearfix in markup makes it quite difficult for themers if anything needs to be floated next to a field collection entity.
I would argue that field collection should concentrate on producing markup that uses classes that are used in core to handle it's styling. The patch attached removes the stylesheet completely and simply uses the core classes "links" and "inline" to handle the inline editing.
Comment #5
fearlsgroove commentedSorry patch was empty
Comment #6
johnalbinfearlsgroove's patch in #5 removes the aggressive CSS completely and re-uses classes in core. +1 to that direction, though I haven't tested the patch.
An alternative to using the current formatter with or without the aggressive CSS is to use a formatter without the CSS and without the markup. See #1337694: Add formatter that doesn't add any HTML wrappers around field collection items
Comment #7
tim.plunkettI think we should mark this issue as a duplicate of #1337694: Add formatter that doesn't add any HTML wrappers around field collection items.
There are already people using the existing formatters and their CSS.
Any objections?
Comment #9
tim.plunkettClosing.