Allow to merge AjaxCommands inside of AjaxResponse [#3343670]

Problem/Motivation

We want to merge an AjaxResponse with another AjaxResponse and this currently not possible.

Inside of MediaLibraryWidget the code is returning an AjaxResponse.
Our code is also returning AjaxReponse and we wanted to merge them.

Merge request is provided.

If there is a better option please let me know.

Steps to reproduce

Proposed resolution

Review merge request.
Added a setCommands function:

  public function setCommands($commands) {
    $this->commands = array_merge($this->commands, $commands);
  }

Remaining tasks

Review

Release notes snippet

Added setCommands function to AjaxResponse class to allow merging of AjaxResponses.

Comment	File	Size	Author
#3	3343670-nr-bot.txt	1.28 KB	needs-review-queue-bot

Issue fork drupal-3343670

Show commands

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

Add & fetch this issue fork’s repository

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

Add & fetch this issue fork’s repository

11.x compare
Check out this branch
10.1.x compare
Check out this branch
3343670-allow-to-merge-ajaxcommands-11-x changes, plain diff MR !4902
Check out this branch
3343670-allow-to-merge changes, plain diff MR !3512
Check out this branch
3343670-allow-to-merge-ajaxcommands-10-1-x changes, plain diff MR !3544
Check out this branch

About issue forks

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

22 February 2023 at 10:55

Mschudders created an issue. See original summary.

Comment #2

22 February 2023 at 11:25

Mschudders opened merge request !3512

Comment #3

needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commented 22 February 2023 at 13:52

Status:

Needs review

» Needs work

File	Size
3343670-nr-bot.txt	1.28 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Comment #4

22 February 2023 at 19:18

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

Comment #5

enaznin CreditAttribution: enaznin at Axelerant for Drupal India Association commented 27 February 2023 at 04:39

Status:

Needs work

» Needs review

Comment #6

nod_

French

Lille

CreditAttribution: nod_ at Eclipse Foundation commented 27 February 2023 at 06:15

Version:	9.5.x-dev	» 10.1.x-dev
Status:	Needs review	» Needs work

There is an addCommand method that does much more than a simple array_merge so there is bound to be issues doing it this way.

MR should be against 10.1.x since that is the current dev version

Naming of the new function is not ideal. It's not about setting commands, I'd be more of a mergeCommands method
At the moment commands can be added the addCommand method, so what the issue raised could be worked around @Mschudders would that be enough?

Comment #7

Mschudders CreditAttribution: Mschudders at Calibrate commented 27 February 2023 at 07:39

@nod thanks for the information.

Indeed the naming was crappy.

Created a new branch against 10.1.x with the bot fixes + naming changes to the function.

Let me elaborate why this is usefull:

I have overridden MediaLibraryWidget so that I can attach an extra Ajax command to the callback of updateWidget:

  public static function updateWidget(array $form, FormStateInterface $form_state) {
    $response = parent::updateWidget($form, $form_state);

    $lb = new LayoutBuilderExtras();
    $responseLbExtraLiveUpdates = $lb->blockAjaxSave($form, $form_state);
    $commandsLiveUpdate = $responseLbExtraLiveUpdates->getCommands();
    $response->mergeCommands($commandsLiveUpdate);

    return $response;
  }

Since updateWidget returns an AjaxReponse already it isn't easy to add an additional command to it.
If you retrieve the commands from AjaxReponse it's already an rendered array and you cannot set them again. That's why I have added a mergeCommands function so that you can merge the commands.
I hope this clarifies things.

Comment #8

27 February 2023 at 07:41

Mschudders opened merge request !3544

Comment #9

27 February 2023 at 07:41

Version:

10.1.x-dev

» 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.