🇩🇪Germany @hctom

Changes have been done!

Okay perfect, didn't see that one coming ;) I will follow along and remove D9 compatibility for my module as well now. Thanks!

hctom → created an issue.

Updated issue summary with info about more findings of the PHP 7.4 code incompatibilities.

hctom → created an issue.

That was the first thing I tried, when identifying the issue with the form element, but it does not fix the error, because the container itself still gets serialized due to $this usage in the getInfo() implementation (because all its information gets serialized when written to cache).

As I already mentioned: ALL core elements use the static approach instead of using $this, so this should be done with this element as well - and of course to avoid these errors ;)

Done ;)

hctom → created an issue.

This is fixed now

This is fixed by using the maximum supported PHP version for previous major tests now.

hctom → created an issue.

hctom → created an issue.

I am closing this ticket now, because there was no feedback anymore. Feel free to open a new issue, if there is still an issue with this module.

Any news on this? Because this prevents a site from being installed with both the cl_editorial and storybook module initially enabled.

You are welcome... and this is actually really something I was already thinking about, to allow setting custom labels for the field options. I'll give this a try soon, when I have some spare time again.

In the meantime you may alter the displayed label of the field's empty value by implementing a corresponding field widget form alter hook.

For now, I will close this issue as "works as designed" and I will start on a follow up for the custom labels soon.

Thanks for the details - I think, I know what the problem is: You are only switching the default view mode, but you are accessing the entity in full view mode.

So you have to check full view mode (because you want to switch away from that one) under "View modes to switch" in field storage config instead of enabling it under "View modes allowed to switch to".
Additionally your field should be optional - if no switch is selected later, the full view mode (with layout builder and potential overrides enabled) is displayed, if standard is selected, that view mode will be used instead. I have some of these setups running without problems.

Please let me know, if that fixes the issue for you.

Thanks for the report. In order to track down this issue, I need some more information from you:

• How is your VMS field configured? Especially: What are the View modes to switch in Field storage? Is it enabled for Full and/or Default

It would be nice, if you could provide this information, so I can try to reproduce this potential issue.

Thanks!

Please see the issue fork for the fix. While fixing this issue, I also realized, that there was a submitForm callback registered, which is not defined in the ComponenSelectorElement, so I removed that one completely from the element information.

And I think I even found the cause, why this issue only happens when cl_editorial and storybook module are both installed: The storybook module provides a custom Drupal\storybook\Cache\SkippableBackend cache backend when development mode is enabled for that module - and this seems to result in the serialization error... but I am still confused, why this happens when re-installing a site from config, but maybe someone else has an idea, if maybe something has to be fixed at the storybook module as well?!

I digged through the code a little more to track down this issue and I think I got it: the getInfo() implementation defines its callbacks like this:

 '#process' => [[$this, 'populateOptions']],
 '#element_validate' => [[$this, 'validateExistingComponent']],
 '#submit' => [[$this, 'submitForm']],

Using $this here results in the element class itself (and all its injected dependencies) being added to the element info array, which at last gets serialized before being written to cache.

Tomorrow, I'll create an issue fork that contains the necessary changes to rewrite all those callbacks to static - which is also how core does it in all its form element plugins.

+1 for the interfaces... I am currently trying to write some units tests for functionality that relies e.g. on ComponentPluginManager and it is impossible to mock these SDC services at all without adding some wrapper methods in my own classes that I can swap out during tests. Additionally this also prevents the sdc module itself from providing unit tests (which it actually should do to test all code on a low execution level).

So please do not restrict everything within this module only to keep the "single directory" methodology intact - IMHO developers writing extensions for this, should be free to do so.

UPDATE: This only seems to happen, when drupal/storybook is enabled as well - I added it to the steps to reproduce.

hctom → created an issue.

hctom → created an issue.

Yeah, now oEmbed are also draggable again as well - Thank you so much!

I'm sorry, but we missed one important thing: The JS from core's Layout Builder UI to disable those elements was unfortunately not enough. I digged a little more through the code and what the JS actually does is adding a tabindex="-1" to e.g. iFrame elements. But to get them draggable, there is the following CSS snippet needed as well (otherwise clicks on such elements will be handled by e.g. the iFrame content) - taken from layout_builder/css/layout-builder.css:

.layout-builder-block [tabindex="-1"] {
  pointer-events: none;
}

Maybe this even solves your "issue" that you had to add the custom stylings from 📌 Disable block links Fixed additionally.

Should I create a follow up, or should this ticket be reopened again?

You're welcome - I also like your UI much more than the core UI, hehe!

I just created a follow-up to this ticket: ✨ Disable interactive elements in blocks to allow dragging Active - this is needed because this issue only handles disabling of links in blocks, but there are more interactive elements, which may interfere with dragging behavior. In addition: The custom solution introduced with this ticket should be replaced with core's implementation in layout_builder module to disable interactive elements in blocks in layout builder UI.

hctom → created an issue.

Thank you for your feedback and I'm glad I was able to help.

...but I am not really sure about your fix ;) I'd be careful with this, because of the quite hacky parsing of the view mode and in addition: I think you will have to add something to vary your page*.html.twig render cache entries per used suggestion. Maybe it is even enough to add a view mode based cache key in a hook_preprocess_page() implementation, but I am not really sure about this and would have to do some testing with this. Without this cache key Drupal may cache rendering of page.html.twig with the same cache entry as page--your-view-mode-based-suggestion.html.twig... but as I already said: That would require a lot more testing and debugging to see what really happens there ;)

Thanks for the details. That definitely helps identifying the potential problem. I again tried to reproduce this with a vanilla Drupal 10.2.3 installation and I can see, that the following theme suggestion are custom ones maybe defined by some code or another module:

   x page--page--no-headers-or-footers.html.twig
   * page--node--page.html.twig
   * page--node--72661.html.twig

Core's theme suggestions → are based on internal path only.

So maybe the custom code providing those new suggestions does either not have the correct view mode available or itself some caching issues?

Hi NicholasS, thank you for your report.

Unfortunately I doubt that the View Mode Switch Field module is really the cause for this cache issue. It only uses core's hook_entity_view_mode_alter() hook and that one is called in EntityViewBuilder::getBuildDefaults() BEFORE any caching happens. Because this field is a "normal" Field API field, its current value is always respected in the cache entry being build later, while saving an entity also always invalidates any cached entries already (e.g. see EntityStorage::doPostSave() and respectively ::resetCache()).

I just tried to reproduce your issue with a vanilla Drupal install and the Basic page content type having a view mode switch field for the Full view mode, but with no luck. When I enable the cache debug in services.yml, I can even see in the rendered markup, that there are cache keys used, that vary between the original view mode and the view mode to switch to (which will result in different cache entries):

View mode not switched

<!-- CACHE KEYS:
   * entity_view
   * node
   * 1
   * full
-->

View mode switched to teaser via View Mode Switch Field

<!-- CACHE KEYS:
   * entity_view
   * node
   * 1
   * teaser
-->

In your video, the actually changing part of the node page is quite "far away" from the rest of your node (in the header above navigation and hero element), so I wanted to ask, if this is something you render programmatically or is the whole page created by your node template? I'm only asking, because if you render some parts programmatically, you have to make sure, that any of the node's cache tags/contexts/keys bubble up in the render pipeline and do not get lost, which in turn may result in caching issues.
Another thing that makes me wonder, is that e.g. node A should never change cache entries of node B.

Can you please have a look at the cache debug values in your project or if there may be something else, that might interfere with Drupal core's caching mechanisms on these pages. And just another question: Does the display also change when you reload the switched node several times after it has been cached?

This is implemented and all reported code quality issues are fixed as well.

hctom → created an issue.

hctom → created an issue.

This is fixed now.

hctom → created an issue.

Hi @earthday47, I just wanted to know, if you found something that caused this issue or have any new information for me? If so, feel free to re-open or update this ticket, please. Thanks!

Sorry, I just realized that we use an outdated version of this module in our project :( This was already fixed in 📌 Automated Drupal 10 compatibility fixes Fixed - So I am closing this issue again.

hctom → created an issue.

Hiding forgotten patch files

Created an issue fork for 11.x and applied patch from #21 because DrupalCI tests no longer allow testing for versions > 10.1 - all new development for this issue should happen in there, so I also hide all the patch files except the last one.

Here is a new re-roll, that should be applicable for 10.2.x and 11.x. Unfortunately, I was not able to create an interdiff with patch from #20, because of another changed line (const glue = url.includes('?') ? '&' : '?';) that landed in core in the meantime.

Also bumping the version to 11.x, where all new development happens.

Tests are still needed.

Please see the issue fork and MR for a proposal to fix this issue.

hctom → created an issue.

Added a ticket relation to 📌 Improve image style width detection and avoid obsolete image derivatives Fixed , that is the cause for this issue.

Please check the issue fork and MR with the fix for this issue. Thanks.

hctom → created an issue.

Just found a small bug with this patch: The following element is added to ParagraphsTypeForm::form() method's form array by this patch:

<?php
$form['static_icon_file']['description'] = [
  '#type' => 'item',
  '#description' => $description,
];

Unfortunately an element with '#type' => 'item' is treated as an input value and thus interferes with the form's actual description form field value.

So my changes add '#input' => FALSE to the new $form['static_icon_file']['description'] element to NOT treat this element as a form value anymore.

See the new MR for the 3.x version for review as well

hctom → changed the visibility of the branch 3.0.x to hidden.

Whoops, I just messed up the MR by rebasing it with the 3.x version branch. But I rebuild the MR branch and now everything should be fine again. I will open up a new issue for for the 3.x version instead.

hctom → made their first commit to this issue’s fork.

To be honest, I also think that this may have something to do with the migration. I double checked the core code invoking this alter hook (in EntityViewBuilder::getBuildDefaults()) and everywhere only string are used and all calling methods use full as fallback value for their $view_mode arguments. So either there is some code that tries to render blocks programmatically with an explicitly set NULL value for $view_mode when calling ANY_ENTITY_LIST_BUILDER::viewMultiple() or your migrated blocks are somehow causing this issue (which is really hard to identify from far away).

So please use your patch for now and I am setting the issue's priority back to "Normal" and status to "Needs more info", because the code itself follows core's argument value type.

If you have any other information that might help here, feel free to update this issue.

Done ;)

Let's use this logo for now (same icon as field type on 'Add field' screen in Field UI).

hctom → created an issue.

Thanks for the report. This is a strange problem, because core's hook_entity_view_mode_alter documentation says, that it MUST be a string. I am also using this module in combination with custom block types and I have never experienced this issue so far.

Do you have some more information (e.g. if it is an inline block in layout builder or a normal custom block, list any other modules that may interfere here in your setup) for me, that helps to get an understanding of this issue?

Implemented in current dev branch

hctom → created an issue.

Please check the MR for the proposed changes.

hctom → created an issue.

hctom → created an issue.

+1 to get this merged!

Fixes are contained in the associated issue fork and MR.