Problem/Motivation

Currently there is no way to render paragraphs_library_item with desired view mode.

Steps to reproduce

  1. I have content type Article. It has two fields field_paragraph_1 and field_paragraph_2.
    • It is configured to render paragraphs in field_paragraph_1 withDefault view mode.
    • It is configured to render paragraphs in field_paragraph_2 withTeaser view mode.
  2. I have paragraph Text. It has two view modes Default and Teaser
  3. For from_library paragraph I configure the same two view modes.
    • Default to render field_reusable_paragraph with Default view mode.
    • Teaser to render field_reusable_paragraph with Teaser view mode.
  4. For paragraphs_library_item I configure the same two view modes.
    • Default to render paragraphs with Default view mode.
    • Teaser to render paragraphs with Teaser view mode.
  5. Create Article node, add Text paragraph to both fields. Click Add to Library for both.

Expected result

  • Paragraph from field_paragraph_1 rendered with Default view mode
  • Paragraph from field_paragraph_2 rendered with Teaser view mode

Actual result

Both rendered with Default view mode.

Proposed resolution

  1. Pass the chosen view mode when performing rendering in paragraphs_library_preprocess_paragraph
  2. Remove paragraphs_library_preprocess_paragraph and use default rendering mechanism

Comments

l0ke created an issue. See original summary.

l0ke’s picture

Status: Active » Needs review
StatusFileSize
new886 bytes

This patch represents the attempt to implement option 1 of resolution.

I keen to hear other opinions.

valthebald’s picture

Status: Needs review » Reviewed & tested by the community

I have faced exactly the same issue, and the patch fixes it

valthebald’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new883 bytes

Actually, one small correction - it's better to take `#view_mode` from the elements settings, not from the first item

l0ke’s picture

Status: Needs review » Reviewed & tested by the community

Patch #4 looks nicer and works properly.

miro_dietiker’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Need tests

Thank you for fixing this bug. It is now definitively a candidate for my tests wishlist.

pingevt’s picture

I think the patch in #4 only works if you have the two same view mode machine names on paragraphs_library_item and paragraph entities (which currently there is nothing to enforce that and is currently not how I have it set up in my use case). I think you would have to dive deeper into the $variables['content'] to get the proper view mode.

Personally, I don't understand why paragraphs_library_preprocess_paragraph() is hijacking the whole render process for that. paragraphs_library_item is a field-able entity if you do add fields (and I would argue you probably should not add fields to them), you should be able to configure it like any other entity. A site editor should be able to configure those view modes normally and expect to get the "normal" results.

I would suggest removing that whole method for the "from_library" paragraph bundle (paragraphs_library_preprocess_paragraph()). There is no way do "undo" what this preprocess does without re-doing what the normal render process does. This is convenient only if you want what is "out of the box" for this module. Maybe add an option to enable this or something but let site builders know you are hijacking the normal render process.

pingevt’s picture

StatusFileSize
new1.44 KB

Adding a patch to test against with paragraphs_library_preprocess_paragraph() removed

berdir’s picture

That is how the library module is designed to work, that it transparently replaces the used paragraph. If you don't want that then just don't use the library but use any other entity type & reference field, you could use another node type or blocks or a custom one. You'll lose some functionality like being able to convert to library and back, but you could also reimplement that yourself for your paragraph type/field.

miro_dietiker’s picture

let site builders know you are hijacking the normal render process.

@pingevt I'm all for improving documentation. What documentation are site builders looking at?

pingevt’s picture

First of all, I just want to say the Library module is a great addition to paragraphs module and thanks for all the work on this. Since I started working with paragraphs in D7, I've always used nodes or custom entities on projects to create re-usable items as you mentioned @Bedir. Having Libraries as a contributed module is great for the community. Less random and unsupported code. I completely understand what you are saying @Bedir, but I would like to explain my case a little further though...

Also, @miro_dietiker Looking back at the README for Libraries, your last paragraph alludes to this issue. I'll be honest though, when I first looked at it, I skipped over it, because I only thought it was talking about labels. As far as improving documentation, I would add more and add more technical documentation as well as the actual hook you are referring to. As far as other documentation, you could also add a note to the Manage Display tab for Library Item (/admin/config/content/paragraphs_library_item/display) to explain that nothing will change however you have these settings. Which gets to my actual issue... Let me explain my use case a little further:

---

Let's say I have 3 Display modes for my paragraphs, Form Summary, Full, Teaser. Form Summary is or the node edit forms, Full is for the Full node display and Teaser is for the Teaser node display. With some complex Paragraphs, I like to have the Form Summary display mode to show/hide exactly what I want and still keep the vertical height of the forms as small as possible. Also, I use paragraphs field for header options, which is also why I show them on a Teasers sometimes.

The module as currently coded with paragraphs_library_preprocess_paragraph() will always display Library items in `default` display mode in all 3 cases. As I was trying to enable the Library on my current project, the default view mode was showing up in my forms, instead of the Form Summary. At first I figured I would just need to set the Display modes. As I messed with those and the settings, nothing changed. I finally followed the trail to paragraphs_library_preprocess_paragraph().

This is why I say paragraphs_library_preprocess_paragraph() is "hijacking" the normal process. It is Drupal convention to be able configure an entity to display a certain way and have it actually display that way on the "Front end". Also, there is no setting, hook or template to "undo" what is happening. I could create my own hook_preprocess_paragraph() and totally re-do what Drupal already did before paragraphs_library_preprocess_paragraph() ran... but this is pretty hacky in my opinion and not quite the "Drupal way".

@Bedir, I understand why you would want to force no labels and only render the attached paragraph. It simplifies things out of the box. And most sites can get up and running without having to configure the crap out of anything... But for the others, they either need to patch the module or adding more technical debt to get it to work within their site. (So far for my project, I am just using the patch above to remove that).

After thinking about this a little more, I do think I have a proposed solution (but no patch yet - i'll try and work on it):

  • inparagraphs_library_preprocess_paragraph() Instead of overriding $variables['content'], you could just add a new variable, say $variables['library'].
  • create a new template for library item paragraphs, and only render that only 'library' instead of 'content'.

this would not override everything Drupal has already done in 'content' AND other devs can create their own template overrides to do what they want...

I think thats all I have for now...thanks again for all the work on tis module, I'll try and work on a patch soon to sho what is going on.

pingevt’s picture

StatusFileSize
new5.53 KB

Attached is a patch with that option... if you just enable the module, should work the same way... and gives devs the ability to do other things with hooks, hook_preprocess_paragraph(), or templates.

miro_dietiker’s picture

Priority: Normal » Major

@pingevt Thank you for the inputs.

I agree, maybe it would have been better to provide a new preprocess variable instead of hijacking content. But your proposal is breaking our current (stable) contract and with it most custom library templates.

For the sake of stability, we can maybe provide the original content under a new name. But i'll still discuss quickly if restoring content is a route we can go down.

I'm not sure if we are in scope of the original issue. It was a simple obvious bug that we forgot to pass on the view mode.
I'm promoting this issue as the loss of a view mode is really annoying and it confused me also already.

But now you are proposing to change variables, templates, ... and i can't see anymore from your code if the view mode was really passed on.
And we still don't have a single test that shows the bug and maybe other aspects how it could appear.

Can you please help write a test that checks for the view mode situation. Otherwise we can't commit this.

pingevt’s picture

@miro_dietiker - All makes sense... and yes, that last patch wasn't necessarily meant to be committed, more proof of concept, but seeing what you said about stability totally makes sense. And I think providing the original content under a new name would actually solve the problem and keep things backwards compatible (Plus get rid of all the template stuff). Do you want me to create a new ticket to deal with the original content, or keep that all in here?

I'm a little newer to writing tests, so may have to run a few things by you... but let me see what I can do.

primsi’s picture

Status: Needs work » Needs review
StatusFileSize
new3.67 KB
new4.72 KB

Patch atop of #2, because otherwise aren't we using the view mode from the host entity? I might be wrong though, because I am a bit confused with all this levels of references :)

The last submitted patch, 15: paragraphs-library-item-view-mode-2948658-15.testonly.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

pingevt’s picture

StatusFileSize
new4.93 KB

This all looks good to me, I just added saving the original content in a new variable "content_original".

Status: Needs review » Needs work

The last submitted patch, 17: 2948658-view-modes-17.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

pingevt’s picture

Well, not sure why that failed... my testing environment is not totally working at the moment. I'll have to get back to that.

berdir’s picture

Status: Needs work » Needs review
Issue tags: -Need tests

That looks like a random fail, asserting on times is always suspicious.

That said, can you create a new issue for your use case/scenario? I slightly revised my opinion and I'm open to exploring your use case but I'd prefer doing so in a new issue. This is a bugfix for the the way things currently work and I'd like to get that in as a first step and then look for a clean approach to support what you need.

The last submitted patch, 8: 2948658-view-modes-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

The last submitted patch, 12: 2948658-rearrange-12.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Berdir committed 92ee5b2 on 8.x-1.x authored by Primsi
    Issue #2948658 by pingevt, Primsi, l0ke, valthebald: Paragraphs Library...
berdir’s picture

Status: Needs review » Fixed

Committed #15.

Status: Fixed » Closed (fixed)

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

juanramonperez’s picture

The patch is not working for me, I did the following in a clean installation

1. Created a paragraph type called featured_item and created a view mode Card for it
2. Created a view mode Card for from_library paragraph type
3. On that view mode I configured the field Reusable paragraph to display Rendered entity as Card
4. Added a field Components for Page content type and configured the view mode to display Rendered entity as Card
5. Go to paragraphs list and create one for reuse.
6. Create a new Page and use the paragraph just created.

Expected result, the Featured Item is rendered as Card
Actual result, the Featured Item is rendered as Default

I had to change the hook a little bit to have this working

/**
 * This hook is based on the patch https://www.drupal.org/project/paragraphs/issues/2948658
 *
 * Implements hook_preprocess_HOOK().
 */
function my_module_preprocess_paragraph(&$variables) {
  if (isset($variables['paragraph']) && $variables['paragraph']->getType() == 'from_library' && $variables['paragraph']->hasField('field_reusable_paragraph') && $variables['paragraph']->field_reusable_paragraph->entity) {
    $library_item = $variables['paragraph']->field_reusable_paragraph->entity;
    // Only replace the content if access is allowed to the library. Access
    // cacheability metadata is already returned by the original widget in case
    // access is not allowed.
    if ($library_item->access('view')) {
      $view_builder = \Drupal::entityTypeManager()
        ->getViewBuilder('paragraphs_library_item');
      $library_item_render_array = $view_builder->view($library_item, $variables['elements']['field_reusable_paragraph'][0]['#view_mode']);
      // This will remove all fields other then field_reusable_paragraph.
      $build = $view_builder->build($library_item_render_array);
      if (!empty($build['paragraphs'])) {
        foreach($build['paragraphs'] as $key => $paragraph) {
          if(is_int($key)) {
            $build['paragraphs'][$key]['#view_mode'] = $build['paragraphs']['#view_mode'];
          }
        }
        $variables['content'] = $build['paragraphs'];
      }
    }
  }
}

The only thing I added to the original hook is the `foreach` part

mfrosch’s picture

For me it's also not solved.
The Library Item itself get displayed in a specific viewmode.
But the referenced Paragraphs always get rendered as default.

Testet in latest Stable and dev Release:

  • 8.x-1.12
  • 8.x-1.x-dev updated 28 Jun 2020 at 10:39 UTC

As it looks like I am not the only one I suggest to reopen this issue.

mfrosch’s picture

StatusFileSize
new836 bytes

I've did a little research and solved the issue for me.
Attached a little patch based von 8.x-1.12.

Seems like loading the paragraph with a view mode wasn't enough.
$build['paragraphs']['#view_mode'] was the desired view mode.
$build['paragraphs'][0]['#view_mode'] was still the "default" view mode.
Overwriting this solved it for me.

berdir’s picture

Please open a new issue. This has a test, if you think this isn't working, we'll need test coverage that proves that.

megha_kundar’s picture

Above patch #28 Worked for me.

volker23’s picture

I have tried the patch #28, but unfortunately it did not work. Referenced Paragraph Item still shows in default view mode. Drupal 9.2.10, Paragraphs 8.x-1.12