Problem/Motivation

The problem is in ParagraphsStylePlugin::getStyles():

  public function getStyles(ParagraphInterface $paragraph) {
    $paragraph_styles = [];
    foreach ($this->configuration['groups'] as $group_id => $group_configuration) {
      $paragraph_styles[$group_id] = $group_configuration['default'];
    }
    if ($styles_config = $paragraph->getBehaviorSetting('style', 'styles')) {
      // Loop over all groups to get the behavior setting.
      foreach ($styles_config as $group_id => $style) {
        $paragraph_styles[$group_id] = $style;
      }
    }

If some style didn't had default value before and paragraphs are created with empty style value for this style then when you define style default value you would expect that all paragraphs, new and old which does not have value, would get this default value.

However, the second loop will always override default styles even in the case when paragraph style value is empty.

CommentFileSizeAuthor
#5 default-style-value-overriden-3117486-5.patch3.34 KBBerdir
#5 default-style-value-overriden-3117486-5-fail.patch1.71 KBBerdir
#4 default-style-value-overriden-3117486-4.patch1.75 KBBerdir
#2 default-style-value-overriden-3117486-2.patch895 bytespivica
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pivica created an issue. See original summary.

pivica’s picture

pivica CreditAttribution: pivica at MD Systems GmbH commented
Status: Active » Needs review
FileSize
default-style-value-overriden-3117486-2.patch895 bytes

Here is a patch.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Assigned: Unassigned » Berdir 
+++ b/src/Plugin/paragraphs/Behavior/ParagraphsStylePlugin.php
@@ -375,7 +375,10 @@ class ParagraphsStylePlugin extends ParagraphsBehaviorBase implements ContainerF
     if ($styles_config = $paragraph->getBehaviorSetting('style', 'styles')) {
       // Loop over all groups to get the behavior setting.
       foreach ($styles_config as $group_id => $style) {
-        $paragraph_styles[$group_id] = $style;
+        // If the style value is empty then keep the default style value.
+        if (!empty($style)) {
+          $paragraph_styles[$group_id] = $style;
+        }
       }

ok, so we didn't test for the case when there's an empty value in there. Instead of the separate condition, couldn't we just write this as:

$paragraph_styles = array_filter($styles_config) + $paragraph_styles;

Will also have a look at testing this

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
FileSize
default-style-value-overriden-3117486-4.patch1.75 KB

Hm, we have no kernel tests for this stuff, and I don't want to mess with the old web tests as it would conflict with the conversion of them.

Refactored the whole thing a bit for better variable names and the suggested simplification, array_filter() and just combining it with the defaults.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
FileSize
default-style-value-overriden-3117486-5-fail.patch1.71 KB
default-style-value-overriden-3117486-5.patch3.34 KB

Testing this indirectly by removing the duplicate default handling in other places, which showed that we also didn't handling defaults correctly when styles had been disabled globally.

The last submitted patch, 5: default-style-value-overriden-3117486-5-fail.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Berdir committed 00fe719 on 8.x-1.x 
    Issue #3117486 by Berdir, pivica: Default style value will be overridden...
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Fixed

Committed. Figured out what's the problem with those random fails and will commit that separtaely.

Status: Fixed » Closed (fixed)

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