Add summary method into paragraph entity

Uff, this method is quite weird :)

I know we're just moving it, but I think we should clean it up at least a bit, we can move some more complicated things including more tests to a follow-up.

1. +++ b/src/Entity/Paragraph.php
   @@ -429,4 +429,90 @@ class Paragraph extends ContentEntityBase implements ParagraphInterface, EntityN
   +   * {@inheritdoc}
   +   */
   +  public function getCollapsedSummary() {
   +    $text_types = [
   

   this now being a method on the paragraph, we could actually move/improve test coverage as a kernel test, which would be faster and easier than trying to do everything through the widet.

2. +++ b/src/Entity/Paragraph.php
   @@ -429,4 +429,90 @@ class Paragraph extends ContentEntityBase implements ParagraphInterface, EntityN
   +
   +    $summary = [];
   +    foreach ($this->getFieldDefinitions() as $key => $value) {
   +      if ($value->getType() == 'image') {
   +        if ($this->getImageSummary($key) != '') {
   +          $summary[] = $this->getImageSummary($key);
   

   $field_name => $field_definition are much better names for this.

   getImageSummary() can be protected.

3. +++ b/src/Entity/Paragraph.php
   @@ -429,4 +429,90 @@ class Paragraph extends ContentEntityBase implements ParagraphInterface, EntityN
   +      if (in_array($value->getType(), $text_types)) {
   

   text types is no including plain text types. A hardcoded list is also a pretty limited approach. tmgmt tries to be a bit more flexible, but it's tricky stuff.

   but not that behaviors field is string_long, so we'll have to skip that.

4. +++ b/src/Entity/Paragraph.php
   @@ -429,4 +429,90 @@ class Paragraph extends ContentEntityBase implements ParagraphInterface, EntityN
   +        if (strlen($text) > 50) {
   +          $text = strip_tags(substr($text, 0, 150));
   +        }
   

   we can possibly use \Drupal\Component\Utility\Unicode::truncate() for this. I also don't get the logic with checking for 50 and then cutting of at 150?

5. +++ b/src/Entity/Paragraph.php
   @@ -429,4 +429,90 @@ class Paragraph extends ContentEntityBase implements ParagraphInterface, EntityN
   +      if ($value->getType() == 'entity_reference_revisions') {
   +        if ($this->get($key) && $this->get($key)->entity) {
   +          $summary[] = $this->get($key)->entity->getCollapsedSummary();
   +        }
   +      }
   

   this blindly assumes that the reference is a pargraph, that's not guaranteed, could be anything. check the entity against ParagraphsInterface

6. +++ b/src/Entity/Paragraph.php
   @@ -429,4 +429,90 @@ class Paragraph extends ContentEntityBase implements ParagraphInterface, EntityN
   +        if (!in_array($key, ['type', 'uid', 'revision_uid'])) {
   +          if ($this->get($key)->entity) {
   +            $summary[] = $this->get($key)->entity->label();
   +          }
   +        }
   

   don't understand why we include both uid and revision_uid here. Also, pargraphs doesn't have a uid field, actually. and the revision_uid field seems hardly interesting for a summary.

   I'd just start the summary hardcoded with the type and drop this.

7. +++ b/src/Entity/Paragraph.php
   @@ -429,4 +429,90 @@ class Paragraph extends ContentEntityBase implements ParagraphInterface, EntityN
   +    $collapsed_summary_text = implode(', ', $summary);
   +    return strip_tags($collapsed_summary_text);
   

   if we strip tags here, then we don't need to do it again here.

8. +++ b/src/Entity/Paragraph.php
   @@ -429,4 +429,90 @@ class Paragraph extends ContentEntityBase implements ParagraphInterface, EntityN
   +  /**
   +   * Returns summary for image paragraph.
   +   *
   +   * @param string $field
   +   *   Field name from field definition.
   +   *
   +   * @return string
   +   *   Summary for image.
   +   */
   +  public function getImageSummary($field) {
   

   why not support file fields too?

9. +++ b/src/Entity/Paragraph.php
   @@ -429,4 +429,90 @@ class Paragraph extends ContentEntityBase implements ParagraphInterface, EntityN
   +        elseif ($text = $image_value->entity->filename->value) {
   +          $text = $image_value->entity->filename->value;
   +        }
   

   getFilename()

10. +++ b/src/Entity/Paragraph.php
    @@ -429,4 +429,90 @@ class Paragraph extends ContentEntityBase implements ParagraphInterface, EntityN
    +        if (strlen($text) > 50) {
    +          $text = strip_tags(substr($text, 0, 150));
    

    again, truncate()...

11. +++ b/src/ParagraphInterface.php
    @@ -21,4 +21,13 @@ interface ParagraphInterface extends ContentEntityInterface, EntityOwnerInterfac
    +  /**
    +   * Return collapsed summary for paragraph.
    +   *
    +   * @return string
    +   *   The text without tags to return.
    +   */
    

    Returns.

    The summary isn't collapsed, we just use it in the collapsed/closed widget state. So maybe just getSummary()?

    And "to return" doesn't really make sense.

12. +++ b/src/Plugin/Field/FieldWidget/InlineParagraphsWidget.php
    @@ -678,6 +678,7 @@ class InlineParagraphsWidget extends WidgetBase {
           }
     
    +
           $element['subform']['#attributes']['class'][] = 'paragraphs-subform';
    

    empty newline added.

Or should we even create a service?

The reason why we didn't care too much about this summary hardcoding was - it is super easy to override in a widget subclass. After moving i need to alter / replace the entity class..

We still have discussions about when to use rendered paras and when to create the summary. Imho the summary currently is related a lot to how we display it and is intentionally in the experimental interface domain. Moving it to the entity disallows us further api changes :-!

Also i don't like that this is blocking other important issues.

This is not experimental, it was already in the classic one as well.

The only API here is that you get a string back. We make no promises on how that is build and what it contains, so I don't think this blocks further improvements on that. And however we end up using it, I think the concept of having a text summary seems generally useful, for example for that library use case that we have no and possibly other things as well.

Out of my mind we have two extra requirements:
A flag if recursion into children is wanted
A flag if it should be full or reduced.
The reduced version will skip elements that have an icon, covered in the ui already.. and here's the connection with the ui...

Dunno how to cover those and how to bring the icons into the ui. There might be multiple possible locations for the icons.

> A flag if recursion into children is wanted

Sure? That might make sense at first, but it would likely not make much sense at do this for container paragraphs that contain nothing (except some behavior settings maybe) but nested paragraphs.

> A flag if it should be full or reduced.
Yes, I was wondering about the type for example as well, that is now and likely also in the future in the UI somehow. also doesn't actually seem to be in the summary despite being in the code, will check that. But again, letting code decide that is tricky, just because we *support* icons doesn't mean everyone will have time, so possibly a setting instead.

But we can also add a currently empty $context or $options array and we'll add options through that in the future. then we're flexible.

Exclusion of recursion: Absolutely sure. For instance the drag and drop mode shows a summary AND also always shows the childrens below, indented. Repeating the children on the parent would lead do weird duplication. Also "container only" are likely to have plugins enabled and thus if no content fields, the summary would simply tell styles or grid settings chosen. And if none, then an empty summary is perfectly what i expect. :-)

A pattern about summary could be that the (tbd) icon methods will return icon definitions with keys. Then we could pass in options to the summary with some skip key that gets the icon keys that are displayed otherwise. The summary then excludes them on creation.

I was thinking if we should add some kind of alter to allow other modules to change this summary given the concerns raised in #6. Thouths?

There is a space before the comma in the text paragraph, could add trim() to the summary?
Other than that, seems ready.

Committed, but needs work as in the follow-ups from the discussions are not yet created.

Adding related issues.