Convert a normal paragraph into the library

Paragraphs Collection it is!

And it's the instance that is converted, not the type. And not sure if this really needs determination by type.

And also not sure about supporting converting back in the first issue, that can easily be split up.

WIP patch. Missing tests and probably something else.

One question I had is what to use as the title for the library item? Created from paragraph + date. Or should we move away from the approach I took (add the link to the drop button) and have a separate button + text field for creating the library item?

Worked a bit more on that. Still needs tests.

I will continue with this one.

IMHO I don't like how we are making title. Is not very readable and doesn't provide information about paragraph.
Providing screenshot.

Any idea how we can improve this?

Yeah, we already discussed this before...

My idea was to include a confirmation step that contains a label text field that the user can or is required to fill out? We could possibly even use the closed summary as a better default value, limited to the first N characters.

Might not be really possible to actually do that without changes in paragraphs, so we could make a folow-up for that. People tried to refactor the different paragraph states into a pluggable system before, we might actually have a use case for it now :)

Yeah, so you would prefer a "multistep" form inside the paragraph?
So when choosing the action to convert it to a library item, we show a form with title (and the other metadata fields?)
And then on confirmation, we save, connect the new paragraph with the library wrapper?

So yeah, we can simply make it work with a default title, preferrably from the collapsed summary and then get into all the complexity in follow-ups. Thus, i think we should create the follow-ups for this plan and update the parent meta issue.

Yep, that's my suggestions, similar to the delete -> confirm delete form.

Cleanup patch, fixing small bug and added tests.
One more stuff here, do we want to avoid converting empty paragraphs, e.g. text paragraph without text, or images without image?
Created follow-up: #2856568: Allow editing label (title) while converting paragraph item to library

No special treatment for empty paragraphs. It would need hardcoded special treatment and create even UX problems..

Than, this is ready for review.

I think we could improve the test a bit by adding text to the text paragraph we create and then when the node is saved assert that we see the same text we added there.

Sorry, I totally forgot on that part.

+++ b/modules/paragraphs_library/paragraphs_library.module
@@ -5,6 +5,102 @@
+  $target_type = $field_definition->getSetting('target_type');
...
+    if ($entity_type && $entity_type->isSubclassOf(ParagraphInterface::class)) {

This is not a proper condition. You need to check if we are in a paragraphs widget. The code would also apply on an entity reference field widget pointing to a paragraph target (although people should not do it)...

Also i'm missing a summary update and the issue creation of the "and backwards" step.

Since this conversion is that important, we can get this in with an alter. But we need a high priority follow-up:
Paragraphs should make it easy to register operations on that button, for instance by triggering a specific operation alter hook.
This code complexity to just register an operation is leading us to total eclipse.
We should wait with the issue implementation until we have a decision in markcarvers refactoring proposal
#2854444: Refactor field widget code to reduce duplication and make theme abstract

So before further coding, please help with issue management.

1. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -5,6 +5,102 @@
   +  $field_definition = $context['items']->getFieldDefinition();
   +  $target_type = $field_definition->getSetting('target_type');
   +  if ($target_type) {
   +    $entity_type = \Drupal::entityTypeManager()->getDefinition($target_type);
   +
   +    // Add the "Convert to library item" option only to paragraphs.
   +    if ($entity_type && $entity_type->isSubclassOf(ParagraphInterface::class)) {
   +      $parents = $element['#field_parents'];
   

   yes, $context contains the widget object, so we should be able to check that is our widget. And I guess only the new experimental, we don't want to mess with class as its structure will eventually be different.

2. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -5,6 +5,102 @@
   + *   The form state object.
   + */
   +function make_library_item_submit(array $form, FormStateInterface $form_state) {
   +  $button = $form_state->getTriggeringElement();
   

   not namespaced.

3. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -5,6 +5,102 @@
   +  // Inserting new element in the array.
   +  $widget_state = WidgetBase::getWidgetState($parents, $field_name, $form_state);
   +  $delta = $button['#delta'];
   +  $widget_state['items_count'];
   +  $widget_state['real_item_count']++;
   +  $widget_state['original_deltas'] = array_merge($widget_state['original_deltas'], ['1' => 1]);
   

   the items_count line is strange. also not sure about the original deltas logic.

4. +++ b/modules/paragraphs_library/src/Entity/LibraryItem.php
   @@ -95,4 +97,16 @@ class LibraryItem extends ContentEntityBase implements LibraryItemInterface {
   +  public static function createFromParagraph(ParagraphInterface $paragraph) {
   +    $library_item = static::create([
   +      'label' => $paragraph->label() ?: t('Paragraphs library item placeholder @date', ['@date' => (new DrupalDateTime())->__toString()]),
   +      'paragraphs' => $paragraph,
   +    ]);
   

   didn't we want to use the closed summary, not seeing that anywhere? (yes that is currently on the widget class where it can't be reused I guess, that's needs to be move somewhere else)

   My suggestion for the default label would be "$type: substr($summary, 0, 50)"

#3. Is working fine. We are exporting current paragraph and on the same place we add item from library and we are not adding one more new paragraph after exporting.
#4. This part is a little bit tricky. We can not use summary for label, summary function is in ParagraphsWidget class and we can not access from LibraryItem. One solution is to make function in ParagraphsWidget static and another one is to add summary function in ParagraphInterface and overwrite in Paragraphs. I think the second one is better approach because summary should be part of paragraph not ParagraphWidget.

This part with adding summary in Paragraph we can do in follow-up and after that we can change that label use part of summary in this #2856568: Allow editing label (title) while converting paragraph item to library

Yes, agreed with moving it to a method on the entity. That makes sense to me.

Lets open an issue for that and work on that now, I think we should probably include that in a first patch for this feature, Miro also said that above, although he probably didn't except that this also needs a paragraphs change.

Created new issue to move summary into paragraphs #2857112: Add summary method into paragraph entity. This is is kind of postponed until we finish summary issue.

This also needs a follow up for the conversion in the other direction if we are not doing it here.

@toncic and the title still proposes both direction. Needs title + summary update. Please always do that first.

Postponing for now until #2857112: Add summary method into paragraph entity is in.

I can't see the paragraph reverse issue linked or created?

After committing this #2857112: Add summary method into paragraph entity we can continue with converting paragraphs item into library.

Changed to use part of summary for label. Rebased and fixed test failing.

Rerolled and fixed test failing.

1. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -7,6 +7,97 @@
   +      $paragraphs_entity = isset($widget_state['paragraphs'][$context['delta']])
   +        ? $widget_state['paragraphs'][$context['delta']]['entity'] : FALSE;
   

   We could use a ternary operator here.

   isset($widget_state['paragraphs'][$context['delta']]) ?: FALSE;

2. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -7,6 +7,97 @@
   +          '#value' => t('Convert to library item'),
   

   Just noticed in the mocks that the wording is "Add to library". I think we should use that.

3. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -7,6 +7,97 @@
   +  $widget_state['real_item_count']++;
   

   This was copied from the duplicate feature. Should we really update the count? Because we are replacing here, not adding.

4. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -7,6 +7,97 @@
   +
   

   Extra new line

I think this heavily conflicts with the widget refactoring in #2854444: Refactor field widget code to reduce duplication and make theme abstract
That should make an extensibility point much easier.

We should not add a form alter here. Instead the widget should offer an extension point or a hook an event or a behavior plugin interface callback so everyone can register actions to these buttons.
This extensibility point us super important in the evolution for Paragraphs. Let's create a high priority issue there

Implemented comm #32 and created issue in paragraphs which @miro_dietiker mentioned in #33. #2862428: Add hook function for widget

Postponing this then.

Now we have a hook. Plz reroll and use that extensibility point instead of the alter.

Rerolled patch and changed to use hook function from paragraphs.

Fixing test failing.

The first interdiff is wrong. Here is a patch.

+++ b/modules/paragraphs_library/paragraphs_library.module
@@ -7,6 +7,86 @@
+    $id_prefix = implode('-', array_merge($parents, array($field_name, $context['delta'])));
...
+      '#name' => strtr($id_prefix, '-', '_') . '_add_to_library',

First dashes, then conversion to underscore? :-)

We do that a lot in paragraphs widget,

1. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -7,6 +7,86 @@
   +      '#weight' => 503,
   

   taht's a strange weight, usually those numbers are a lot smaller.

2. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -7,6 +7,86 @@
   +      '#submit' => ['make_library_item_submit'],
   

   this needs paragraphs_collection prefix.

3. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -7,6 +7,86 @@
   +      '#limit_validation_errors' => [
   +        array_merge($parents, [$field_name, 'add_more']),
   +      ],
   

   this is not the add_more button.

4. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -7,6 +7,86 @@
   +        'callback' => [
   +          '\Drupal\paragraphs\Plugin\Field\FieldWidget\ParagraphsWidget',
   +          'itemAjax',
   +        ],
   

   use ParagraphsWidget::class

5. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -7,6 +7,86 @@
   +
   +  // Inserting new element in the array.
   +  $widget_state = WidgetBase::getWidgetState($parents, $field_name, $form_state);
   +  $delta = $button['#delta'];
   +  $widget_state['items_count'];
   +  $widget_state['real_item_count'];
   +  $widget_state['original_deltas'] = array_merge($widget_state['original_deltas'], ['1' => 1]);
   

   uhm, what? we're not inserting something new, we replace?

6. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -7,6 +7,86 @@
   +  $library_item = LibraryItem::createFromParagraph($widget_state['paragraphs'][$delta]['entity']);
   +  $library_item->save();
   

   does this ensure that the paragraph is saved, including changes?

7. +++ b/modules/paragraphs_library/paragraphs_library.module
   @@ -21,6 +101,7 @@ function paragraphs_library_preprocess_paragraph(&$variables) {
        $variables['content'] = $view_builder->build($library_item_render_array)['paragraphs'];
      }
   +
    }
   

   unrelated leftover?

8. +++ b/modules/paragraphs_library/src/Entity/LibraryItem.php
   @@ -95,4 +98,23 @@ class LibraryItem extends ContentEntityBase implements LibraryItemInterface {
   +    if (strlen($summary) > 50) {
   +      $summary = Unicode::truncate($summary, 50);
   +    }
   

   you don't need to check strlen() yourself, Unicode::truncate() does that.

9. +++ b/modules/paragraphs_library/src/Entity/LibraryItem.php
   @@ -95,4 +98,23 @@ class LibraryItem extends ContentEntityBase implements LibraryItemInterface {
   +    $label = $paragraph->getType() . ': ' . $summary;
   

   isn't getType() the machine name? we need getParagraphType()->label() or so.

10. +++ b/modules/paragraphs_library/src/Tests/ParagraphsLibraryItemTest.php
    @@ -138,4 +150,29 @@ class ParagraphsLibraryItemTest extends ParagraphsExperimentalTestBase {
    +    ]);
    +    $this->drupalGet('node/add/paragraphed_test');
    +    $this->drupalPostForm(NULL, NULL, 'Add Text');
    +    $edit = [
    +      'field_paragraphs[0][subform][paragraphs_text][0][value]' => 'Random text for testing converting into library.',
    +    ];
    

    so we are testing for a new entity, that's good.

    I think we should also cover converting an existing paragraph on an edit form and changing an existing paragraph and then converting it to a library item at the same time.

11. +++ b/modules/paragraphs_library/src/Tests/ParagraphsLibraryItemTest.php
    @@ -138,4 +150,29 @@ class ParagraphsLibraryItemTest extends ParagraphsExperimentalTestBase {
    +    $this->assertText('From library');
    +    $this->assertText('Reusable paragraph');
    +    $edit = [
    +      'title[0][value]' => 'TextParagraphs',
    +    ];
    +    $this->drupalPostForm(NULL, $edit, 'Save and publish');
    +    $this->drupalGet('node/1');
    

    lets make sure we also assert the label of the library item,.

Addressed comm #41 and #42.
@Berdir:
1. I think we should check '#weight' in $links in PargraphsWidget class, and give the similar number, so this seems OK.
5. Yes, we are replacing.
6. I think so. Also Added some test for this.

I've tested this manually and it seems to work fine. I opened two follow ups:
#2867297: Paragraphs library table expands with content
#2867299: Improve library item widget item experience

Let's wait for #42.5 if we meant to add, not replace.

Because we are replacing element I changed comment.

Checked with @Berdir about #42.5, we had some unrelated lines, so removed it.

Committed, thanks!

Hi, I'm curious about this one...it looks like most of the patch is on the core paragraphs_library module, but it belongs to the paragraphs_collection module. Is there a chance this functionality could be applied to paragraphs? I imagine there would be interest, I'm interested for one, and don't run paragraphs collection due to the strict "no production" warning and the fact that it wont install for me b/c I already have a paragraph type called "container".

Thanks!

I don't know what you mean, this was committed to paragraphs_collection and then moved into the paragraphs project. It's opt-in per paragraph type, so make sure you allow it for the paragraph types where you want to use it.

Thank you for the response @Berdir. I was confused by this being an issue on paragraphs_collection, I can confirm that I was able to add preexisting paragraphs to the library without the paragraphs_collection module, thanks for the nice addition!