Problem/Motivation

As soon as a layout paragraph to be added has a validation error, it's no more possible to save the layout paragraph modal.

Steps to reproduce

This can be easily reproduced with a text paragraph:

  1. Create a paragraph with a text field. Set the textfield to be required.
  2. Add this text paragraph to a node with layout paragraphs, but don't fill any text and click "Save". A validation error pops up correctly, saying the value is required.
  3. Now fill in text to fix the error and try to click "Save" again. Nothing happens.

Proposed resolution

The modal action buttons have to be updated on content validation errors.

Remaining tasks

User interface changes

API changes

Data model changes

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Tag1 supports Drupal development!Tag1 logo

Comments

Anybody created an issue. See original summary.

Anybody’s picture

Issue summary: View changes
Anybody’s picture

FileSize
65.66 KB
96.6 KB

Okay, I found the reason and an ugly workaround until this is fixed. Here are screenshots of the reproduction as written above:

Create a text paragraph and leave the required field empty:

This triggers a validation error:

The green buttons are appearing with the validation error and THEY WORK! The modal action buttons are both NOT WORKING anymore. So until this is fixed, you can click the newly added ones.

I'll leave this "Major" anyway as users might not see the workaround-buttons and this breaks core functionality of the module.

Anybody’s picture

Title: Modal form can't be submitted anymore once a subform has a validation error » Modal form actions broken once a subform has a validation error

Peacog made their first commit to this issue’s fork.

Peacog’s picture

Status: Active » Needs review

I agree that this is major since with long forms the user will not see the original form buttons unless they scroll down to the bottom of the form. The problem is caused by the fact that the original buttons are recreated by the ajax callback and have ids containing a different randomly generated section for example edit-field-paragraphs-actions-submit--6FRd8hbaqyk. I've created a patch that looks for the buttons by excluding the randomly generated part of the id.

pp.panatom’s picture

Thanks, patch #6 works for me.
The buttons are still "duplicated".

InaW’s picture

Issue summary: View changes
FileSize
59.86 KB

Any Updates on this issue? I also still see the action button twice with the Patch #6

nicross-com’s picture

Title: Modal form actions broken once a subform has a validation error » Modal form actions broken if subform has validation errors or certain AJAX callbacks

#6 allows the actions to be clickable again. However, as mentioned, the duplicated buttons persist.

Here is another path to reproduce this issue. If a form element has an AJAX callback which alters the entire form, then its actions will be duplicated as well. A workaround is using a more specific wrapper element than the <form> itself. I mention this because AJAX seems to be the general cause of this issue, not specific to validation errors:

$form['field_example']['#ajax'] = [
  'callback' => 'my_ajax_callback',
  'trigger' => 'change',
  'wrapper' => $form['#id'],
];

function my_ajax_callback(&$form, FormStateInterface $form_state): array {
  return $form;
}

My first thought was that the patch could simply add a line to remove the new actions from the DOM. But then I considered a use case where one might alter those actions in an AJAX callback for whatever reason. So might we replace the old actions with the new ones instead? Unfortunately I can't speak to the complexity of that solution.

nicross-com changed the visibility of the branch 3265794-modal-form-actions to hidden.

nicross-com changed the visibility of the branch 3265794-modal-form-actions to active.

nicross-com’s picture

Here's my solution in builder.js which moves code from the dialog:aftercreate event handler into a reusable updateDialogButtons() function. We also call it within the behavior attachment so it's called after AJAX complete:

Drupal.behaviors.layoutParagraphsBuilder = {
  // ...
  updateDialogButtons();
}

var $lpDialog;

$(window).on('dialog:aftercreate', function (event, dialog, $dialog) {
  if ($dialog.attr('id').indexOf('lpb-dialog-') === 0) {
    $lpDialog = $dialog;
    updateDialogButtons();
  }
});

$(window).on('dialog:afterclose', function () {
  if ($lpDialog && $dialog[0] === $lpDialog[0]) {
    $lpDialog = undefined;
  }
});

function updateDialogButtons() {
  // ...
}

Sorry about misclicking the hide button on the old issue fork. I'll try moving this into a new issue fork with ES6 as well. Please feel free to clean this up or fix edge cases. I'm still learning the Drupal JS standards and how to contribute to this project.

nicross-com’s picture

I think I've taken this as far as I can. I don't have the ability to autogenerate the JS from ES6 source, so I did that by hand. Thanks!

Edit: I figured out the issue with the patch not applying cleanly to dev for me with composer. I needed to use the diff and not the patch. It works fine on my site.

NathanDanielsonCOM’s picture

Our team has been doing in-depth testing of this patch with a wide range of our paragraphs, some very simple and some complex, without running into the duplicated Save/Cancel bug. We've tried various scenarios without seeing any unintended issues and are planning to take this patch into a large production site. Does anyone else have any testing feedback?

CbStuart’s picture

FileSize
7.92 KB

Composer does not like the patch file for Merge Request 151 so I've recreated it. #151

sethhill made their first commit to this issue’s fork.

sethhill’s picture

The above approach by @nicross-com of updating the dialog buttons works as expected. In MR156 I refactored that approach to avoid setting the global variable to track the dialog, and addressed some ESLint errors that were reported in the builder.

justin2pin made their first commit to this issue’s fork.

  • justin2pin committed 9e6ce297 on 2.0.x authored by sethhill
    Issue #3265794 by nicross-com, sethhill, Peacog, CbStuart, Anybody, InaW...
justin2pin’s picture

Status: Needs review » Fixed

Merged MR156 – Thanks all!

Status: Fixed » Closed (fixed)

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

berliner’s picture

The patches have been a bit greedy though. Instead of only moving the form action submit buttons and button links into the dialogs button pane, it now moves all submit buttons and button links there.
So it's now doing exactly what it was meant not to do, according to this comment: https://git.drupalcode.org/project/layout_paragraphs/-/commit/9e6ce29756...

I have raised #3442062: Dialog form button logic is too greedy as a follow-up issue to fix this.