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.
Problem/Motivation
Follow-up of #3109571: Hide "Add above" when switching to the behavior tab
Currently, the Add Above feature works only with the Modal Add mode. If the Add Mode is set to other than Modal, the form submits empty values and refreshes making the user lose all data.
Proposed resolution
Either just allowing Add Mode when Moda is the Add Mode set or make the Add Above feature work with the other Add modes.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#21 | interdiff-3133439-18-21.txt | 2.32 KB | johnchque |
#21 | 3133439-disable-add-above-modal-21.patch | 17.61 KB | johnchque |
| |||
#21 | 3133439-disable-add-above-modal-21-NO-TESTS.patch | 7.13 KB | johnchque |
#18 | interdiff-3133439-16-18.txt | 2.72 KB | johnchque |
#18 | 3133439-disable-add-above-modal-18.patch | 15.14 KB | johnchque |
Comments
Comment #2
johnchqueThis should work! Let's see about tests.
Comment #3
johnchqueSo I think we can trigger the add button when we have it there. Had to move the delta hidden field so we can add above in other scenarios.
Comment #5
johnchqueFinally found a way to move the delta textfield and updating the JS. :)
Comment #7
BerdirWe need to figure out why the existing test breaks and add test coverage for the extra mode where this is now supposed to work.
Comment #8
johnchqueIt seems I just had to make some other small adjustments. As we are changing the delta class, that is what was causing the errors.
Comment #9
johnchqueAdding some comments here and there. Also added a no tests patch so I can use it in some projects.
Comment #11
sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commentedHere is the patch that fixes the form validation issue and page refresh issue. I also added a check for the number of buttons, I guess we want to trigger the click to add above only if there is only one button/paragraph that can be added? We're still left with the original issue I see, that the Add above button is still visible for multiple dropdown buttons and it doesn't work atm - shouldn't it be hidden?
Comment #12
Berdir> Add above button is still visible for multiple dropdown buttons and it doesn't work atm - shouldn't it be hidden?
Correct, we only support the case with one button atm, so we need to check for that.
Comment #13
BerdirComment #14
sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commentedAdded the check for the number of buttons to the paragraphsAddAboveButton() function now. Should work as expected.
Comment #16
johnchqueHmm so I think we have to count the add buttons in a different way as the Add buttons in the modal have the same class that we are checking. I added an extra check for that, this should work. :)
Comment #17
sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commentedCan we add a more specific class to the buttons in the code where they get added (clearfix)? Reading this line is not very obvious what we are looking for. Also not very happy with the closest selector (table), but works in this case.
Typos. :)
These two cases can be merged with a OR selector.
Comment #18
johnchqueOh wow, my bad for the typo. Addressing suggestions. :) This should fix the tests and should make the class selector easier to understand.
Comment #21
johnchqueFixing tests. :)
Comment #23
sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commentedTested again locally with the latest patch. Seems to work fine now.
Maybe another code review from @Berdir before merging?
Comment #24
BerdirThanks, committed.