Add a "Required" and "Show by default" option for "Text area with a summary" field/widget.

Tested and reviewed, working perfectly.

I'm not sure why this is a useful feature, or what exactly it'll look like when used. Adding 'needs usability review' and 'needs screenshots' tags.

Fixing tag

Can't review. Needs screen.

I was just looking for this in 7. Going to backport it as a module for my own use.

Here's the screenshot from #14

Having this feature means instead of adding two fields to a content type (eg a body and a teaser), you can just force the body field to display the summary field and make it required (editors regularly miss the 'show summary' link). Then you can use that as the teaser.

I think its a worthy feature, but it need tests.

Also the description 'leave blank' needs to be changed when the required flag is set.

Adding back ux tag

Looks fine.

This looks great, and now with tests.
Down to minor nitpicks now.

+++ b/core/modules/text/lib/Drupal/text/Plugin/field/widget/TextareaWithSummaryWidget.phpundefined
@@ -36,19 +36,23 @@ function formElement(array $items, $delta, array $element, $langcode, array &$fo
+    $required = empty($form['#type']) && !empty($this->instance['settings']['required_summary']) ? TRUE : FALSE;

This can be simplified to $required = (empty($form['#type']) && !empty($this->instance['settings']['required_summary']));

+++ b/core/modules/text/lib/Drupal/text/Plugin/field/widget/TextareaWithSummaryWidget.phpundefined
@@ -36,19 +36,23 @@ function formElement(array $items, $delta, array $element, $langcode, array &$fo
-        'js' => array(drupal_get_path('module', 'text') . '/text.js'),

Do we need a followup to make this use libraries instead since #1737148: Explicitly declare all JS dependencies, don't use drupal_add_js, will ask nod_

+++ b/core/modules/text/lib/Drupal/text/Tests/TextSummaryTest.phpundefined
@@ -180,4 +181,29 @@ function testOnlyTextSummary() {
+    $summary = $this->randomName();

I don't think this is used?

+++ b/core/modules/text/lib/Drupal/text/Tests/TextSummaryTest.phpundefined
@@ -180,4 +181,29 @@ function testOnlyTextSummary() {
+    $this->assertText('Summary field is required.', 'Article with no summary when summary is required cannot be saved.');

Nitpick but the assertion message is confusing. How about 'Unable to save article without a required summary' or something else, not fussy.

+++ b/core/modules/text/text.moduleundefined
@@ -108,6 +108,18 @@ function text_field_instance_settings_form($field, $instance) {
+      '#default_value' => !empty($settings['show_summary']) ? TRUE : FALSE,

Again, this can be simplified to !empty($settings['show_summary'])

+++ b/core/modules/text/text.moduleundefined
@@ -108,6 +108,18 @@ function text_field_instance_settings_form($field, $instance) {
+      '#description' => t('Remove collapse option to who the Summary.'),

Text here is wrong. Suggest 'Summary field is always visible' or similar.

+++ b/core/modules/text/text.moduleundefined
@@ -108,6 +108,18 @@ function text_field_instance_settings_form($field, $instance) {
+      '#default_value' => !empty($settings['required_summary']) ? TRUE : FALSE,

This can be simplified too

+++ b/core/modules/text/text.moduleundefined
@@ -108,6 +108,18 @@ function text_field_instance_settings_form($field, $instance) {
+      '#description' => t('Marks the Summary as required.'),

Mark not marks

Assuming bot comes back green, this looks good to go

+++ b/core/modules/text/lib/Drupal/text/Plugin/field/widget/TextareaWithSummaryWidget.phpundefined
@@ -36,19 +36,23 @@ function formElement(array $items, $delta, array $element, $langcode, array &$fo
+    $js = empty($this->instance['settings']['show_summary']) ? array(drupal_get_path('module', 'text') . '/text.js') : array();
...
+        'js' => $js,

This will cause drupal_process_attached() to be called even when it doesn't need to be.

Instead it should be something like this, after the initial element is made

if (empty($this->instance['settings']['show_summary'])) {
 $element['summary']['#attached']['js'][] = drupal_get_path('module', 'text') . '/text.js';
}

['js'] = array( vs ['js'][] would be a problem if this wasn't for an element we just finished making, so this is good. Thanks @nick_schuch!

Setting back to RTBC per #22.

Sorry; since that one was a straight-up bug fix and this one is a feature and blocked on thresholds, it seemed more prudent to commit it first.

Can we get an interdiff if possible?

And we're back

There've been lots of changes to this part of the code recently, so going for another testbot run.

In case it doesn't finish before I go to bed, tagging as something that was RTBC in time for feature freeze.

#28: 1704864-28-required-and-show-by-default-for-text-summary.patch queued for re-testing.

This is good to go again.
Also helps with proposed solution from @yched at #626546-62: The body field on the Basic Page content type should not allow a summary to be input

Two small things, then this looks good to go:

+++ b/core/modules/text/text.moduleundefined
@@ -128,6 +128,18 @@ function text_field_instance_settings_form($field, $instance) {
+      '#title' => t('Show Summary'),
...
+      '#description' => t('Summary field is always visible.'),

Not sure "Show summary" is obvious enough that that's what that means? How about "Always show summary" and ditch the description?

+++ b/core/modules/text/text.moduleundefined
@@ -128,6 +128,18 @@ function text_field_instance_settings_form($field, $instance) {
+      '#title' => t('Mark Summary as required'),
...
+      '#description' => t('Mark the Summary as required.'),

:)

Don't need a description that says exactly what the field title is. :) Also we could shorten this to just "Require summary"

Fixed as requested

#39: 1704864-39-required-and-show-by-default-for-text-summary.patch queued for re-testing.

I've discovered a bug unfortunately. :(

1. Use Chrome. (I think this matters because of HTML5 validation.)
2. On a fresh install of D8 standard with the patch applied, edit the article body field and check "Require summary".
3. Create a new article. Fill in the title and body without clicking "Edit summary".
4. Try to submit the form. Nothing happens--it fails on HTML validation, but the failure is invisible because the field is hidden.

So, we need to fix the JS to show the field when that happens.

Also, it would be good to add some screenshots: the field instance settings with the patch applied, the summary failing validation, the "always show" behavior, etc.

This might have do do something with the WYSIWYG editor on top. There's a similar issue here: http://drupal.org/node/1338956 But it has to be fixed upstream by the editors. I tried raising the issue at ckeditor, but my voice doesn't carry enough weight. That's why I provided a workaround.

If it's only the summary field without the editor you might just want to add a "novalidate" property to the textarea input so HTML5 validation doesn't catch it. I'm not a big fan of browsers HTML5 validation. In fact I think Drupal should provide a checkbox to disable it entirely. Unless the browser also wants to take over the theming for us :)

I actually think that when the summary is required, the option to hide/show should go away and the field should always be shown.

used issue summary template and added links for new contributors to do tasks

Im going to perform the screenshots for this issue.

I tried to use the simplytest.me with Dreditor and there was an error. Then I tried it without and still received the same error. I dont know what to do.

Great job by everyone working on this issue! Here are some screenshots of the issue in question after applying patch 1704864-47-required-and-show-by-default-for-text-summary.patch by nick_schuch in #47
I confirm that the patch hides the "always show summary" option when "require summary" is checked per #46
1) Drupal 8 without the patch
2) Drupal 8 with the patch
3) clicking save and publish does not visibly do anything because the html5 browser error message happens in the hidden summary area
4) older, non-html5 browsers get this message from Drupal with a blank, required summary

Manually tested and screenshots added in #56, removing tags.

Rerolled the patch to make it green again.
I also removed the test for it. It seems a couple of tests (webcasetests) have been removed in favour of unit tests. Do we need this small test for the required fields, any idea's?

Ok, agreed.
I'll try to cover that today.

I did not add an interdiff since it wouldn't add any value. Lots of files were moved etc ... . The old patch does not apply in the new core by miles.

So the unit test is extended so it can check the required summary.

Took the diff in the wrong order. Correct patch

Tested this patch, but found one issue.
When the summary is marked as required, and you edit the field again. You are required to enter a summary on the edit field screen.

Thx for the review.
I had that error as well and it went away. The problem is that the extra added element does not go through the widget flow as the field itself does.
There is a quick and easy fix for it to check if we are rendering the default widget or the normal one.
Patch coming up.

The patch and interdiff.

Sorry, had the patch the other way around. New patch / interdiff.

#70: text_required_summary.1704864-69.patch queued for re-testing.

#70: text_required_summary.1704864-69.patch queued for re-testing.

With the tests applied, it's all green. Don't know why this is crashing on a fatal error atm.

Rerolled it against latest dev.

Rerolled again.

again

This comes very late in the game, but does not appear to have been questioned yet:

Why do we want to enhance the compound textarea+summary field in these substantial ways, if all of your goals are covered by adding a separate summary text field?

The moment you make the summary required, we're really talking about a separate field → proper validation, error messaging, separate values, widget/formatter control, integrations, etc.pp.

It's not clear why we'd want to shoehorn all of that into a compound textarea+summary field?

Thoughts?

Agreed actually, so would we need a separate compound then?

In the meantime there is a contrib module to accomplish this: Summary settings

I had this use case once and agree that this would be useful. Should we leave it contrib or put that in core?

There has been no new work on this issue in quite some time. So I'm assuming the person assigned is no longer being actively pursuing it. Sincere apologies if this is wrong.

> Why do we want to enhance the compound textarea+summary field in these substantial ways, if all of your goals are covered by adding a separate summary text field?

I would venture to guess that it's a usability answer here. Taking the optional "Required" setting out of this, making it visible by default would help encourage (or even show users they CAN) add a summary for the teaser. By default the verbiage kinda gets overlooked.

For the required. You are correct. At that point, yeah we are talking about a full on another field. What it means for site builders is they can add in this without having to go reconfigure all the displays and/or templates that currently say "Display the body field"

Not saying that these are compelling reasons. Just thought I would add to this old discussion to see if it spawns any progress and/or decisions other than "Needs Work"

1) For Drupal 7: https://www.drupal.org/project/text_summary_options
2) Any update on this issue for Drupal 8?

I've just done a port to D8 for text_summary_options module. More details here #2923443: Offering to maintain Summary Options (Drupal 8 version)

Worked on this with @irawan and @ling-drupal

OK, self review - I know I RTBC'd this a couple of times in the last 6 years (gasp) but things have changed since then - so I think there are two more changes required.

1. +++ b/core/modules/text/src/Plugin/Field/FieldType/TextWithSummaryItem.php
   @@ -95,6 +97,23 @@ public function fieldSettingsForm(array $form, FormStateInterface $form_state) {
   +    $element['show_summary'] = [
   +      '#type' => 'checkbox',
   +      '#title' => t('Always show summary'),
   +      '#default_value' => $settings['show_summary'],
   +      '#states' => [
   +        'invisible' => [
   +          'input[name="instance[settings][required_summary]"]' => ['checked' => TRUE],
   +        ],
   +      ],
   +    ];
   

   This belongs on the widget in my opinion

2. +++ b/core/modules/text/src/Plugin/Field/FieldWidget/TextareaWithSummaryWidget.php
   @@ -64,21 +64,25 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
   +      '#required' => $required,
   

   We also need a validation constraint for this, for API clients

We also need an update path now too, to update the configuration

Adds update path
Adds test coverage for update path
Adds validation constraint for API clients with the required summary field
Adds test coverage for validation constraint
Moves the show summary from the field settings to the widget settings, as its more appropriate there (its not a data option, its a display option).

Fixed deprecation error from test and phpcs issue

The patch looks good to me nothing to nit pick. Needs screenshot about new settings in UI and error. Needs change record with screenshot and it might need a usablity review for the new string and settings added here.

+++ b/core/modules/text/text.post_update.php
@@ -0,0 +1,49 @@
+  $config_entity_updater->update($sandbox, 'entity_form_display', $widget_callback);
+  $config_entity_updater->update($sandbox, 'field_config', $field_callback);

Conventionally, we perform on config change per hook so that other module can run their changes in between if they want to but in this case, I don't this is necessary.

Added screenshots and a change record

Thanks, this looks good now.

random fail

Exception: Warning: file_put_contents(/var/www/html/sites/simpletest/.htaccess): failed to open stream: Permission denied
file_save_htaccess()() (Line: 376)

Setting back to RTBC!
This is not happening anymore.

Tagging for usability and product manager review since the last time webchick looked at this was six (!) years ago.

Wow, this is a blast from the past. :)

The use case in the issue summary makes sense to me, and looks like @Bojhan signed off UX-wise in #11 7(!) years ago.

I read through the whole issue, in part to figure out why it took this long to resolve, and it seems like the main objection was @sun in #84, essentially asking what the value of these options are over having two text fields.

I think the value is that, for better or for worse, Drupal has shipped with this Body + Summary field concept since ages ago (Drupal 5?), and there are various parts of the system that make assumptions about that summary field / teaser view (e.g. the default front page view). Adding these settings here mean not having to manually go find all of those places and manually undo their assumptions, to instead utilize a custom "Teaser" field. And in the event that something like #1378350: Clean up the "Long text and summary" field ever happens, it seems like these settings help bring the field further inline with others, and would help make that migration easier.

So with that....

Committed and pushed to 8.8.x! TENACITY OF THE DECADE AWARD, FOLKS!! :D However, I am not planning to backport this to 8.7.x, since it's a "nice to have" vs. a bug fix.

published change record