core/misc/ajax.js
✖ 84 problems (0 errors, 84 warnings)
CommentFileSizeAuthor
#13 interdiff.txt747 bytesnod_
#13 core-jsdoc-ajaxjs-2559693-13.patch16.36 KBnod_
#11 core-jsdoc-ajaxjs-2559693-11.patch16.81 KBnod_
#6 interdiff-2559693-4-6.txt7.86 KBleolandotan
#6 core-jsdoc-ajaxjs-2559693-6.patch17.33 KBleolandotan
#4 core-jsdoc-ajaxjs-2559693-4.patch9.6 KBnod_

Comments

nod_ created an issue. See original summary.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Issue summary: View changes
nod_’s picture

nod_
Primary language French
Location Lille
commented
Assigned: Unassigned » nod_
nod_’s picture

nod_
Primary language French
Location Lille
commented
Assigned: nod_ » Unassigned
Status: Active » Needs work
StatusFileSize
new core-jsdoc-ajaxjs-2559693-4.patch9.6 KB

I have one code change with this patch:

-  Drupal.ajax.WRAPPER_FORMAT = '_wrapper_format';
+  Drupal.Ajax.WRAPPER_FORMAT = '_wrapper_format';

It was supposed to be renamed, see #1305882-93: drupal_html_id() considered harmful; remove ajax_html_ids to use GET (not POST) AJAX requests, but I forgot.

Need to add documentation to all Drupal.AjaxCommands stuff. Drupal.Ajax has been taken care of.

leolandotan’s picture

leolandotan commented
Assigned: Unassigned » leolandotan
Issue tags: +drupalconasia2016
leolandotan’s picture

leolandotan commented
Status: Needs work » Needs review
StatusFileSize
new core-jsdoc-ajaxjs-2559693-6.patch17.33 KB
new interdiff-2559693-4-6.txt7.86 KB

Hi,

Here I have worked on the additional ESLint errors for the Drupal.AjaxCommands. I also got some idea from ajax.inc's documentations.

Additional thing to note

In the Drupal.AjaxCommand alert(), I removed the response.title @param because window.alert() only accepts one parameter and I'll not update that here because it's not in the JSDoc premise.

Hope everything is in order.

Thanks!

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community

This is great! no more eslint warning for doc in ajax.js. It's not perfect but as decided previously, we're getting the docs in and fixing them once they are in there :)

The last submitted patch, 4: core-jsdoc-ajaxjs-2559693-4.patch, failed testing.

catch’s picture

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

Doesn't apply.

Would be good to fix this in the re-roll, was going to do it locally but then the patch didn't apply.

+++ b/core/misc/ajax.js
@@ -959,10 +1050,15 @@
+     *   A JQuery selector string.
catch’s picture

catch
he/him
Primary language English
commented
Version: 8.0.x-dev » 8.1.x-dev

Doesn't apply to 8.1.x specifically.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs work » Needs review
StatusFileSize
new core-jsdoc-ajaxjs-2559693-11.patch16.81 KB

Made it apply, changed JQuery into jQuery and removed the (optional) in parameter description because jsdoc has syntax to declare something as optional and it is already used.

leolandotan’s picture

leolandotan commented
Assigned: leolandotan » Unassigned

I checked the latest patch against 8.1.x. It applies cleanly with the said branch.

I'm not sure if the patch for 8.1.x will be the one to be used in 8.0.x too because I'm still new on this scenario. So I just tried to apply it into 8.0.x but it doesn't apply cleanly. Just disregard this if it's not needed.

Will just need others to double check this if it's good to go.

Thanks!

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new core-jsdoc-ajaxjs-2559693-13.patch16.36 KB
new interdiff.txt747 bytes

Removed code change.

  • catch committed 47c4639 on 8.1.x 
    Issue #2559693 by nod_, leolando.tan: JSDoc ajax.js
catch’s picture

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

Committed/pushed to 8.1.x, thanks!

Status: Fixed » Closed (fixed)

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