When views have empty row, it must display as:
<div class="row"></div>
But it displays as:
<div class="row">
</div>
As a result, css selector :empty does not work.
Therefore I propose to remove the spaces around row.content in views-view-(unformatted, list, gird).html.twig
PS.
I do not want to filter empty row, because it is used as a fix for the problem last row of flexbox layout.
I also do not want to override a template, just for the sake of removing spaces.
An empty tag should be empty anyway, is not it?
Steps to reproduce:
1. Create some articles with and without tags
2. Create a view that shows unformatted list, grid, HTML list of fields content: tags
3. Activate "Hide empty fields" in list settings
4. Verify the markup in the browser
Before:
After:
Comment | File | Size | Author |
---|---|---|---|
#18 | after.png | 24.57 KB | Lendude |
#18 | before.png | 25.31 KB | Lendude |
#16 | 2789909-16.patch | 9.17 KB | Anonymous (not verified) |
#16 | 2789909-16-test-only.patch | 2.94 KB | Anonymous (not verified) |
#12 | remove_spaces_around_row_content-2789909-12.patch | 6.18 KB | Anonymous (not verified) |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedvaplas created an issue. See original summary.
Comment #2
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #3
lucur CreditAttribution: lucur at Reinblau commentedTest on simplytest.me:
1. Create some articles with and without tags
2. Create a view that shows unformatted list, grid, HTML list of fields content: tags
3. Activate "Hide empty fields" in list settings
4. Verify the markup in the browser
The patch works well for unformatted list.
Grid output:
HTML list output:
Hide empty fields is still active for HTML list... The output is kind of buggy since it produces an empty bullet (Tag is the only field that is shown). But that would be another issue.
Comment #4
lucur CreditAttribution: lucur at Reinblau commentedComment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedlucur, thank you for good testing! Grid layout to be changed twice, of course (for row and column regime).
"Hide empty fields" option is still active for HTML list because it is hide only fields (views.theme.inc, line 100), but not hide row. As a result, the template takes a list of empty rows. It's not a bug, but a feature :)
Comment #6
dawehnerDon't we want to make the same changes in core itself, so basically just not change stable?
Comment #7
Anonymous (not verified) CreditAttribution: Anonymous commented+1. And I created a new issue. I created issue here because of my stupidity). But now I diff the classy templates from the views templates and.. and i'm surprised) Why all these files exist in the classy:
if they do not differ from the core templates?
Or classy policy is to copy all of the templates? Then why did not copy these files:
And yet, it seems we have forgotten to remove @ingroup themeable in views-view-row-rss.html.twig of classy template. I do not know what it is, but maybe a patch for this - the alone benefit from this comment :)
Comment #8
dawehnerYeah the idea is to potentially change them in the future, in case some css classes would be helpful.
Comment #9
davidhernandezI can see an argument that it is ok to remove the whitespace in Classy, Stable, and core Views. This is kind of a bug fix, and doesn't actually alter the markup. Can anyone think of a use case where making this change in Stable and Classy will break and existing theme's layout?
Comment #11
Anonymous (not verified) CreditAttribution: Anonymous commentedOkay, I can describe this case:
It is a sad story. But I propose to take the risk anyway. Perhaps we will be able to compensate for the risks through article on Drupal Planet :)
Comment #12
Anonymous (not verified) CreditAttribution: Anonymous commentedPatch to remove the whitespace in Classy, Stable, and core Views
Comment #16
Anonymous (not verified) CreditAttribution: Anonymous commentedTest-only is interdiff with #12.
Comment #18
LendudeAdded the steps to reproduce from #3 to the IS and some screenshots from the html before and after.
Followed the steps and saw the results. The fix makes sense.
Had a little think about possible BC implications, but I don't see how removing these spaces would break styling or layouts, so in my mind this feels like a safe change.
Moved to the views module queue, since this feels like a views bug and not a classy bug.
Comment #19
larowlanAdding lucur and dawehner to review credits for manual testing and comments that changed the path of the patch
Comment #22
larowlanCommitted 251c996 and pushed to 8.6.x.
Cherry-picked as 6f707f6 and pushed to 8.5.x
Thanks
Comment #23
Anonymous (not verified) CreditAttribution: Anonymous commented🎉 @Lendude, @larowlan, many thanks! 🙏🏻