Problem/Motivation

Regression from #2969136: Behavior settings should not include hidden styles on save.

How to reproduce:

  1. Install paragraphs collection demo
  2. Start creating a paragraphed node
  3. Add a container paragraph

The dropdown style selection is there even when not choosing the "Behaviors" js tab.

Proposed resolution

Use the group settings from the $form array provided to the \Drupal\paragraphs_collection\Plugin\paragraphs\Behavior\ParagraphsStylePlugin::buildBehaviorForm method.

Remaining tasks

User interface changes

API changes

Data model changes

Comments

Primsi created an issue. See original summary.

primsi’s picture

Status: Active » Needs review
StatusFileSize
new794 bytes

Not sure about tests here. We don't seem to have collection js tests yet.

berdir’s picture

The only reason we stopped just using $form is to fix some problems, lets check this with @mbo next week.

mbovan’s picture

#3: Yes, we didn't want to produce any additional markup if there are no styles to select. That was the reason to switch away from $form and use $build.

+++ b/src/Plugin/paragraphs/Behavior/ParagraphsStylePlugin.php
@@ -70,6 +70,9 @@ class ParagraphsStylePlugin extends ParagraphsBehaviorBase implements ContainerF
+    // The form array only contains the group information that is needed for
+    // the behaviors tab to work properly.
+    $build = $form;

Either we do this here for the style plugin only, or we append the plugin behavior form array in Drupal\paragraphs\Plugin\Field\FieldWidget\ParagraphsWidget::formElement() as $element['behavior_plugins'][$plugin_id] += $plugin_form;?

Related issues: #2971115: Do not print behavior form if there is nothing a plugin can do, #3002419: Base buildBehaviorForm should not add markup for plugins with no behavior form

johnchque’s picture

Assigned: Unassigned » johnchque

Working on tests for this. :)

johnchque’s picture

I was checking the code of other plugins and it seems we always used the form variable to add all the plugin elements. I think is better to make them all equal to making it easier to change later. (e.g. not adding any markup at all if there are no plugins).

The last submitted patch, 6: style_always_visible-3001207-6-test-only.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Status: Needs review » Needs work

The last submitted patch, 6: style_always_visible-3001207-6.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

johnchque’s picture

Status: Needs work » Needs review
StatusFileSize
new2.43 KB
new4.82 KB
new1.5 KB

My bad. :(

The last submitted patch, 9: style_always_visible-3001207-9-test-only.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Berdir committed 741d6de on 8.x-1.x authored by yongt9412
    Issue #3001207 by yongt9412, Primsi: Style selection always visible
    
berdir’s picture

Status: Needs review » Fixed

Yes, lets revert that change for now, we do have test coverage now in case we decide to improve that later, then we could also add test coverage for the specific problem that solves in regards to the markup...

Status: Fixed » Closed (fixed)

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