Empty table cells never hidden if twig debug is true

I was having a hard time trying to figure out why the option to hide empty columns wasn't working and found this issue. Here's a quick and dirty patch that fixes this problem for me.

Add a test to check if the field has been excluded.

Here is an updated patch which contains the test against debug mode status.

I'm not quite sure if the regex is totally bullet-proof to exclude HTML comments. Maybe somebody could check this?

Oh, maybe we could implement a dedicated test for that case ? Something like \Drupal\Tests\views\Functional\Plugin\StyleTableTest::testEmptyColumn but with the twig debug enabled.

It's not only views tables problem. I use fields in views. I set "hide field if empty", but it doesn't hide if twig debug is on

Yes, I can confirm this is a bug in views-view-fields as well in support of #13

This also applies if the title for the display is not empty on Drupal 8.7.7.

I added the twig_debug check and stripping and trimming of the field if it's enabled to the isValueEmpty check in FieldPluginBase.php.
This should fix the "hide field if empty" problems for fields in views when twig_debug is enabled. (#13 & #14)

I remade the patch because it failed to apply.

This to me is not a bug, it's not empty => hence it isn't hidden.

But yeah I've run into this too, but still....

Adding a workaround for twig debug changing our definition of empty sounds like a feature (and not one I'm in favour off, turn off twig debug if you want to see what it looks like on production, we don't need more regexes and complexity in the views rendering pipeline, we have plenty of that already). Just my 2ct obviously.

Unfortunately #18 strip also the comment like this "<!--form-item-entity_browser_select--paragraphs_library_item:1-->" and in the result checkboxes not visible:
Without patch: https://take.ms/iDQOU
With patch: https://take.ms/i9g8N

I found this issue with the "Paragraph Library" module.

$value = preg_replace('/<!--.*?-->/s', '', $value);

This seems to break Media Library pretty badly. The checkboxes disappear on each item.

The regex captures and delete the placeholder <!-- -->, e.g <!--form-item-media_library_select_form--0-->. See \Drupal\media_library\Plugin\views\field\MediaLibrarySelectForm::getValue

I just made a small commit that solves for me the issue mentioned in #28.

I also made a merge request.

You can download the patch as a diff directly from my commit. I'm also attaching a patch file here.

Tried to fix CCF for #31.

32 incorrectly introduces a space in the regex.

Noticed a problem with the patch. The following error is raised, likely due to PHP8 changes.

Deprecated function: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in Drupal\views\Plugin\views\field\FieldPluginBase->isValueEmpty() (line 1226 of core/modules/views/src/Plugin/views/field/FieldPluginBase.php).

We should ensure we don't enter the if (\Drupal::service('twig')->isDebug()) { condition if value is NULL.

Adding a new patch addressing points from #34 and #35, and also fixing some phpcs errors. please review.

1. +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
   @@ -1236,6 +1240,15 @@ public function isValueEmpty($value, $empty_zero, $no_skip_empty = TRUE) {
   +    if ($twig_debug_on == TRUE && !is_null($twig_debug_on)) {
   

   the previous way of doing it, from 32 and before, was fine. no need to assign and check TRUE

   this is also checking the wrong value for whether it is NULL. you should be checking $value. Not the result of isDebug()

   $value !== NULL will be fine for the null check.

2. +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
   @@ -1236,6 +1240,15 @@ public function isValueEmpty($value, $empty_zero, $no_skip_empty = TRUE) {
   +      $value = preg_replace('/<!--.*?-->/s', '', (string) $value);
   

   this correctly removed the leading space

@TanujJain-TJ there are many changes in the new patch compared to the previous without explanation, in addition to an overly large interdiff? Have you brought in irrelevant changes?

Tests continue to fail.

NW

@dpi sorry for that null check i interpreted it wrong at first updating the patch to correct all the changes required, also these out of scope changes are just PHPCS fixes i got while running this below given command, did it so the patch doesn't fail Custom commands check. adding new interdiff with #32.
phpcs --standard="Drupal,DrupalPractice" --extensions="php,module,inc,install,test,profile,theme,css,info,txt,md,yml"

Please address the CS changes relevant to the lines being modified only.

updated according to #40, please review.

@Tanuj ... for tests to pass i think you also need to check the twig service exists

So, maybe you could update patch (or create new MR) with something like:

    // If twig_debug is enabled we strip out the
    // html comments before running the empty check.
    if ($value !== NULL) {
      if (\Drupal::hasService('twig') && \Drupal::service('twig')->isDebug()) {
        $value = preg_replace('/<!--.*?-->/s', '', (string) $value);
        $value = trim($value);
      }
    }

@Tanuj: Patch #41 works, but the core media library form widget does not work anymore. You cannot select items there.

@Tanuj: I can confirm that after applying patch #41, the core media library widget (both grid and table views) is not functioning correctly. The checkboxes are not being rendered when twig debugging is enabled.

I am not sure whether this solves all the problems people face here, but I have a completely different approach that solves it for me with views at least and does not introduce any additional logic.

When I check for empty fields, the problem is not the added XML-commented debug output per se, but the added whitespace/line breaks that are not inside the comments and therefore lead to twig believing there is content indeed.

I first thought that removing the line breaks would mess up the generated markup (but I wanted to live with it, as long as it would solve the initial problem of seeing different rendered things with and without debug mode enabled).
But as it turns out, both firefox and chromium take care of adding linebreaks themselves when using developer tools in the browser and the markup looks just the same as with the patch. And it makes checks in twig whether fields are empty truely true :-)

I am not a developer so please improve as you see fit. Just posting here as it fixes it for me and seems less intrusive.
Applies against 10.1.x and 10.2.x.

I have created a new patch over #41 after observing the patch issues in #46. This patch fixes the issue of checkboxes not showing in media library picker when twig debug is on.

Does not apply to Drupal 11.3.2

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

https://www.drupal.org/about/core/blog/introducing-the-main-branch-for-d...

Hello ,
I am using Drupal 11.3.3, Drush 13.7.1.0 & PHP 8.3 and patch #41 is not working for me

Thanks in advance!

This patch is a snapshot of MR!14452 so it can be safely used with composer-patches. It applies on D11.3.

Back to NR because NW was only because of the review bot in #56

Issue summary could use some love.

Also appears to be missing test coverage