Problem/Motivation

With the changes of #2882783: Hide add paragraph buttons when viewing behaviors perspective the add button is hidden when the user switches to the Behavior tab.
As the Add above button is still there, when pressing it the modal is open without any buttons.

Proposed resolution

Either hide the add above button or make the modal work on the behavior tab for that action.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#14 interdiff-3109571-12-14.txt4.35 KBjohnchque
#14 3109571-hide-add-above-behavior-14.patch9.07 KBjohnchque
#14 3109571-hide-add-above-behavior-14-TEST-ONLY.patch5.64 KBjohnchque
#12 interdiff-3109571-10-12.txt3.77 KBjohnchque
#12 3109571-hide-add-above-behavior-12.patch4.39 KBjohnchque
#10 3109571-hide-add-above-behavior-10-interdiff.txt1.17 KBsasanikolic
#10 3109571-hide-add-above-behavior-10.patch2.52 KBsasanikolic
#8 interdiff-3109571-6-8.txt2.17 KBjohnchque
#8 3109571-hide-add-above-behavior-8.patch2.37 KBjohnchque
#6 interdiff-3109571-4-6.txt844 bytesjohnchque
#6 3109571-hide-add-above-behavior-6.patch947 bytesjohnchque
#4 3109571-hide-add-above-behavior-4.patch893 bytesjohnchque
#2 3109571-hide-add-above-behavior.patch1.99 KBjohnchque
#2 3109571-hide-add-above-behavior-TEST-ONLY.patch1.21 KBjohnchque
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yongt9412 created an issue. See original summary.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
Status: Active » Needs review
FileSize
3109571-hide-add-above-behavior-TEST-ONLY.patch1.21 KB
3109571-hide-add-above-behavior.patch1.99 KB

This should work. :)

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented
Status: Needs review » Needs work

I wasn‘t sure when we were starting to hide those add buttons.

Now agree, if we hide the add button on the behavior tab, we should maybe also hide the action.

However if we hide the „add“ operation, we should similarly hide the „remove“.
And then you realise that maybe most actions interact with the paragraph structure. Should we then hide all of them?

This is against the idea why we introduced the actions. It should be a well collapsed element to declutter the UI that should bring orientation and consistency. This means it is not allowed to vary.

We should imho start with making the overlay work again. I think it worked not long ago. Then we should decide when to hide what consistently.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
3109571-hide-add-above-behavior-4.patch893 bytes

New approach. As you suggested it doesn't make much sense to hide the actions. Instead, I tried removing the class that hides the Buttons from the Modal add form when in the Behavior Tab.

However, this made me realize that we have another bug here. The Add above feature does not work with the Add method set as Dropdown. When clicking Add above there is not js executed and we submit the whole form.

I believe that that should be fixed in another issue but it is worth noting it here.

I will add tests soon.

Status: Needs review » Needs work

The last submitted patch, 4: 3109571-hide-add-above-behavior-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
3109571-hide-add-above-behavior-6.patch947 bytes
interdiff-3109571-4-6.txt844 bytes

Hmm I found a problem here. We basically add the paragraphs-add-wrapper class to all Add buttons in all Adding modes. This hides them all when switching to the Behavior tab, however when I tried removing that class, if we have only one paragraph type available to add, there will be no outside wrapper and the button will require the class to be hidden.

I added that check so the class can be added for alone buttons. I tried adding the class in the outside element but I think it does not have a type and does not recognize to add classes.

Status: Needs review » Needs work

The last submitted patch, 6: 3109571-hide-add-above-behavior-6.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
3109571-hide-add-above-behavior-8.patch2.37 KB
interdiff-3109571-6-8.txt2.17 KB

OK, so I got a bit confused about why we wanted to hide the Add button initially.
Then I found this issue: #2882783: Hide add paragraph buttons when viewing behaviors perspective that later introduced a bug with the dropdown buttons, fixed in #3104790: Dropdown button broken when toggling Behavior tab.

However, this lead to adding a wrapper class to each adding button and thus, hidden when in the behavior tab. The problem is that as those buttons had that class added when Adding above in the behavior tab, the buttons in the modal form were hidden as well.

sasanikolic’s picture

sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commented
Status: Needs review » Needs work 
+++ b/src/Plugin/Field/FieldWidget/ParagraphsWidget.php
@@ -1594,14 +1594,15 @@ class ParagraphsWidget extends WidgetBase {
+    // If there is only one allowed paragraph type, add the wrapper class.
+    $wrapper_class = (count($options) == 1 && $add_mode == 'dropdown') ? 'paragraphs-add-wrapper' : '';

I am not sure I understand this comment and check. Mind to explain it better?

I think we should just prevent that class to be added to the modal itself. If that's not possible, we should fix the css/js selectors. Let's check again together then.

However, this patch seems to fix the issue but I'm unsure of the currently implemented solution.

sasanikolic’s picture

sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
3109571-hide-add-above-behavior-10.patch2.52 KB
3109571-hide-add-above-behavior-10-interdiff.txt1.17 KB

Ivestigated a bit, I see what you mean now - having one allowed paragraph is an edge case that we didn't cover yet.
I improved the check a bit, since it didn't work properly for the other add modes and extended the comment. @yongt9412 what do you think?

sasanikolic’s picture

sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commented

Note that this does not hide the "Add above" button on behavior tab as written in the issue title, but instead makes the modal work - display paragraph buttons on behavior tab. So I guess we need to rename the issue, if we go with this solution.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
FileSize
3109571-hide-add-above-behavior-12.patch4.39 KB
interdiff-3109571-10-12.txt3.77 KB

So discussed with @sasanikolic. We saw that there was an inconsistency in the classes and containers wrapping the add actions in the different forms.

The patch basically sets all of them as containers and adds the wrapper class that will hide them in the Behavior perspective tab. The good side of this approach is that we set that class only once independently of the add mode we have set.

If our tests are correct, they shouldn't break, unless we do assert buttons with that class which would be semantically wrong. Let's see what testbot has to say. :)

Status: Needs review » Needs work

The last submitted patch, 12: 3109571-hide-add-above-behavior-12.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
3109571-hide-add-above-behavior-14-TEST-ONLY.patch5.64 KB
3109571-hide-add-above-behavior-14.patch9.07 KB
interdiff-3109571-12-14.txt4.35 KB

Adding and fixing tests. :)

The last submitted patch, 14: 3109571-hide-add-above-behavior-14-TEST-ONLY.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

sasanikolic’s picture

sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commented
Status: Needs review » Reviewed & tested by the community

Looks pretty good to me now. Tested all add modes and works fine.

Maybe as a followup we should disable the click action on the "Add above" button when the add mode is other than the Modal, as in that case the page refreshes and the content changes get lost?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Can you open an issue for that? Kind of seems like a more severe bug than this.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented

Follow-up created #3133439: Disable Add Above when Add Mode is other than Modal.

  • Berdir committed 6611f51 on 8.x-1.x authored by yongt9412 
    Issue #3109571 by yongt9412, sasanikolic: Hide "Add above" when...
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Reviewed & tested by the community » Fixed

Thanks.

Status: Fixed » Closed (fixed)

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