Problem/Motivation

When providing a block reference as a paragraph field it would be nice to control how/if the block configuration form displays. This allow content authors to select a block to reference, but not have control over any block settings.

Proposed resolution

Add a field setting to the block_field widget.

Sample form display

Remaining tasks

Code, test

User interface changes

Additional form display option.

API changes

None

Data model changes

Addition of 'configuration_form' to config schema.

CommentFileSizeAuthor
#14 interdiff-2877520-2-14.txt4.85 KBfenstrat
#14 2877520-14.patch6.85 KBfenstrat
#2 2877520-2-config_form_settings.patch5.55 KBfenstrat
#2 block_field_form_display_options.png20.06 KBfenstrat

Comments

fenstrat created an issue. See original summary.

fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
CreditAttribution: fenstrat at PreviousNext for Transport for NSW commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new block_field_form_display_options.png20.06 KB
new 2877520-2-config_form_settings.patch5.55 KB

Attached patch adds a 'Configuration form' setting to form display. Didn't go with a boolean (hidden/shown) to allow future flexibility (e.g. a setting which only allowed the overriding of block title).

Getting a odd test exception locally, will see how it goes with the testbot.

fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
CreditAttribution: fenstrat at PreviousNext for Transport for NSW commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 2: 2877520-2-config_form_settings.patch, failed testing.

acbramley’s picture

acbramley CreditAttribution: acbramley at PreviousNext for Transport for NSW commented 
+++ b/tests/src/Functional/WidgetTest.php
@@ -51,10 +52,28 @@ class WidgetTest extends BrowserTestBase {
+    $this->drupalGet('node/6/edit');
+    $this->submitForm([], $this->t('Save and keep published'));
+
+    // Default view items_per_page value of 1 node should show.
+    $test_nodes = $nodes;
+    do {
+      $contains = count($test_nodes) == 5 ? 'pageTextContains' : 'pageTextNotContains';
+      $this->assertSession()->{$contains}(array_pop($test_nodes)->getTitle());
+    }
+    while (count($test_nodes));

I don't think this kind of test is necessary. We just want to test that the configuration option (configuration_form = hidden) hides the configuration on the edit form. In that case you would just be checking that the edit form doesn't contain the config options?

I also find the do while quite confusing especially with the dynamic $contains assertion.

fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
CreditAttribution: fenstrat at PreviousNext for Transport for NSW commented

@acbramley Agreed on all accounts, however that have been a decent refactoring of the test, instead went with with working off of what was there. Will change the dynamic $contains assert and work on the failing test when I get a moment.

jrockowitz’s picture

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented

@fenstrat @acbramley If you need commit access to the repository, just ask.

acbramley’s picture

acbramley CreditAttribution: acbramley at PreviousNext for Transport for NSW commented

@fenstrat understood, no need to refactor the existing test though just adjust your own ones :)

@jrockowitz if you're happy with that, that'd be great!

jrockowitz’s picture

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented

@acbramley I am ecstatic to get you involved. My ongoing work on the Webform module has forced me to neglect all my other Drupal projects.

fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
CreditAttribution: fenstrat at PreviousNext for Transport for NSW commented

@jrockowitz no worries. If it frees you up more on Webform happy to help out :) If you could give @acbramley and I commit access we'll see where we can help out, cheers!

jrockowitz’s picture

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented

Done!

Feel free to also list Previous/Next as a 'Supporting organization" on the Block field's project page.

finne’s picture

finne
Primary language Dutch
Location Amsterdam
CreditAttribution: finne at Dx Experts commented

So what is the work that needs doing now:
The patch code seems fine, but the test code could be separated to simplify anything new we want to test (the config form visible or not)

Is that a good description of the current status of this issue?

fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
CreditAttribution: fenstrat as a volunteer and at PreviousNext for Transport for NSW commented
Assigned: Unassigned » fenstrat

@finne yep, that's a good summary :) Sorry, this slipped off my radar, taking a look at this now.

fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
CreditAttribution: fenstrat as a volunteer and at PreviousNext for Transport for NSW commented
Status: Needs work » Needs review
StatusFileSize
new 2877520-14.patch6.85 KB
new interdiff-2877520-2-14.txt4.85 KB

Attached:

  1. Fixes tests, split this into a simpler testConfigurationFormOptions() method.
  2. Fixes short array syntax.
acbramley’s picture

acbramley CreditAttribution: acbramley at PreviousNext for Transport for NSW commented
Status: Needs review » Reviewed & tested by the community

Nice!

  • fenstrat committed 3dc43bd on 8.x-1.x 
    Issue #2877520 by fenstrat, acbramley, finne: Add form display option to...
fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
CreditAttribution: fenstrat as a volunteer and at PreviousNext for Transport for NSW commented
Status: Reviewed & tested by the community » Fixed

Cheers! Thanks everyone.

fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
CreditAttribution: fenstrat as a volunteer and at PreviousNext for Transport for NSW commented
Assigned: fenstrat » Unassigned

Status: Fixed » Closed (fixed)

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

dennis cohn’s picture

dennis cohn CreditAttribution: dennis cohn at ezCompany, iO commented

When I set my configuration form to hidden in my form display, my default field settings won't be applied to the block field.
I also can't edit my default settings anymore in the field setting.

Anyone else has this issue too?

acbramley’s picture

acbramley CreditAttribution: acbramley at PreviousNext for Transport for NSW commented

@Dennis Cohn please create a new bug report and add steps to reproduce there :)