Use Drupal dialog instead of directly using jQuery dialog for add modal

Had a quick discussion with @miro_dietiker about this. We are not 100% sure but it does make sense that we should use Drupal Dialog API instead of jQuery dialog directly.

It seems that we have a dependency to destroy jQuery dialog and that's not available in the Drupal replacement and that is maybe a reason why paragraphs is using jQuery dialog directly instead of going over Drupal API.

It's worth to investigate this more and figure can we switch to Drupal Dialog API. Re-opening the issue.

This fixes a problem where the modal window did not close properly.

I think we have to call close() function in order to trigger all the events that Drupal dialog implemenation invokes.

Will check this a bit.

Here is a new patch based on #7 that

- Cleans/improve code a bit,
- Adds a lot of comments explaining what is going on,
- Fix multiple initializations of .field-add-more-submit elements on each dialog open,
- Enables draggable again when using Drupal Dialog API.

New patch, changes:

- Converted from custom close jQuery UI dialog callback to dialog:afterclose event,
- Fixed issue with core dialog.ajax.js removing .paragraphs-add-dialog element in some cases,
- Various fixes and code cleanup in paragraphs.add_above_button.js file. The most important fix is that behaviour is not running all the time and attaching events.

This seems the tests are failing and this seems to be affecting the nested Paragraphs too.

Steps to reproduce:
- Add a nested paragraph via a modal
- Use "Add above" on the container/nested paragraph to add a new paragraph above the container
- There are 2 dialogues present.

> and this seems to be affecting the nested Paragraphs too.

A new patch should fix this. Let's check what tests are reporting now.

This should fix the tests from #13 in Drupal\Tests\paragraphs\FunctionalJavascript\ParagraphsExperimentalWidgetElementsTest.

However, I think the test fail in Drupal\Tests\paragraphs\FunctionalJavascript\ParagraphsExperimentalAddWidgetTest is an actual bug.

Steps to reproduce the bug:

1. Add a new node
2. Add a text paragraph
3. Click Add above on the text paragraph action and close it
4. Click the main Add Paragraph field on the node and add a user paragraph
5. The user paragraph was inserted before the text paragraph.

Expected behavior:

1. Add a new node
2. Add a text paragraph
3. Click Add above on the text paragraph action and close it
4. Click the main Add Paragraph field on the node and add a user paragraph
5. The user paragraph was inserted after the text paragraph.

I‘m confused here.

If you cancel an „add before“ operation then the next regular add should add at the regular end position. Not before anything. Why should this be a bug?

I agree.

"Steps to reproduce" in #14 are the steps to reproduce the bug. I edited comment #14 to make it clear.

Settings to "Needs work" to address #14.

The other two test fails are going to be fixed after #3109279: Fix entity usage assertions in ParagraphsLibraryItemTest fails.

> Settings to "Needs work" to address #14.

Should be fixed now.

1. +++ b/js/paragraphs.add_above_button.js
   @@ -8,44 +8,44 @@
   +   * * @type {Drupal~behavior}
   

   One extra * here.

2. +++ b/js/paragraphs.add_above_button.js
   @@ -8,44 +8,44 @@
   +          var $button = $('<input class="paragraphs-dropdown-action paragraphs-dropdown-action--add-above button js-form-submit form-submit" type="submit" value="' + Drupal.t('Add above') + '">');
   

   Can this be done in a preprocess or somewhere else? Creating form elements, especially with fixed class names like that with js is a bad practice imho. Maybe a followup since it's not so much related to this ticket.

Fixed the nitpick here. Apart from the 2. comment above, looks good to me.

> Can this be done in a preprocess or somewhere else?

Probably, i don't really remember why this part is in JS. But definitely this is a separate follow-up issue if we want to improve this, @miro_dietiker?

No new feedback for latest patch, let's try to commit this.

Going to trust you guys here, don't make me regret that :p

I did ignore one change in paragraphs.libraries.yml that's just formatting on a part that we'll remove soon.

Just FYI: this broke our test suite when we tried to add paragraphs in DTT tests. We needed to replicate the logic change that was done to the JS tests in this patch to get it back to green.

The fix is not compatible with Drupal 9.1 with the Claro admin theme. See https://www.drupal.org/project/drupal/issues/3191527

The patch in #27 is pretty much guaranteed not to land because

• This issue has been closed for over a year
• It’s not a change that is in scope with the requirements specified in the issue summary, it just happens to touch on code that was also touched here

Create a new issue with an issue summary describing what the patch is intended to address, and there’s an actual chance it will land. If this is just here so the patch is available to composer, it’s better to add the patch to a gist instead of adding noise a closed issue where it is out of scope.

Created a new issue for this, posted the patch there, #3221429: Enable autoResize for paragraphs modal