Problem/Motivation

In ManagedFile, the ajax does this:

      'options' => [
        'query' => [
          'element_parents' => implode('/', $element['#array_parents']),
        ],
      ],

This allows query parameters to be passed in the AJAX request.

In this particular case, it allows the AJAX callback to receive the #array_parents. These are what allows the callback to know where it is situated in the form, and so how to get the form element being operated on. It's essential for using AJAX in form elements (as opposed to complete forms).

However, it's not documented in core.api.php's section on AJAX.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#16 ajax-interdiff-16.txt1.83 KBxjm
#16 ajax-3324726-16.patch1.33 KBxjm

Issue fork drupal-3324726

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:

About issue forks

Comments

joachim created an issue. See original summary.

Akhil Babu made their first commit to this issue’s fork.

Akhil Babu opened merge request !3079

Akhil Babu opened merge request !3080

akhil babu’s picture

akhil babu
he/him
commented
Status: Active » Needs review

For reviews and corrections.

Akhil Babu closed merge request !3079

java008’s picture

java008 commented

I reviewed this MR and it looks good. Moving to RTBC.

java008’s picture

java008 commented
Status: Needs review » Reviewed & tested by the community
feuerwagen’s picture

feuerwagen
he/him
Primary language German
Location Bonn 🇪🇺
commented
Status: Reviewed & tested by the community » Needs work

This still needs work. Seems like the formatting of the added docs doesn't follow the Drupal standards:

Drupal code quality checks failed.
To reproduce this output locally:
* Apply the change as a patch
* Run this command locally: sh ./core/scripts/dev/commit-code-check.sh
OR:
* From the merge request branch
* Run this command locally: sh ./core/scripts/dev/commit-code-check.sh --branch 10.1.x

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

gauravvvv’s picture

gauravvvv commented
Status: Needs work » Needs review
akhil babu’s picture

akhil babu
he/him
commented

Thanks @Gauravvv.
Composer error seems like a common issue: https://drupal.slack.com/archives/C51GNJG91/p1673357627190639

sharayurajput’s picture

sharayurajput commented
Assigned: Unassigned » sharayurajput

I will review it

sharayurajput’s picture

sharayurajput commented
Assigned: sharayurajput » Unassigned
Status: Needs review » Reviewed & tested by the community

Reviewed the MR its looking good We can move this to RTBC

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

The tests are failing, so the issue can't be RTBC. The test failures are due to an existing issue with our testing infrastructure, but once that is resolved, we need a clean test run here to proceed.

xjm’s picture

xjm
she/her
Primary language English
commented
StatusFileSize
new ajax-3324726-16.patch1.33 KB
new ajax-interdiff-16.txt1.83 KB

I verified that the coding standards checks will pass by applying the patch locally and running:

[ayrton:drupal | Thu 11:46:41] $  sh ./core/scripts/dev/commit-code-check.sh --cached
warning ../../../package.json: No license field
1/1 ./core.api.php 852.20ms
CSpell: Files checked: 1, Issues found: 0 in 0 files

CSpell: passed

----------------------------------------------------------------------------------------------------

Running PHPStan on changed files.

                                                                                
 [OK] No errors                                                                 
                                                                                


PHPStan: passed

----------------------------------------------------------------------------------------------------
Checking core/core.api.php

PHPCS: core/core.api.php passed
core/core.api.php passed

----------------------------------------------------------------------------------------------------

However, when I applied the patch locally, I found a number of code style errors, like formatting that did not match other sections of the doc and lines not wrapping at 80 characters. Since MRs are having testing issues today, I've suppled the above MR as a patch, with an interdiff showing my changes.

What I haven't done is do the research on the API to confirm the accuracy of the documentation. That's a task a credit-worthy code review could do.

Thanks!

xjm’s picture

xjm
she/her
Primary language English
commented 
+++ b/core/core.api.php
@@ -2444,19 +2444,21 @@ function hook_validation_constraint_alter(array &$definitions) {
+ * 'ajax_form=1' parameter (See \Drupal\Core\Render\Element\RenderElement).
...
+ * This helps to identify the position of the element in the form (See
+ * \Drupal\file\Element\ManagedFile).

Also maybe the parenthetical See... would be better as @see, or at least added as @see at the end of the section?

xjm’s picture

xjm
she/her
Primary language English
commented

I also pushed my change from #16 to the MR, in the hopes that we can go back to using the MR once this Composer issue on DrupalCI is resolved.

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

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Removing credit from myself. But I rebased the MR to jerk it loose so it will run.

  • catch committed 258cef9b on 10.1.x 
    Issue #3324726 by Akhil Babu, xjm, Gauravvv, joachim: #ajax 'options'...

  • catch committed 04f2121f on 10.0.x 
    Issue #3324726 by Akhil Babu, xjm, Gauravvv, joachim: #ajax 'options'...

  • catch committed 26a15a09 on 9.5.x 
    Issue #3324726 by Akhil Babu, xjm, Gauravvv, joachim: #ajax 'options'...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 258cef9 and pushed to 10.1.x. Thanks! Also cherry-picked to 10.0.x and 9.5.x

Status: Fixed » Closed (fixed)

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