Allow to merge AjaxCommands inside of AjaxResponse

Created on 22 February 2023, over 1 year ago
Updated 28 February 2024, 4 months ago

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

NA

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.

✨ Feature request
Status

Needs work

Version

11.0 πŸ”₯

Component
AjaxΒ  β†’

Last updated 1 minute ago

Created by

πŸ‡§πŸ‡ͺBelgium Mschudders

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Issue created by @Mschudders
  • @mschudders opened merge request.
  • Status changed to Needs work over 1 year ago
  • 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.

  • πŸ‡§πŸ‡©Bangladesh eashika

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

  • Status changed to Needs review over 1 year ago
  • πŸ‡§πŸ‡©Bangladesh eashika
  • Status changed to Needs work over 1 year ago
  • πŸ‡«πŸ‡·France nod_ Lille

    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?

  • πŸ‡§πŸ‡ͺBelgium Mschudders

    @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.

  • @mschudders opened merge request.
  • First commit to issue fork.
  • last update about 1 year ago
    29,420 pass
  • Status changed to Needs review about 1 year ago
  • πŸ‡§πŸ‡ͺBelgium tim-diels Belgium πŸ‡§πŸ‡ͺ

    Rebased to be up-to-date.

  • Status changed to Needs work about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Can the MR be updated for 11.x as that's the current development branch.

    Also could use a simple test assertion to show it's working.

  • last update 9 months ago
    29,644 pass
  • last update 9 months ago
    30,341 pass
  • last update 9 months ago
    30,361 pass
  • πŸ‡§πŸ‡ͺBelgium tim-diels Belgium πŸ‡§πŸ‡ͺ

    Still needs test.

  • πŸ‡§πŸ‡ͺBelgium tim-diels Belgium πŸ‡§πŸ‡ͺ

    The module Layout Builder Extras - live update β†’ requires this patch.

  • First commit to issue fork.
Production build 0.69.0 2024