Closed (fixed)
Project:
Layout Paragraphs
Version:
2.0.x-dev
Component:
User interface
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
28 Apr 2021 at 14:53 UTC
Updated:
4 Dec 2023 at 07:57 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
marcom2021 commentedComment #3
anybodyHi @justin2pin,
could you perhaps have a look and write down what's the current expected status (and perhaps plan for 2.x) for the Paragraphs Library compatibility of Layout Paragraphs?
For many projects it's quite important to be able to reuse paragraphs from the library, but it doesn't seem to work well yet. Is this on the roadmap for 2.x?
Thanks a lot in advance!
Comment #4
anybodyShould be checked / implemented against 2.x!
Comment #5
anybody@justin2pin: Is this on the roadmap? For larger sites with resuable paragraphs this would be really really handy and helpful. Currently it this paragraphs submodule doesn't seem to be working with layout_paragraphs 2.x?
Comment #6
justin2pin commentedHey Anybody! Yes this is (or needs to be). I'll add it to the current roadmap issue. Thanks!
Comment #7
anybodyThank you very much @justin2pin, so much looking forward to the 2.x release, you're doing a great job for Drupal here!
Comment #9
justin2pin commentedComment #10
anybodyWHAO @justin2pin, another outstanding feature!
Great work once again, here's my feedback from manual review:
1. Let's start with the question if submodule or not:

First we should mention, that Paragraphs Library is also a submodule of paragraphs (even with the further dependency on entity_usage):
Which means, that the module might not be enabled in all environments and we have to ensure that all related functionality only affects cases where the module is enabled.
That might be a good reason to also split layout_paragraphs_library off from the main module for two reasons:
But it's also acceptable to include it in the main module code, if it doesn't lead to code clutter because of the dependency checking or possible errors / side effects if paragraphs_library is /not enabled.
Thinking about these points, I'd personally prefer the submodule!
2. Paragraphs displayed twice in node view (eventually unrelated)
This is the most major problem I found:
All paragraphs are displayed twice, below different parent structures:
Output (node view):

Also mind the strange DOM structure, it's not just a duplication. I didn't use layout_builder on that node or similar!
Input (node edit):

I've sent you a link by PM to the simplytest.me sandbox with the latest 2.x-dev version of the module and MR!53 applied.
(As I didn't test without the patch, it might be possible that this is unrelated)
3. Admin UX optimization suggestions

3.a)
Presumably, layout_paragraphs should hide the checkbox (by form state?) to add the layout paragraph type itself to the library? As I don't think that would make any sense and might produce unexpected results?
3.b) This might also be unrelated, but the message "Behaviour plugins are only supported by the EXPERIMENTAL paragraphs widget" in the Layout Paragraph Type is confusing:


The widget to use with layout_paragraphs intentionally is layout_paragraphs:
I don't know if it already appeared without the patch, and I guess it comes from paragraphs module, but if possible layout_paragraphs should hide it or extend the message to also mention layout_paragraphs.
PS: Setting up the fresh test environment and going through all the currently always required initial setups steps, convinced me that once the module is stable, #3254889: Provide better first-use experience (Onboarding) would really help users trying the module and developers testing the module to speed things up and prevent mistakes and frustration. But you've already done so much for this module and the Drupal community that this might be a task for someone else ;)
4. Content editor UX optimization suggestions
I know this isn't a layout_paragraphs issue directly, but the combination of the preconfigured "From Library" paragraph type from Paragraphs Library module and the "Delete" message is confusing for users.
If the "F" wasn't capital, you had no chance to understand that you're not deleting the paragraph FROM THE LIBRARY!
So the combination of the delete message and the placeholder is problematic here. Perhaps quotation marks would be a good first step but I think there might be better ways to work around this... I hadn't had a good idea yet.
To sum it up: GREAT GREAT WORK! Honestly!! Whao! :)
Comment #11
justin2pin commented@Anybody - thanks for the detailed feedback! Some responses:
Also: we still need tests for this. Will work on that.
Comment #12
johnpitcairn commentedI don't agree with point #3. Shouldn't layout paragraphs honor that setting and not provide the "add to library" button on that paragraph type?
I definitely have some paragraph types I don't want editors to re-use.
Comment #13
justin2pin commented@John Pitcairn I'm not sure what you mean. I don't think we can support allowing Section paragraph types to be added to the library -- it's too complex and raises a number of concerns in the UI. We're now just removing that checkbox -- which wouldn't have done anything anyway -- from the paragraphs type form for Section types (and Section paragraph types only).
Comment #14
johnpitcairn commentedAh I see, my mistake, apologies.
It would be great to allow fully populated sections to be added to the library, for a really slick speedy page building experience, but I agree that would present some issues. A future feature request?
Comment #15
anybody@justin2pin:
Re #11:
1. Cool! :)
2. My mistake!! I didn't switch to Layout Paragrahs in Manage Display -.- only for the widget... Any chance to show a warning in such a case where "Manage form display" uses Layout Paragraphs, but display not?
3. a) Cool! :)
b) Ok, I created a separate issue for that: #3257408: Confusing message "Behaviour plugins are only supported by the EXPERIMENTAL paragraphs widget" in Layout Paragraph Type
4. Would you create an upstream issue at paragraphs(_library)? Is the modal message also from paragraphs? Otherwise, I'd also suggest turning around the text and placeholders a bit to prevent confusion?
@John Pitcairn: Aah now I got your point. Thought the same as @justin2pin in #13, but well... Saving whole sections to the library... well that's not too abstract idea and might be useful indeed. So I'd vote for a separate, well documented, far far future minor feature request issue. What do you think @justin2pin?
Until that, I guess, the option should definitely be hidden to prevent problems!
Comment #16
justin2pin commented@Anybody - thanks for this. I added #3257666: Misleading dialog title for "Delete" action. At this point I think all this just needs test coverage before it should be marked as fixed.
Comment #17
justin2pin commentedTest coverage added.
Comment #19
anybodyThank you, @justin2pin, great work once again! I agree this can be set fixed as soon as the tests go green! :)
Any ideas to prevent the configuration mistake I made?
Guess I might not be the only (stupid) one and we're using layout_pargraphs since month... I guess the main problem is, that you don't have to switch the display for paragraphs (without layout builder) so users might not be used to that.
What do you think about simply documenting this "problem" here: #3254889: Provide better first-use experience (Onboarding) ?
Comment #20
justin2pin commentedThanks @Anybody!
The MR was failing tests because of incomplete dev dependencies in composer.json. That's fixed, merged, and passing tests in 2.0.x-dev. Marking as fixed.
Comment #24
anybodyOkay funny, we're now running into exactly the case discussed above in #15 by @John Pitcairn:
which was based on 3a in #10.
While it was a good idea to hide the checkbox for now as it's not supported yet, we should indeed create a follow-up feature request to support adding layouts to the paragraphs library as quite often this is exactly what you'd like to reuse!
I'll do that and link it here later. Combined with important library related UI improvements.
Comment #25
anybodyFollow-up as of #24: #3274985: Allow adding Layouts (incl. children) to Paragraphs library
Comment #26
dippers commentedA possible workaround for storing layout paragraphs in library items has been proposed at #3405737: Add Layout Paragraphs to Paragraphs Library.
Comment #27
thomas.frobieter