Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The "Remove" button should be removed from forms if "Required field" is true and the "Allowed number of values" field of the Entity Reference Revision (or some of fields of the referenced entity) is configured as "Limited" to 1... It does make sense to display this button if the paragraph type should always exists.
Also, it should be interesting to have an option in the of the "Manage form display" to always hide control buttons.
Comments
Comment #2
@baux CreditAttribution: @baux commentedComment #3
miro_dietikerIt does. If you have multiple allowed paragraph types with cardinality 1, we may add a default paragraph while you still can remove it and add a different one.
If the allowed types are limited to 1 though, we should remove the button. But i was pretty sure we already hide it in that case.
Plz chk and provide feedback.
Comment #4
@baux CreditAttribution: @baux commentedHi Miro,
you are right. I was not thinking about more than one allowed types, but if I have just one (which is my case), the system display the Remove button... It´s not an important issue, but it should be hided. Thanks
Comment #5
miro_dietikerOK then, yeah. Let's hide that button in this case.
Updating title to better identify the situation.
Comment #6
Daniel.Moberly CreditAttribution: Daniel.Moberly commentedAttaching a patch for this. Should check for both conditions.
Comment #7
johnchqueWe also need tests. :) And let's move this to dev. :)
Comment #8
santhosh.p CreditAttribution: santhosh.p at DrupalPartners commentedComment #9
santhosh.p CreditAttribution: santhosh.p at DrupalPartners commentedPatch #6 applied and tested successfully. After applying the patch #6 "Remove" button is removed from the form when "Required field" is true and the "Allowed number of values" is configured as "Limited" to 1.
Comment #10
santhosh.p CreditAttribution: santhosh.p at DrupalPartners commentedComment #11
miro_dietiker@santhosh.p Thank you for manually testing the patch. However, we do not accept RTBC patches without automated test coverage.Not all maintainers are that strict, but we are.
That's what @yongt9412 meant with "needs tests" and we always refer to.
See: https://www.drupal.org/issue-tags/special
Comment #12
CRZDEV CreditAttribution: CRZDEV at Metadrop commentedHi, i've checked patch #6 and seems not taking into account the required field configuration.
Remove button was not visible even with required field disabled.
Here goes a new patch with that fixed and adding some comments to explain the reason of the change.
Test are still pending so i'll keep the issue into needs work state. Please review.
Comment #13
miro_dietikerStill triggering the testbot once.
Comment #14
miro_dietikerFor the old widget (the code you touch) this would be a feature and it is feature frozen.
I was always talking of the new experimental widget.. (actually quite stable..) :-)
Comment #15
miro_dietikerFinally tested the new widget and yes, the Remove button really still shows with cardinality 1 and required.
So let's focus on that widget.
Comment #16
CRZDEV CreditAttribution: CRZDEV at Metadrop commentedHere goes a new version for experimental widget, also including test that may need some fixes.
Please review. Thanks!
Comment #18
CRZDEV CreditAttribution: CRZDEV at Metadrop commentedAdding new version with some test fixes.
Comment #20
CRZDEV CreditAttribution: CRZDEV as a volunteer and at Metadrop commentedComment #21
CRZDEV CreditAttribution: CRZDEV as a volunteer and at Metadrop commentedComment #22
BerdirI really don't like our usage of $paragraphs_entity. We don't do $node_entity either. Changing the existing variables is tricky as it would conflict, but lets not add new ones. Just $paragraph.
Comment #24
CRZDEV CreditAttribution: CRZDEV as a volunteer and at Metadrop commentedThanks for the reply Berdir, i used the variable name that is used in the whole paragraph widget file.
Maybe is a good idea to change it.
Comment #25
CRZDEV CreditAttribution: CRZDEV as a volunteer and at Metadrop commentedFixed some test exceptions
Comment #26
CRZDEV CreditAttribution: CRZDEV as a volunteer and at Metadrop commented@Berdir I completely agree with that, uploading new version with that variable name suggestion. Thanks!
Comment #27
johnchqueSolid patch, thank you! :) Please remember to upload interdiff files every time you upload a new patch. :)
Comment #29
miro_dietikerRewrote the access method a bit. The code is longer, but the logic is flattened. I prefer trivial-to-read early access methods.
And committed, thx all for helping. :-)
Comment #30
johnchqueWhen running tests I noticed something strange, this changes should make tests more reliable.
Comment #31
johnchque[Duplicated]
Comment #32
miro_dietikerBefore fixing "something strange" i like to hear what the problem was.
Likely needs a reroll anyway with the most recent commits.
Comment #33
johnchqueIndeed, rebasing.
Sorry, I should have been more specific.
There were two problems in tests. First, we had two paragraphs types created but we were suppose to have only one so we can check that the remove button when field cardinality is 1, required and only one allowed paragraph type, is not present.
Second, since the fixed bug was not being triggered, cause there are two paragraphs types, we never really checked if the remove button was present or not. So I removed the second paragraph type created and added an assertion for the text field that it is supposed to be shown. :)
Comment #34
BerdirAh yes, makes sense.
Comment #36
miro_dietikerAaaand committed! :-)
Comment #37
CRZDEV CreditAttribution: CRZDEV at Metadrop commentedThanks all guys and great work :)
Comment #39
JasmineWu CreditAttribution: JasmineWu commentedI try to hide 'Remove' button of paragraphs when field is required, and i patched #12 but failed. So i create a patch for V1.11
Comment #40
JasmineWu CreditAttribution: JasmineWu commentedComment #41
JasmineWu CreditAttribution: JasmineWu commentedComment #42
codechefmarc CreditAttribution: codechefmarc at Four Kitchens commentedWe also ran into this issue and the original fix didn't seem to make it work for us, but I added JasmineWu's patch #41 and that worked perfectly. I wonder if we need to open a new issue to get this patch included in a future release?
Comment #43
BerdirThat patch is changing the old legacy widget. If you want to use newer features, you need to use the new widget which has tons of improvements.
Comment #44
codechefmarc CreditAttribution: codechefmarc at Four Kitchens commentedGot it. We aren't using the Paragraphs widgets, we're using Layout Paragraphs widgets, so this patch will work for us for now. Thanks!