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:

CommentFileSizeAuthor
#18 after.png24.57 KBlendude
#18 before.png25.31 KBlendude
#16 2789909-16.patch9.17 KBAnonymous (not verified)
#16 2789909-16-test-only.patch2.94 KBAnonymous (not verified)
#12 remove_spaces_around_row_content-2789909-12.patch6.18 KBAnonymous (not verified)
#7 2789909-7.patch442 bytesAnonymous (not verified)
#5 2789909-5.patch2.08 KBAnonymous (not verified)
#2 2789909-2.patch1.7 KBAnonymous (not verified)

Comments

Anonymous’s picture

Anonymous (not verified) commented

vaplas created an issue. See original summary.

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Needs review
StatusFileSize
new 2789909-2.patch1.7 KB
lucur’s picture

lucur commented

Test 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:

<div class="views-col col-2" style="width: 25%;">
            
          </div>

HTML list output:

<ul>
  <li><div class="views-field views-field-field-tags"><div class="field-content"><a href="/taxonomy/term/3" hreflang="en">London</a></div></div></li>
  <li></li>
  <li><div class="views-field views-field-field-tags"><div class="field-content"><a href="/taxonomy/term/1" hreflang="en">Amsterdam</a>, <a href="/taxonomy/term/2" hreflang="en">Mexico City</a></div></div></li>
</ul>

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.

lucur’s picture

lucur commented
Status: Needs review » Needs work
Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
StatusFileSize
new 2789909-5.patch2.08 KB

lucur, 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 :)

dawehner’s picture

dawehner
Primary language German
commented

Don't we want to make the same changes in core itself, so basically just not change stable?

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 2789909-7.patch442 bytes

+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:

  • views-view-grid.html.twig
  • views-view-list.html.twig
  • views-view-mapping-test.html.twig
  • views-view-opml.html.twig
  • views-view-row-opml.html.twig
  • views-view-rss.html.twig
  • views-view-unformatted.html.twig

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:

  • views-view-field.html.twig
  • views-view-fields.html.twig

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 :)

dawehner’s picture

dawehner
Primary language German
commented

Yeah the idea is to potentially change them in the future, in case some css classes would be helpful.

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Status: Needs review » Needs work

I 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?

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Anonymous’s picture

Anonymous (not verified) commented

Okay, I can describe this case:

  1. Bod made a view, which have a list output and empty rows (like #3).
  2. He tried to use css selector ":empty", but nothing happened.
  3. Then he had a brilliant idea - to replace the dull list-style-type on pretty flowers.
  4. Of course, this does not solve the problem, but it was the killer feature of his site.
  5. But after fixed problem of spaces, plenty flowers is lost. (Bob just forgot to remove the style)
  6. Bob few days trying to find the cause of the incident, but in vain.
  7. Bob goes to Joomla!

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 :)

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
Issue tags: +views, +stable
Related issues: +#2791683: Spaceless around row.content in templates
StatusFileSize
new remove_spaces_around_row_content-2789909-12.patch6.18 KB

Patch to remove the whitespace in Classy, Stable, and core Views

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Anonymous’s picture

Anonymous (not verified) commented
Related issues: +#2795133: Hide field or block on no results fails because it adds spaces
StatusFileSize
new 2789909-16-test-only.patch2.94 KB
new 2789909-16.patch9.17 KB

Test-only is interdiff with #12.

The last submitted patch, 16: 2789909-16-test-only.patch, failed testing. View results

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Component: Classy theme » views.module
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -views, -stable
StatusFileSize
new before.png25.31 KB
new after.png24.57 KB

Added 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.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Adding lucur and dawehner to review credits for manual testing and comments that changed the path of the patch

  • larowlan committed 251c996 on 8.6.x 
    Issue #2789909 by vaplas, Lendude, lucur, dawehner: Remove spaces around...

  • larowlan committed 6f707f6 on 8.5.x 
    Issue #2789909 by vaplas, Lendude, lucur, dawehner: Remove spaces around...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Committed 251c996 and pushed to 8.6.x.

Cherry-picked as 6f707f6 and pushed to 8.5.x

Thanks

Anonymous’s picture

Anonymous (not verified) commented

🎉 @Lendude, @larowlan, many thanks! 🙏🏻

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.