- Issue created by @pdureau
- 🇧🇪Belgium tim-diels Belgium 🇧🇪
Not sure if this is the right approach, but we fixed it in our setup with the attached patch.
We would love to discuss this if there are other ways to fix this. - 🇫🇷France pdureau Paris
Hi Tim,
Your proposal is interesting. It works only for layouts but maybe it is enough and we don't need a more generic solution. For exeample, this issue is about layouts: 🐛 Drupal entities containing optional or multiple fields do not render patterns correctly Closed: won't fix
However, you are dealing only with empty #cache and #weight values:
['#cache' => [], '#weight' => NULL];
We need to deal also with non empty #cache values, and bubble them.keys
- 🇫🇷France fb-multimedia
Not sure of the issue, but I've you try simply using the |render filter, like this :
{% if description|render %}
{{ description }}
{% endif %}
- 🇮🇳India ravi kant Jaipur
{% if description.0 is not empty %}<p class="fr-tile__desc">{{ description }}</p>{% endif %}
- 🇮🇳India ravi kant Jaipur
{% if description|render|trim %}<p class="fr-tile__desc">{{ description }}</p>{% endif %}
- 🇧🇪Belgium tim-diels Belgium 🇧🇪
Hey Pierre,
I did not have the full context there on what could all be possible, so just made something so our team had something working.
Happy to discuss this approach and see if we get there with this approach or if it does something different.I understand what you say with none empty #cache and maybe non empty #weight values.
You have an idea on how to bubble the cache keys?@fb-multimedia and @ravi kant Those are not solutions as you can read in the original D.O. core issue.
- Status changed to Postponed
about 1 year ago 12:38pm 23 October 2023 - 🇫🇷France pdureau Paris
Let's move this to 2.x now the development rbanch is open and active.
However, not in the scope of 2.0.0, so let's target 2.1.x
- 🇫🇷France pdureau Paris
If the issue is found only in Layout Builder, we can do the fix in:
- https://www.drupal.org/project/ui_patterns_layout_builder → for UI Patterns 1.x
- ui_patterns_layout for UI Patterns 2.x
- 🇫🇷France pdureau Paris
We also have this weird behaviour with empty markup:
$variables['test'] = [ "#markup" => "", ];
{{ test ? "true" : "false" }}
Result:
true
Maybe we have to create an issue for Drupal Core.
- Status changed to Active
10 months ago 9:36am 5 March 2024 - 🇩🇪Germany Christian.wiedemann
This is actually a SDC Issue. We are doing an ugly workaround here.
The main problem is still there is no real good method to check if the array is empty. The way described in the issue only works in a list of cases but there will be cases where which it doesn't work.
Maybe we switch the scope of this issue and only checking slots as fields in layout builder and with field formatter and nothing more. Because this array guessing if it is maybe empty works only sometimes.
- 🇫🇷France pdureau Paris
Yes, it is a SDC issue, and Core team has the same worries as you: 🐛 SDC: Make empty render arrays evaluate to false in component templates Active
But it is also doable at a UIP2 level IMHO, and we can propose to move the fix to Core later
- Merge request !263Issue #3383544 by christian.wiedemann, pdureau: Empty field values when not renderable → (Merged) created by Christian.wiedemann
- 🇩🇪Germany Christian.wiedemann
I implemented a empty check for render arrays for 1 slots.
- 🇫🇷France pdureau Paris
I have added a few coding related comments, but my main concern is the logic being located in
ComponentElementBuilder
instead ofComponentElementAlter
.Because those 2 methods are only called from
ComponentElementAlter
for now, and we want to keep the calls here so all renderables will benefit from the logic instead of only the renderables built fromComponentElementBuilder
.Am I missing something?
- 🇩🇪Germany Christian.wiedemann
I moved the stuff to ComponentElementAlter and made them protected. My initial idea was that maybe other consumers using this method before they provide the slot. For example if layout builder surround the slots with an container. But it is better to make it first protected and if there is a need we introduce it as an API.
- 🇫🇷France pdureau Paris
That looks great. I will add a few comments for the future us, and I will merge.
- 🇫🇷France pdureau Paris
I am doing a big rebase because 📌 [2.0.0-alpha3] Replace component() function by native Twig mechanisms? Active was merged before.
With this rebase, I may lose this MR comment from @duaelfr:
suggestion (perfs): merge metadata without using the renderer service by directly calling BubbleableMetadata::createFromRenderArray, BubbleableMetadata::merge and BubbleableMetadata::applyTo methods
Christian, have you seen this comment?
- 🇩🇪Germany Christian.wiedemann
I checked BubbleableMetadata right now. There is only BubbleableMetadata::createFromRenderArray static. All other methods are not static. Do I miss somtehing?
- First commit to issue fork.
- 🇫🇷France duaelfr Montpellier, France
@christian.wiedemann Sorry my comment was unclear.
The idea was to replicate the feature of\Drupal\Core\Render\Renderer::mergeBubbleableMetadata()
without calling therenderer
service, which is not needed here.
I added a commit to the MR to illustrate. -
pdureau →
committed 486c760c on 2.0.x authored by
christian.wiedemann →
Issue #3383544 by christian.wiedemann, pdureau, duaelfr: Empty field...
-
pdureau →
committed 486c760c on 2.0.x authored by
christian.wiedemann →