Problem/Motivation

In UI Suite Bootstrap, in the table component, there is:

  {% if caption %}
    <caption>{{ caption }}</caption>
  {% endif %}

The table presenter template is passing an empty string as the caption variable.

But in the component template I have an array with an empty #children.

In ui_patterns/src/Plugin/UiPatterns/PropType/SlotPropType.php, there are 2 problematic places :

    if (is_string($value)) {
      return ['#children' => Markup::create($value)];
    }

After making the proposed change I obtain an empty array with #cache from:

protected static function cleanRenderArray(array $value): mixed {
    if (empty($value)) {
      // Element::isRenderArray() returns FALSE for empty arrays.
      return ['#cache' => []];
    }

Steps to reproduce

Using UI Suite Bootstrap, create a View using a table display.

Proposed resolution

    if (is_string($value) && !empty($value)) {
      return ['#children' => Markup::create($value)];
    }

I don't know what to do about the other place where there is a problem.

Remaining tasks

CommentFileSizeAuthor
#8 withoutEmptyCache.png279.63 KBsmovs
#8 withtEmptyCache.png295.48 KBsmovs
#5 Screenshot 2025-05-24 at 18.27.33.png88.62 KBsmovs

Issue fork ui_patterns-3523756

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

grimreaper created an issue. See original summary.

pdureau’s picture

pdureau commented
Title: False positive when checking empty slots » [2.0.5] False positive when checking empty slots
smovs’s picture

smovs commented
Assigned: Unassigned » smovs

smovs opened merge request !386

smovs’s picture

smovs commented
StatusFileSize
new Screenshot 2025-05-24 at 18.27.33.png88.62 KB

Hi @grimreaper
I have added a check for a non-empty value according to your proposal.

But I'm still confused about this false positive when checking slots.
E.g. I set the caption in the table with wysiwig source with an empty textarea.

In this case, the table contains an empty caption tag because wysiwig source creates an array with text format settings.
It would be cleaned by self::cleanRenderArray(), but didn't

I am not sure if it is related to the false positive that you mentioned. Probably I went the wrong way.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Version: 2.0.3 » 2.0.x-dev
Status: Active » Needs work

Hi @smovs,

The problem occurs for example when using the table component with the views table presenter template.

If you are using a component "manually" or with site building it is ok.

But with a presenter template, like written in the issue summary, you can get an empty string as slot value and in this case the problem occurs.

So, to test you can create a view using the table display (not component display using the table component).

pdureau’s picture

pdureau commented
Title: [2.0.5] False positive when checking empty slots » [2.0.6] False positive when checking empty slots
smovs’s picture

smovs commented
StatusFileSize
new withtEmptyCache.png295.48 KB
new withoutEmptyCache.png279.63 KB

Hi @grimreaper

Sorry for the delay.

What I found.
From my point of view, rendering works as expected in general, because:
When we check if $value is empty and set it ['#cache' => []], we create a non-empty array. It means we redefine the variable from non-array to array in this case. See SlotPropType::normalize

    if (!is_array($value)) {
      return empty($value) ? ['#cache' => []] : ['#plain_text' => (string) $value];
    }

In Twig template, when we check {% if caption %} is always true because in this case, we pass a non-empty array.

I am not fully understanding the purpose of the forced setting ['#cache' => []] each time.

I added screenshots for better understanding

pdureau’s picture

pdureau commented
Assigned: smovs » grimreaper
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Hi @smovs,

In Twig template, when we check {% if caption %} is always true because in this case, we pass a non-empty array.

Yes, and that's the purpose of this issue, we should be able to keep a simple if statement in Twig and having check as expected. We don't want an empty caption to appear.

we create a non-empty array.

That's what needs to be challenged.

Note: I can't reassign the issue to you.

smovs’s picture

smovs commented
Assigned: grimreaper » smovs

@grimreaper
Thank you for the clarification. I will take a look on this weekend

pdureau’s picture

pdureau commented
Title: [2.0.6] False positive when checking empty slots » [2.0.7] False positive when checking empty slots
smovs’s picture

smovs commented
Assigned: smovs » Unassigned
Status: Needs work » Needs review

Hi team!
Please review my changes to fix the issue with false positive when checking empty slots.

pdureau’s picture

pdureau commented
Assigned: Unassigned » grimreaper
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » pdureau
Status: Needs review » Reviewed & tested by the community

Thanks @smovs !

Tested on the encountered case of empty caption in views table: OK

@pdureau, should we let Christian or Mikael merge?

pdureau’s picture

pdureau commented
Assigned: pdureau » christian.wiedemann
just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Title: [2.0.7] False positive when checking empty slots » [2.0.8] False positive when checking empty slots
Status: Reviewed & tested by the community » Needs work

hello,
sorry trimming here is not appropriate :)
we need to cope with this edge case in a better way.

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Assigned: christian.wiedemann » pdureau
pdureau’s picture

pdureau commented

i agree there is something to do: a string resolving as a false boolean (so, empty) must become a renderable resolving as a false array (so, empty)

However, we have already stated than SDC validator is sometimes confused by an empty slot: https://git.drupalcode.org/project/ui_patterns/-/blob/2.0.x/src/Element/...

So maybe it is a 2 steps fix: first fix at SDC level, then do something like that (naive, not tested, proposal):

    if (is_string($value)) {
      return empty($value) ? [] : ['#markup' => $value];
    }
pdureau’s picture

pdureau commented
Assigned: pdureau » Unassigned
smovs’s picture

smovs commented

Hi all!
@pdureau
Your approach in #19 has an edge case with the string "0". We need additional checking for it.
empty("0") return TRUE

See here stackoverflow

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Title: [2.0.8] False positive when checking empty slots » [2.0.9] False positive when checking empty slots
smovs’s picture

smovs commented
Assigned: Unassigned » smovs
smovs’s picture

smovs commented
Assigned: smovs » Unassigned
Status: Needs work » Needs review

Hi team!
I've updated the condition for the string normalization to replace trimming with empty().

Please review

pdureau’s picture

pdureau commented
Assigned: Unassigned » just_like_good_vibes

thanks @smovs

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Title: [2.0.9] False positive when checking empty slots » [2.0.10] False positive when checking empty slots
pdureau’s picture

pdureau commented
Title: [2.0.10] False positive when checking empty slots » [2.0.11] False positive when checking empty slots
just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Assigned: just_like_good_vibes » pdureau
Status: Needs review » Needs work

hello, sorry to come late on that one, but testing for '0' seems not appropriate neither.
it doesn't make sense on the PHP side. any other thoughts, like pdureau?

smovs’s picture

smovs commented
Assigned: pdureau » smovs

I will try to figure out how to find a better approach

smovs’s picture

smovs commented
Assigned: smovs » Unassigned
Status: Needs work » Needs review

Hi guys! I changed my approach to checking NULL values in normalize. Please review.

pdureau’s picture

pdureau commented
Assigned: Unassigned » smovs
Status: Needs review » Needs work

Is a normalization tests enough? We may need an other specific test here.

Also, just thinking aloud, but the removal of ['#cache' => []] may be harmful. My memories are blurrry but we may have added that to prevent an empty render array to be processed as a list instead of a mapping in Twig.

Let's discusss