[DS-1327] Add the Layout builder copy/paste section module #215
Conversation
composer.json
Outdated
| @@ -16,6 +16,7 @@ | |||
| "drupal/lb_accordion" : "^2.1.0", | |||
| "drupal/lb_cards" : "^2.0.4", | |||
| "drupal/lb_carousel" : "^2.0.3", | |||
| "drupal/lb_copy_section" : "^1.0", | |||
There was a problem hiding this comment.
what about upgrade path?
We need to add this module to standard|extended profile variation and enable it via hook_update_N @aleevas
There was a problem hiding this comment.
Hi @podarok, we think this might overload the interface for some YMCAs who might not need it, that's why we made it optional, but if it's necessary then so be it.
There was a problem hiding this comment.
I agree with @podarok. If we're not installing and configuring this module, then we should not include it in composer and simply document the installation and use. I'm checking in with Shelley to discuss the best path forward.
There was a problem hiding this comment.
@rollki I've discussed with Shelley and also tested the module and I think it's ok to go ahead with, but we need to test and ensure it works well before committing.
I tested and it seems there are some issues with cloning. My test steps:
- go to the homepage
- clone the body section
- edit the cloned block
- BUG 1: Nested entity references are not editable.
- BUG 2: Creating a new sub-block and attempting to save results in the error "This entity could not be referenced"
- I ran into this on another project and had to add two different patches and make some changes in how entity references work. It will probably take more work to make complete. https://www.drupal.org/node/3098245 https://www.drupal.org/project/inline_entity_form/issues/3002175
- BUG 3: "Copy" text is larger than "Configure"
There was a problem hiding this comment.
@froboy the issue with cloned blocks should be fixed in this PR YCloudYUSA/y_lb_demo_content#45
The Copy Layout Builder was added to the OpenY profile YCloudYUSA/yusaopeny#188
And this module was removed from composer of the Y LB module 7195418d50986862c0431a37f2e37683631d0f8f
There was a problem hiding this comment.
Ok, I think this solves the current problems even though it affects a few modules. There is probably no simple solution.
But now since we make blocks for LB reusable at YCloudYUSA/y_lb_demo_content#45 it means that these cloned blocks will appear in the "All system blocks" section when adding a block to LB, there mainly system blocks and probably few people use these blocks, but here is the question: will it be ok for us?
сс @froboy
There was a problem hiding this comment.
@rollki I strongly disagree with this approach.
as you say:
it means that these cloned blocks will appear in the "All system blocks" section when adding a block to LB...
This means that dozens or hundreds of blocks will be listed in "All system blocks". This is a very bad idea and will cause chaos for content editors.
There was a problem hiding this comment.
See my comment above:
I ran into this on another project and had to add two different patches and make some changes in how entity references work. It will probably take more work to make complete.
https://www.drupal.org/node/3098245
https://www.drupal.org/project/inline_entity_form/issues/3002175
We'll probably need some patches to get this to work (which will resolve other issues related to cloning) if we still want to proceed down this path. I'm not sure though.
There was a problem hiding this comment.
I agree that making reusable blocks is not a solution considering that new blocks may appear and it will be necessary to keep in mind that they must also be reusable + that there will be too many inline blocks.
In this case, we need to try to apply these patches, but there is a high probability that they will need to be modified and updated. сс @aleevas
@froboy please confirm if this sounds good to you. Also, PR YCloudYUSA/y_lb_demo_content#45 is probably not needed.
Note: Patching may take some time, which means this feature probably won't make it to the current release.
Jira issue: DS-1327
Steps for review: