Integrate CTools AJAX framework with Drupal to extend (and replace) existing ahah framework

Awesome! Almost ready to commit for me.

1) "Drupal.AJAX" - why all uppercase? Not really in line with our camelCase naming for JS. (And I personally find that Drupal.ajax['foo'] looks much nicer than Drupal.AJAX['foo'])

2) I'm by all means no OOP expert, but those ajax_command_*() functions as well as the example invocation snippet made me question whether we shouldn't use a class/object on the PHP side as well?

$commands = array();
$commands[] = ajax_command_replace('#object-1', 'some html here');
$commands[] = ajax_command_changed('#object-1');
ajax_render($commands); // this function exits.

...to something like this:

$ajax = new DrupalAJAX;
$ajax->replace('#object-1', 'some html here');
$ajax->replace('#object-2', 'some other html here');
$ajax->setChanged('#object-1');
$ajax->render(); // this function exits.

That way, we'd have pretty similar AJAX objects on both layers - PHP and JS. No?

For the record, fields 'add more' JS callback (field_add_more_js()) is a god-awful piece of code, directly inherited from CCK D6.
#367567: Use AJAX framework for "Add more" / load include file containing form / introduce $form_state['build_info'] was open to update it to D7's ahah improvements and hopefully streamline it in the process - but it was agreed to postpone it while #399982: Simplify and standardize returning an AHAH response settles.
Since this issue now takes over, it's probably a good time to revive this, but I'm not sure where to start, and I not-so-secretly hope quicksketch will chime in :-p.

Also, note that much of the ugliness in field_add_more_js() was added in D6 to account for filefield widgets specific needs - or at least the needs reported by the folks with their hands in filefield back in spring 08 (filefield governance was a little hectic back then)
For this reason too, i hope quicksketch will be of good help on this.

And major yay for the patch, BTW !

from patch #2 :

function ajax_form_callback() {
  list($form, $form_state) = ajax_get_form();
  // Build, validate and if possible, submit the form.
  drupal_process_form($form_id, $form, $form_state);

It looks like the $form_id is never set ?

+  $commands[] = ajax_command_replace('#' . $field_name_url_css . '-wrapper', $output);

Having to specify the HTML target to be replaced looks odd - isn't it already specified in the $element['#ajax']['wrapper'] property ?

[edited] I fixed #8 by adding $form_id = $form['#form_id']; after the
ajax_get_form() call in ajax_form_callback().

Then, it seems the *second* time you click the 'add more poll choices'
button fails.
$form = form_get_cache($form_build_id, $form_state); returns NULL in
ajax_get_form().

Actually, $form_build_id in ajax_form_callback() has the same problem than $form_id: not set.
Fixed that locally by changing ajax_get_form() to return ($form, $form_state, $form_id, $form_build_id).

(sorry for the flood, I'm playing with the patch to refactor field's own 'add more' code - works beautifully, but it will require some confirmation from quicksketch that it doesn't break filefield, so it's probably best to keep it for a followup. The current patch keeps the existing ugly code working anyway...)

oops, crosspost : edited my comment #10 with the fix.

I'm not so sure about having it return 4 items in a list

Yeah. Did the trick for me locally, but obviously YMMV whether it's the best fix.

IRC: sorry, bedtime :-). I should've logged on IRC earlier, actually, would have saved 5-6 comments...

This looks truly awesome. In order to take it for a serious spin, I'd want to convert Quicktabs over to using it, but unfortunately I think this patch cannot wait that long :-(. In the meantime, I did just test out the poll creation form and there's still a problem when you add a fourth choice - it adds the the elements fine but the tabledrag handles don't get added. Will look at it some more tomorrow if I get a chance.

subscribe

Minor: the PHPdoc for ajax_get_form() needs to be updated for the new return values.

...really AJAJ since we return JSON, not XML but AJAX sounds cooler

Will "being cool" justifies a DX confusion of expected return format? Or is AJAJ such a weird term no-one gets? Wikipedia seems to redirect from AJAJ to AJAX: http://en.wikipedia.org/w/index.php?title=AJAJ&redirect=no

Subscribing..... Awesome to the max.

subscribing. this is awesome. looking forward to it being committed.

Fixed the test failures.

+++ includes/ajax.inc	10 Aug 2009 20:34:49 -0000
@@ -0,0 +1,603 @@
+/**
+ * Set the HTML of a given selector to the given data.
+ *
+ * @param $selector
+ *   The CSS selector. This can be any selector jquery uses in $().
+ * @param $html
+ *   The data to use with the jquery html() function.
+ * @param $settings
+ *   An optional array of settings that will be used for this command only.
+ *
+ * @return
+ *   An array suitable for use with the ajax_render() function.
+ */
+function ajax_command_html($selector, $html) {

3 parameters in phpdoc, only 2 in the function.
No new line before @return (as with all the others in the patch).

+++ includes/ajax.inc	10 Aug 2009 20:34:49 -0000
@@ -0,0 +1,603 @@
+/**
+ * Create a changed command for the AJAX responder.
+ *
+ * This will mark an item as 'changed'.

This description is for the previous function. This one is about css.

+++ includes/ajax.inc	10 Aug 2009 20:34:49 -0000
@@ -0,0 +1,603 @@
+/**
+ * Render a commands array into JSON and immediately hand this back
+ * to the AJAX requester. This function will render and immediately
+ * exit.
+ *
+ * @param $commands
+ *   A list of macro commands generated by the use of ajax_command_* functions.
+ * @param $header
+ *   If set to FALSE the text/javascript header used by drupal_json will not
+ *   be used, which is sometimes necessary when using iframe.
+ *   If set to 'multipart' the output will be wrapped in a textarea, which
+ *   can also be used when uploading files.
+ */
+function ajax_render($commands = array(), $header = TRUE) {

Missing the one line summary.

+++ includes/ajax.inc	10 Aug 2009 20:34:49 -0000
@@ -0,0 +1,603 @@
+function ajax_render_error($error = '') {

Do we need a @param here or is just self explanatory?

+++ includes/ajax.inc	10 Aug 2009 20:34:49 -0000
@@ -0,0 +1,603 @@
+/**
+ * Get a form submitted via #ahah during an ajax callback.
+ *

#ahah should be #ajax

+++ includes/entity.inc	10 Aug 2009 20:34:53 -0000
@@ -0,0 +1,281 @@
+<?php
+/**
+ * Interface for entity loader classes.

Pour interface. Are you lost? Where are your parents?

This review is powered by Dreditor.

entity.inc probably comes from catch's reroll ;-)
What is surprising though is that there's only 1kb difference between #26 and #17, while there are somthg like 200 additional lines of unrelated entity.inc ?

Not sure about the PHPDoc syntax for linking to another defined group.

@catch: entity.inc is missing a CVS $Id$ at the top of the file ;)

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+/**
+ * @defgroup ajax AJAX framework
+ * @{
...
+ * @see ajax_commands

Is that the proper syntax for linking to another @defgroup ?

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+ *   If set to FALSE the 'text/javascript' header used by drupal_json() will not
+ *   be used, which is sometimes necessary when using an IFRAME. If set to

"sometimes"? When?

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+  // Automatically extract any 'settings' added via drupal_add_js and make them

Missing parenthesis after drupal_add_js().

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+  if ($header === 'multipart') {

Why type-agnostic matching?

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+    // We don't use drupal_json here because the header is not true. We're not

Missing parenthesis after drupal_json(). We avoid abbreviations comments ("don't").

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+/**
+ * Send an error response back via AJAX and immediately exit.
+ *
+ * This function can be used to quickly create a command array with an error
+ * string and send it, short-circuiting the error handling process.
+ *
+ * @param $error
+ *   A string to display in an alert.
+ */
+function ajax_render_error($error = '') {
+  $commands = array();
+  $commands[] = ajax_command_error($error);
+  ajax_render($commands);
+}

This does not allow for multiple error messages + processing... considering that there may be X modules involved in returning an AJAX response for Y, that might not be a good idea?

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+    // If the returned value is a string, assume it is HTML and create
+    // a command object to return automatically.
+    if (is_string($html)) {
+      $commands = array();
+      $commands[] = ajax_command_replace($ajax['wrapper'], $html);
+    }
+    else {
+      $commands = $html;
+    }
+
+    ajax_render($commands);

We want to squeeze a drupal_alter() in here, right before ajax_render() is invoked.

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+  if ((isset($element['#ajax']['callback']) || isset($element['#ajax']['path'])) && isset($element['#ajax']['event']) && !isset($js_added[$element['#id']])) {

Shouldn't we check for !isset($js_added[$element['#id']]) first?

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+    'text' => $error ? $error : t('Server reports invalid input error.'),

Use !empty() here.

Also, the default error message sounds a bit awkward... (think of regular users)

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+ * The replace command will replace a portion of the current document
+ * with the specified HTML.
...
+ * @param $argument
+ *   An array of key: value pairs to add to the settings. This will be
+ *   utilized for all commands after this if they do not include their
+ *   own settings array.
+ * @return
+ *   An array suitable for use with the ajax_render() function.
+ */
+function ajax_command_settings($argument) {
...
+ * This will attach the name=value pair of data to the selector via
+ * jQuery's data cache.

Does not wrap at 80 chars.

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+ *   The CSS selector. This can be any selector jQuery uses in $().

(and elsewhere)

s/selector jQuery uses in $()/jQuery selector/.

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+ * Creates an after command for the AJAX responder.
...
+ * Creates a before command for the AJAX responder.

I wonder whether we shouldn't stuff "jQuery" before the command name here to make any sense of those summaries...

+++ includes/ajax.inc	11 Aug 2009 03:10:00 -0000
@@ -0,0 +1,552 @@
+ * @param $star
+ *   An optional CSS selector which must be inside $selector. If specified,
+ *   a star will be appended.

Appended to what? The selector provided as $star?

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+ * then executed to make whatever hanges are necessary to the page.

s/whatever/any/

Typo in "(c)hanges".

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+ * Drupal uses this file to enhance form elements with #ajax[path] and
+ * #ajax[wrapper] properties. If set, this file will automatically be included

Array elements should be wrapped in ''.

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+Drupal.AJAX = Drupal.AJAX || {};

I still think that we should use Drupal.ajax - also, to match jQuery.ajax.

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+    // Also bind based on classes.
+    $('.use-ajax:not(.ajax-processed)').each(function() {
...
+    // This class means to submit the form to the action using ajax.
+    $('.use-ajax-submit:not(.ajax-processed)').each(function() {

1) We need a place to document those generic CSS classes. (Very useful!)

2) It's not entirely clear to me what the actual difference between those 2 classes is.

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+      var element_settings = { };
...
+      var element_settings = { };
...
+    button: { },

No space here (empty object).

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+      var base = $(this).attr('id');
+      Drupal.AJAX[base] = new Drupal.AJAX(base, this, element_settings);
...
+      var base = $(this).attr('id');
+      Drupal.AJAX[base] = new Drupal.AJAX(base, this, element_settings);

What happens if #id is not defined?

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+    }).addClass('ajax-processed');
...
+    }).addClass('ajax-processed');

We should add the .processed CSS class first. (Also, to prevent any possible recursion.)

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+ * All AJAX objects on a page are accessible through the global Drupal.AJAX object
+ * and are keyed by the submit button's ID. You can access them from your module's

Exceeds 80 chars.

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+Drupal.AJAX = function (base, element, element_settings) {
+  var defaults = {
+    url: "system/ajax",
+    event: "mousedown",
+    keypress: true,
+    selector: "#" + base,
+    effect: "none",
+    speed: "slow",
+    method: "replace",
+    progress: {
+      type: "bar",
+      message: "Please wait..."

Use single quotes.

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+  jQuery.extend(this, defaults, element_settings);

(possibly elsewhere) Use $ instead of jQuery.

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+  // Replacing nojs with ajax allows for an easy method to detect when we
+  // need to degrade.
+  this.url = element_settings.url.replace('/nojs/', '/ajax/');

Wrap 'nojs' and 'ajax' in single quotes and append "in the URL" to clarify what we're talking about.

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+        // Put our button in.
+        AJAX.form.clk = this.element;

"clk" ?

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+Drupal.AJAX.prototype.getEffect = function(response) {

Missing space before function arguments, i.e. "function (...)".

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+  if ($('.AJAX-new-content', new_content).size() > 0) {
+    $('.AJAX-new-content', new_content).hide();

Especially this class supports the renaming to lowercase ;)

+++ misc/ajax.js	11 Aug 2009 03:15:15 -0000
@@ -0,0 +1,390 @@
+  // Attach all javascript behaviors to the new content, if it was successfully

Forgot the global renaming of "javascript" to "JavaScript" in this file.

Beer-o-mania starts in 20 days! Don't drink and patch.

Field test failures: the first fail is

    // The response is drupal_json, so we need to undo some escaping.
    $response = json_decode(str_replace(array('\x3c', '\x3e', '\x26'), array("<", ">", "&"), $this->drupalGetContent()));
    $this->assertTrue(is_object($response), t('The response is an object'));

in _fieldPostAhah(). I guess that this part needs to be updated too ?
Also, this function was stolen from poll tests, so I guess some poll failures might have the same explanation.

Wow, great patch. This is really flexible and absolutely coreworthy. Makes it really really simple to do the little ajax/ahah link that does a couple of small things on a page. Amazing. Documentation is also really nice.

Couple of nits

+++ includes/ajax.inc	12 Aug 2009 20:56:32 -0000
@@ -0,0 +1,594 @@
+ *   used as an altnerative method when uploading files.

typo: alternerative

+++ includes/ajax.inc	12 Aug 2009 20:56:32 -0000
@@ -0,0 +1,594 @@
+  // them the first command:

typo: colon should be a dot.

+++ misc/ajax.js	12 Aug 2009 20:56:39 -0000
@@ -0,0 +1,396 @@
+/**
+ * Provides AJAX-like page updating via AJAX (Asynchronous JavaScript and XML).

This sounds weird. Let's remove teh "AJAX-like".

+++ misc/ajax.js	12 Aug 2009 20:56:39 -0000
@@ -0,0 +1,396 @@
+  /**
+   * Command to insert new content into the DOM
+   */
+  insert: function (ajax, response, status) {
+    ajax.insertNewContent(response, status);
+  },

Let's just move the code from ajax.insertNewContent over here. ajax.insertNewContent isn't used anywhere else, and is the actual command, so simpler codeflow++

This review is powered by Dreditor.

IMO this is RTBC, after these are fixed. Great work.

WRT to the insertNewContent code: it will still be overrideable even if it's in Drupal.ajax.commands.prototype.insert, so no loss of functionality at all by moving it to the more logical place.

Instead of overriding Drupal.ajax[my_base].insertNewContent you'd override Drupal.ajax[my_base].commands.insert in your behaviour. Both ajax.prototype.insertNewContent and ajax.prototype.commands.insert are eqally accessible/replaceable for a specific form element inside a behavior since the ajax objects were made public in #396466: Support custom success callbacks in ahah.js, I think.

I started a nitpick pass, but frando beat me to it. Didn't spot anything beyond those (or even all the ones frando found).

Two more nitpicks:

+  $field_name_url_css = $field_name;

Seems like this variable is set but never used?

+    // The JSON response will be two AJAX commands. The first is a settings and the second
+    // is the replace.

s/settings/setting

Also, not a nitpick, but what a lovely hunk:

-  // If a newly inserted widget contains AHAH behaviors, they normally won't
-  // work because AHAH doesn't know about those - it just attaches to the exact
-  // form elements that were initially specified in the Drupal.settings object.
-  // The new ones didn't exist then, so we need to update Drupal.settings
-  // by ourselves in order to let AHAH know about those new form elements.
-  $javascript = drupal_add_js(NULL, NULL);
-  $output_js = isset($javascript['setting']) ? '<script type="text/javascript">jQuery.extend(Drupal.settings, ' . drupal_to_js(call_user_func_array('array_merge_recursive', $javascript['setting'])) . ');</script>' : '';
-
-  $output = theme('status_messages') . drupal_render($field_form) . $output_js;
-  drupal_json(array('status' => TRUE, 'data' => $output));
-  exit;
+  $output = theme('status_messages') . drupal_render($field_form);
+
+  $commands = array();
+  $commands[] = ajax_command_replace(NULL, $output);
+  ajax_render($commands);

I haven't played with the AHAH stuff really yet, so not qualified to review on functionality, but in general this makes things a lot more readable from the perspective of someone not familiar with the existing code, and it seems like the people who are familiar with it are also very happy, so should be otherwise rtbc I think.

wow... looks really good... but i manged to find some nits to pick :D

Boolean should be capitalized since it's named after George Boole:

+  // Use === here so that boolean TRUE doesn't match 'multipart'.

I think it would be helpful to add parens after drupal_json so it's clear we're referring to the function:

+    // We don't use drupal_json here because the header is not true. We're not

Please capitalize URL:

+    // Change progress path to a full url.

The description of ajax_command_error() doesn't match the format used by all the other ajax_command_*() functions.

The description of ajax_command_html() doesn't match the format used by all the other ajax_command_*() functions. It should say something about creating a jQuery html command for the AJAX responder. The current description would be great for a second line.

I'm not sure what to make of the "key:" bit here...

+ *   The name or key: of the data attached to this selector.

Actually there's a few of those... it doesn't seem like the rest of core's docs.

The description for ajax_command_css() is wrong. I'd also like another sentence explaining what it does.

I'd love to see a couple more comments in Drupal.behaviors.AJAX.attach. I'd also suggest reviewing the comments that are in there. They could use some work.

The wrapping on this seems off or that there should be two paragraphs:

+ * All AJAX objects on a page are accessible through the global Drupal.ajax
+ * object and are keyed by the submit button's ID. You can access them from
+ * your module's JavaScript file to override properties or functions.
+ * For example, if your AJAX enabled button has the ID 'edit-submit', you can
+ * redefine the function that is called to insert the new content like this
+ * (inside a Drupal.behaviors attach block):

There's an extra space after URL:

+  // Replacing 'nojs' with 'ajax' in the URL  allows for an easy method to

Couple of small things here, the whole "in. form.clk" is odd. We should probably quote the form.clk part. And there's a teh in there:

+        // Put our button in. form.clk is a special variable for ajaxSubmit
+        // that tells the system which element got clicked to trigger teh
+        // submit. Without it there would be no 'op' or equivalent.

I'd love a line of white space after the block of code and before the comment here:

+  });
+  // If necessary, enable keyboard submission so that AJAX behaviors
+  // can be triggered through keyboard input as well as e.g. a mousedown
+  // action.
+  if (element_settings.keypress) {

It would be nice it we explained why we wanted to detect the enter key and special case it:

+      // Detect enter key.
+      if (event.keyCode == 13) {
+        $(element_settings.element).trigger(element_settings.event);
+        return false;
+      }

Small one but I think we capitalize HTML in comments:

+/**
+ * Build an effect object which tells us how to apply the effect when
+ * adding new html.
+ */

misc/ajax.js the insert command references a jQuery issue (http://dev.jquery.com/ticket/1152) that's been marked as a duplicate so we should probably reference the updated ticket: http://dev.jquery.com/ticket/3178

Grammar bug:

+    // Determine what effect use and what content will receive the effect, then
+    // show the new content.

Should probably be "Determine what effect to use..."

This was mentioned once up thread but in poll.test:

+
+    // The JSON response will be two AJAX commands. The first is a settings and the second
+    // is the replace.

You should probably copy the comment from field.test where it's already been fixed.

+  // Passing FALSE forces ajax_render() to not to send a 'text/javascript'
+  // Content-Type header, which does not work with all browsers when using an
+  // IFRAME (necessary for uploads).

I always like it when these type of comments explain why you're doing something then how you're doing it. How about:

AJAX uploads use an IFRAME and some browsers have problems with the 'text/javascript' Content-Type header. Passing FALSE to ajax_render() prevents the header from being sent.

Actually maybe we should make that "<iframe>"? to match the "<textarea>" mentioned in ajax_render()?

On the whole this patch keeps getting better the more I look at it. I've fixed a few capitalization, punctuation and line wrapping issue issues.

I'm not sure what is meant by the header here:

+    // We don't use drupal_json() here because the header is not true. We're
+    // not really returning JSON, strictly-speaking, but rather JSON content
+    // wrapped in a <textarea> as per the "file uploads" example here:

Does that refer to the Content-type header?

Is started synching up the descriptions of ajax_command_*() functions. I kind of punted on the ajax_command_error() one. It'd be nice if someone (read: Earl) could take a look at it.

It would be nice if these commands referenced the JS object where they're implemented, e.g. ajax_command_replace() Drupal.ajax.prototype.commands.insert... and in the process of looking that up I realized I was confused because in PHP space the command is "error" but in JS space it's "alert". I'm not sure if it makes sense to get the names to match up but if we don't then we should definitely reference the JS code that implements the commands.

I started doing some actual testing on this using the upload.module and got the following warnings:

# Notice: Undefined index: process_input in _form_builder_handle_input_element() (line 1092 of /Users/amorton/Sites/dh/includes/form.inc).
# Notice: Undefined index: complete form in form_builder() (line 974 of /Users/amorton/Sites/dh/includes/form.inc).
# Notice: Undefined index: complete form in form_builder() (line 974 of /Users/amorton/Sites/dh/includes/form.inc).

But after reverting the patch found the same warnings so I don't think they're related.

I thought poll module was a little wonky in that if you re-order items and then click "preview" or "more choices" the order gets reset. But reverted the patch and it's wonky both ways.

talked with earl on irc about fixing the docs for the ajax_command_*() functions. i think i've got a pretty good start on explaining what's returned, where those commands are implemented in JS and if/when they map to jQuery code.

the bot doesn't seem to like my patches but lets see if this one works better.

+      // Detect enter key.
+      if (event.keyCode == 13) {
+        $(element_settings.element).trigger(element_settings.event);
+        return false;
+      }

Should also detect event.keyCode == 10, iPhone Safari send 10 for ENTER

mattyoung, i'm not sure i really want to make that change as part of this issue. it's a "pre-existing condition" probably merits it's own discussion. care to open up a new issue for it?

merlin, what do you think about just renaming ajax_command_error() to ajax_command_alert()? the only place it's called is ajax_render_error(). i think it's better to keep the php function->protocol command->js function naming as simple as possible. it seems like ajax_render_error() would be the best place to address error handling anyway, rather than in a command building function.

using replaceWith() seems like a really good solution. i'll start on that once i'm done cooking this batch of beer.

the DJON name sounds fine to me but i'll hold off on changing the docs until there's more feedback.

#55

>care to open up a new issue for it?

#550360: AHAH framework needs to check for keyCode == 10 in addition to keyCode == 13

made the changes in #57.

I've changed replace() to use replaceWith() but removing the unset($choice_form['#prefix'], $choice_form['#suffix']); bits was resulting in extra DIVs. In the process I started trying to see where ajax_command_replace() was called and noticed that the selector was being omitted. After puzzling over that for a bit I discovered that if you omit the selector then it provides the element as the default (right?).

Also does alert really take two parameters? I couldn't seem to find any documentation supporting that.

okay dropped the title from alert and fixed the selector docs. i think i've broken something though... the ajax bits aren't going...

ok, first of all, this is a re-roll of drewish's last patch, because that was partially reversed.

Not quite. It seems, some hunks got lost during the past re-rolls.

Final review + test.

Ready to fly.

A few changes:

- I've introduced a drupal_alter() JUST to have it there, because that would turn out to be an API change later on, unlikely to be added later:

+function ajax_render($commands = array(), $header = TRUE) {
...
+  // Allow modules to alter any AJAX response.
+  drupal_alter('ajax_render', $commands);

- I replaced all references to JavaScript functions with proper @see directives:

- * This command is implemented by Drupal.ajax.prototype.commands.alert()
- * defined in misc/ajax.js.
...
+ * @see Drupal.ajax.prototype.commands.alert()

Just because our API parser doesn't grok JavaScript, this doesn't mean that we can't use the proper syntax.

- I re-introduced blank PHPDoc lines between @param, @return, and @see directives for better readability:

+ * @param $settings
+ *   An optional array of settings that will be used for this command only.
+ *
+ * @return
+ *   An array suitable for use with the ajax_render() function.
+ *
+ * @see http://docs.jquery.com/Attributes/html#val
+ */

- Changed all instances of $().size() into $().length.

- The 'changed' AJAX command now uses a CSS class name with a proper namespace ("ajax"). Additionally, $star has been changed into $asterisk:

+  /**
+   * Command to mark a chunk changed.
+   */
+  changed: function (ajax, response, status) {
+    if (!$(response.selector).hasClass('ajax-changed')) {
+      $(response.selector).addClass('ajax-changed');
+      if (response.asterisk) {
+        $(response.selector).find(response.asterisk).append(' <span class="ajax-changed">*</span> ');
+      }
+    }
+  },

I want to amend that uploading an attachment throws a boat of PHP notices at you. However, that's not caused by this patch (they also appear without it) - it seems like drupal_process_form() is horribly broken currently.

sun,

i'm a bit surprised that you RTBCed your own patch... usually you're a stickler for protocol.

i specifically didn't use the PHPDoc @see tag because it'll give you a useless link that doesn't help you figure out where the method lives. i wanted the reference to be usable. until we have a working parser lets make it easy on the reader rather than being "correct".

i also encountered those those php notices and mentioned them back in #50.

Revamped introduction in ajax.inc.

Watch the patch name! XD

Reverted the change to @see for JS function pointers to keep all of us happy :)

Starting with a docs-only review:

1. I'm really encouraged to see the level of review that this patch has received. Makes me feel pretty comfortable committing it once i've had a good pass at it. Thanks for all of your hard work.

2. I am NOT a JS developer, so I am completely befuddled about some of this stuff and have tried to make comments accordingly. You can argue back that these docs are not for people like me, and that is fine. However, since #ahah ties into the form API, i think it would be lovely if your average PHP developer could use it, and I think that's its intention. That said, if I say something *very* silly, feel free to ignore it and chalk it up to 2am. ;)

+++ includes/ajax.inc	17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * Drupal's AJAX framework creates a PHP macro language that can be used to
+ * instruct JavaScript to perform actions on the client browser. When using
+ * forms, this framework can be used automatically with the #ajax property.

I'm sure that's very technically accurate, however it went zooming way over my head. I don't think that it's Drupal's job to define AJAX, but something that would really help here is an architectural overview of how this system works.

For example (update for actual correctness):

"Drupal's AJAX framework is used to dynamically update parts of a page's HTML based on data from the server. Upon a specified event, such as a button click, a callback function is triggered which performs server-side logic and then returns updated markup, which is then replaced on-the-fly with no page refresh necessary."

Finally, what would really gel it for me is instead of talking about a PHP macro language, show me some code snippets like there are in this issue of a simple form, a simple callback, etc.

+++ includes/ajax.inc	17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * To implement AJAX handling in a normal form, just add '#ajax' to the FAPI

There are far too many "F" APIs in Drupal now to get away with "FAPI". ;)

+++ includes/ajax.inc	17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * - #ajax['path']: The path to use for the request. By default this is
+ *   system/ajax. Be warned that the default path currently only works
+ *   for buttons. It will not work for selects, textfields, or textareas.

Probably specify "menu path" here, as opposed to file path some other kind of path. Also indicate what needs to happen at this path. Maybe something like, "The menu path to use for this request. This path should map to a menu page callback that returns JSON."

Also, I don't think this is your intent, but reading this makes me now terrified to use the default path and thinking it's very limited and will blow up in my face. Can we re-phrase that so it sounds more positive? :P Like "The default path is useful for .... but in the event you need something more fancy, you may also define your own custom path."

Also, on this and others, can we get an indication of which fields are required? See http://api.drupal.org/api/function/hook_theme for an example.

+++ includes/ajax.inc	17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * - #ajax['callback']: If using the default path (system/ajax), this
+ *   callback will be triggered to handle the server side of the #ajax
+ *   event. The callback will receive $form and $form_state as arguments,
+ *   and should return the HTML to replace.

What if I'm not using the default path? Maybe swap these around, so that we actually define the index first, then talk about the default path after?

+++ includes/ajax.inc	17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * - #ajax['wrapper']: The ID of the AJAX area. The HTML returned from the
+ *   callback will replace whatever is currently in this wrapper. It is
+ *   important to ensure that this wrapper exists in the form, and is often
+ *   created using #prefix and #suffix tags.

This assumes I know AJAX areas should have IDs. Let's make sure to mention that in the paragraph at the top that gives an overview of how this works.

I'm confused about "is often created using #prefix and #suffix tags". Is there a way to write this more clearly, and/or could we use a code example here?

+++ includes/ajax.inc	17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * - #ajax['event']: The JavaScript event to respond to. This is normally
+ *   selected automatically for the type of form widget being used, and
+ *   is only needed if you need to override the default behavior.

How do I know the default behaviour? Can you give me some hints?

+++ includes/ajax.inc	17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * - #ajax['method']: The jQuery method to use to place the new HTML.
+ *   Defaults to 'replace'. May be: 'replace', 'append', 'prepend',
+ *   'before', 'after', or 'html'. See the jQuery documentation for more
+ *   information on these methods.

Feel free to say no, since you're pointing me off to the jQuery docs anyway, but is there any way you could put in parentheses what 'html' does? The rest are pretty clear.

+++ includes/ajax.inc	17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * In addition to using FAPI for doing in-form modification, AJAX may be
+ * enabled by adding classes to buttons and links. By adding the 'use-ajax'
+ * class to a link, the link will be loaded via an AJAX call. This is
+ * particularly useful for things such as popups. When using this method,
+ * the href of the link can contain '/nojs/' as part of the path. When
+ * the AJAX framework makes the request, it will convert this to '/ajax/'.
+ * The server can then easily tell if this request was made through an
+ * actual AJAX request or in a degraded state, and respond appropriately.

1. FAPI again.

2. Also, can we move "This is particularly useful for things such as pop-ups" before the implementation details? ("By adding the 'use-ajax' class...") I was just going to type "You should tell me why I'd use this" when you did, a couple sentences later. ;)

Also, this is back into fuzzyville. I can haz code example?

+++ includes/ajax.inc	17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * When responding to AJAX requests, the server should do what it needs to do
+ * for that request, then create a commands array. This commands array will
+ * be converted to a JSON object and returned to the client, which will then
+ * iterate over the array and process it like a macro language.
+ *

What type of commands can go in a commands array? Are these PHP functions or JS functions or something else entirely?

+++ includes/ajax.inc	17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * Each command is an object. $object->command is the type of command and will
+ * be used to find the method (it will correllate directly to a method in
+ * the Drupal.ajax[command] space). The object may contain any other data that
+ * the command needs to process.

Oh, I see. It's an object. please tell me that a little sooner. I don't know what the Drupal.ajax[command] space means. But I guess you mean a JS function that's in scope. How do I get stuff in scope?

Also, "correlate"

+++ includes/ajax.inc	17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * @code
+ *   $commands = array();
+ *   $commands[] = ajax_command_replace('#object-1', 'some html here');
+ *   $commands[] = ajax_command_changed('#object-1');
+ *   ajax_render($commands); // Ends the request.
+ * @endcode

I feel like this code is meant to be self-explanatory, but it's not for me...

I *think* what this might be saying is:

$commands = array();
// Replace the contents of the 'object-1' ID's element on the page with 'some html here'
$commands[] = ajax_command_replace('#object-1', 'some html here');
// Notify the #object-1 element that it has changed since the last request.
$commands[] = ajax_command_changed('#object-1');
// Output new markup to the browser and end the request.
ajax_render($commands);

...but I'm not positive.

14 days to code freeze. Better review yourself.

I've been promising a review on this for a while now so here we go.

First of all, this patch is FLIPPING AWESOME. I'm really happy that merlinofchaos (and team) has made this great effort. The "Core" approach with #ahah and the "Views/cTools" approach with AJAX commands have from the beginning been taking completely dividing efforts. This patch unifies the approaches while maintaining all the functionality from both of them. Despite the length of the patch, this patch introduces a very small API change for people familiar with the current system, since it essentially just renames "#ahah" to "#ajax", which is probably a good change for recognizably.

My only structural complaint is that I'm not a big fan of the "automatic" replacement of "nojs" with "ajax" in URLs. Though at the same time I know that this is what Views and Panels use, and I know it can work for any application, I just usually prefer to use an indicator like "?js=1" in the query string. However... this has absolutely NO effect on the use of the #ajax (formerly #ahah) property, and it doesn't prevent using that other mechanism if desired later. So even though it's not the approach I'd use, there's nothing wrong with it and it's obviously proven to work through the D6 release cycle in Views.

Okay first pass just on reading the code:

- The one-line description for ajax_process_form() is too long:

* Add AJAX information about a form element to the page to communicate with JavaScript.

- Even though I know that ajax_process_form() is essentially a clone of form_process_ahah(), we should switch to using #attached_js and #attached_library in ajax_process_form().
- #ajax['path'] documentation is incorrect, since it can be used on any element. #ajax['callback'] is the one that will only work on buttons.
- The use of @see is a bit strange in places. Usually @see is left until the end of docs, but with all the ajax_command_* functions, it's in two separate places, both before the parameters and after them.

Just to test out *crazy* edge case scenarios I applied the CCK UI patch, installed the CVS HEAD of File module, AND this patch to test compatibility. I was able to "port" File module to the new system in 2 minutes, and every thing work absolutely perfectly. Other than small nitpicks, this patch is completely architecturally sound and a complete breeze to leverage for someone familiar with the current system.

Also, near the top it might be good to mention that there are three ways to attach AJAX to Drupal's output: through form elements, through link clicks, and through button clicks. That would help with my surprise that the docs just go on and on and the bottom part has nothing to do at all with what the top part was talking about. :)

Oopsie.

All nitpicks and feedback incorporated.

- #attached_library doesn't exist yet.

- @see is perfectly valid in between. To be used where it makes sense most for the reader of API documentation to refer to related information.

+++ includes/ajax.inc	17 Aug 2009 06:00:17 -0000
@@ -0,0 +1,712 @@
+    // This is likely a hacking attempt as it never happens under normal
+    // circumstances, so we just do nothing.
+    exit;

Not even a watchdog?

+++ includes/ajax.inc	17 Aug 2009 06:00:17 -0000
@@ -0,0 +1,712 @@
+    // If the returned value is a string, assume it is HTML and create
+    // a command object to return automatically.

And if it's not then...?

+++ includes/ajax.inc	17 Aug 2009 06:00:17 -0000
@@ -0,0 +1,712 @@
+    $html = $callback($form, $form_state);
...
+      $commands = array();
+      $commands[] = ajax_command_replace(NULL, $html);

Use $commands = array(ajax_command_replace(NULL, $html));

But why not just $commands = $callback($form, $form_state); and then use this "if string convert to whatever"? Saves the else (but not the comment I asked for in the previous hunk!)

I stopped reading approx here because a) it's too late and I have a long flight tomorrow (sorry) b) apparently there is a new array structure in town which does the awesomeness and even if it's documented somewhere there needs to be an awful lot of @see to that place.

15 days to code freeze. Better review yourself.

Incorporated most of chx's feedback.

Ready to hit the floor to do some windmills and foot-work, to dis #ahah. Yay! Where's the party at?

You know, I honestly spent quite some time looking for something to nitpick in the code, but it seems like that's basically all been done for me. ;) There are a couple of spots I still find a bit confusing, and I'm sure the documentation could be improved even more, but this has the buy-in from every major player in this space and I'm pretty confident anything else remaining could be handled in a follow-up patch.

Therefore... committed to HEAD!

This will definitely need some upgrade docs. Marking needs work for upgrade documentation.

Sorry!! I get the dunce cap of shame. I remembered ahah.js and ajax.js but missed ajax.inc. All of our lovely docs! Sniff!

Should be good now.

Yay ! I'll update #367567: Use AJAX framework for "Add more" / load include file containing form / introduce $form_state['build_info'] with a new patch when I return to my coding env in a couple days.

Hi all, just stumbled in via earl's tweet; sounds bloody awesome and all that, and trying to wrap my head around it...

Question: does this have any facilities (or leave room for) content [type] negotiation? e.g. jQuery.ajax() options let the client specify the preferred format of the response -- xml, html, json, jsonp, script, or [plain]text -- by setting the Accept HTTP header on the request, and the server/php can process & respond accordingly...

I can understand if this is a non-concern to the discussion of applying this to core, whereas everything in core can follow a JSON/DJON declared "Drupal Way"... but should this consideration be addressed & made available for emerging applications, cross-app interop, etc?

Minor follow-up, adding the requested watchdog() I forgot, and reverting the command syntax.

Additionally, the links to the ajax_command group didn't work out. So trying to fix that, too.

However, looking http://api.drupal.org/api/group/ajax/7, it seems we can just link to that page in the upgrade docs.

so, well - not sure whether it groks that combination of @see and @link, but we'll see.

Committed to CVS HEAD. Thanks!

Thanks!

Still needs documentation. I guess it boils down to replacing the FAPI property #ahah with #ajax (no inner changes). Advanced AHAH usage needs to be replaced with the new command structure, but that's explained in detail in the API.

I have put this into the module update guide: http://drupal.org/node/224333#ahah-now-ajax -- if someone could look over the example and verify that it was a good choice, and/or put in a different one that's better, that might be useful.

I can probably also update the FAPI reference page (http://api.drupal.org/api/file/developer/topics/forms_api_reference.html/7, which is in the contrib repository), though I am not sure what to do with the current #ahah section... I'm tempted just to refer to other doc, such as http://api.drupal.org/api/group/ajax/7 -- or is there more information elsewhere, with examples? Here's the current AHAH section:
http://api.drupal.org/api/file/developer/topics/forms_api_reference.html...

However, before I do that editing, one thing I noticed is that the doc page http://api.drupal.org/api/group/ajax/7 doesn't include the #ajax['progress'] parameter, although it appears that it still applies.

+++ includes/ajax.inc 17 Aug 2009 04:37:08 -0000
@@ -0,0 +1,687 @@
+ * - #ajax['wrapper']: The ID of the AJAX area. The HTML returned from the
+ *   callback will replace whatever is currently in this wrapper. It is
+ *   important to ensure that this wrapper exists in the form, and is often
+ *   created using #prefix and #suffix tags.

>important to ensure that this wrapper exists in the form

Is it true the wrapper can only be part of the form? What if I want ajax update to some area outside of the form?

I'll work the docs. If anybody has any additional info you want to communicate, give me a yell.

@mattyoung (#90) I can confirm through experimentation that, as always, there is no need for the ID to be within the form. However, I will try to figure out if this is just a bug in the docs or if there's some reason for it.

I updated the upgrade guide to the current API, so now people should be able to upgrade successfully. However, this remains a pet issue of mine and I'll attempt to get much better docs of the whole out there. For now, I've posted a working Drupal 7 AJAX module on http://randyfay.com/ahah/drupal7 and will work toward complete docs including docs of the new ctools-derived AJAX Framework Commands.

@jhodgdon: Re #89:

• 'path' and 'callback' are mutually exclusive, and 'path' is (edit) for complicated usages. 'callback' will work for most simple cases, but 'path' is required when you get beyond basic usage, see http://drupal.org/update/modules/6/7#ahah-now-ajax. So we want to point that out in the Ajax framework page as well as in the API doc.
• The statement in http://api.drupal.org/api/group/ajax/7 "#ajax['wrapper']: The CSS ID of the AJAX area. The HTML returned from the callback will replace whatever is currently in this wrapper. It is important to ensure that this wrapper exists in the form." is incorrect as far as I've been able to determine. The ID can be anywhere in the page, although it's most common that you would be replacing a portion of the form.
• I agree with you about #ajax['progress'] - As far as I can tell it is unchanged from D6.

>"It is important to ensure that this wrapper exists in the form." is incorrect as far as I've been able to determine. The ID can be anywhere in the page, although it's most common that you would be replacing a portion of the form.

What if I want to update multiple wrappers all over the page? Can this framework support this?

What if I want to update multiple wrappers all over the page? Can this framework support this?

Of course. Build your own callback and send insert/replaceWith command(s) with a jQuery selector that targets your wrappers.

@merlinofchaos: I agree, deprecated is not the right word. A better way to say is is "for simple jobs, 'callback' will almost always be used. 'path' is for situations that the generic code that calls 'callback' does not accomplish.

@mattyoung: With the framework as it is, you get one wrapper per #ajax. However, if you use merlinofchaos's far more extensible Ajax Framework Commands, then you can issue multiple commands at once, and before long you'll build panels :-)

I updated the Form API Reference with the updated information. Commit #256138

Attached is the patch for ajax.inc to update the documentation included in that file.

A couple of minor issues with the patch:

a) I think a couple of the lines are more than 80 characters.
b) I wouldn't mention "...is new in Drupal 7". We generally don't in API doc.
c) Maybe refer to #ajax['callback'] as "the name of a callback function" rather than just "callback", for clarity.
d) I couldn't figure out from that doc what HTML is replaced by the return value of #ajax['callback']? 'wrapper' just says it is used for 'path'.

Regarding your changes to the FAPI doc, you changed a LOT of lines -- see http://cvs.drupal.org/viewvc.py/drupal/contributions/docs/developer/topi... -- so it's really hard to review. Sigh.

@jhodgdon, I used an HTML editor due to the difficulty of working in HTML with those horrible examples. It added missing elements like tbody to the file. Sorry for the inconvenience. However, when I did a diff, it was obvious what were semantic changes and what were line wrap changes. My preference is always to just work with HTML, but with a document this complex, I was forced out of that strategy.

It isn't obvious which are line wrap changes on that page cited above -- could have slipped in a word change and no one would be the wiser. ;)

Anyway, thanks for the edits!

Here's the updated patch reflecting comments in #99.

+++ includes/ajax.inc
@@ -26,22 +26,24 @@
+ *   This is an easier technique to use than #ajax['path'], but may not meet every
+ *   need as #ajax['path'] is more powerful. Note that #ajax['callback'] currently only
+ *   works for buttons (submit, button, image button) and not for other element types.

Can we move the "Note" before the sentence about 'path' ?

(and elsewhere) Lines are exceeding 80 chars.

+++ includes/ajax.inc
@@ -26,22 +26,24 @@
+ * - #ajax['wrapper']: The CSS ID of the area to be replaced by the HTML returned by
+ *   the #ajax['path'] function. The HTML returned from the
+ *   callback will replace the entire element named by #ajax['wrapper'].
+ *   The wrapper is usually created using #prefix and #suffix properties in the form.

We should additionally state that 'wrapper' must be used without leading # sign. Found it out the hard way on my own. ;)

+++ includes/ajax.inc
@@ -54,6 +56,10 @@
+ *   Possible keys: 'type', 'message', 'url', 'interval'. More information is available
+ *   in the Form API Reference.

A @see would be nice here. (on a new line)

Beer-o-mania starts in 3 days! Don't drink and patch.

Sub.

Here is the updated documentation patch re: the issues in #104. Thanks for your review, @sun. And I finally learned how to turn on Eclipse's 80-character limit line :-)

Re-rolled. Looks RTBC.

Looks like both sun and rfay like this patch, the testing bot likes the patch, and I also think it looks good from a structure and writing point of view. That should be enough. :)

Committed to HEAD. Thanks!

Thanks again to everyone who worked so hard to get this AJAX framework into core. I'm looking forward to seeing some pretty sweet contrib modules come out for D7 that take advantage of it. I submitted a follow-up issue to address a WTF that's been bugging me with respect to AJAX callbacks having a fundamentally different return signature than normal page callbacks: #599804: Unify page, AJAX 'path', and AJAX 'callback' callbacks. I know most of you are super busy with a ton of other issues, but if any of you can spare some time to review it, I'd appreciate it. Thanks!

Documentation followup on this is moving to #610068: Document AJAX no-js and use-ajax. Many issues can be combined there.

Seeking help from someone who understands how AJAX and FAPI are supposed to work together. There appears to be a critical bug in this regard that might require an API change. But we're stuck on knowing what the bug is, because we're confused about how multi-step FAPI forms are supposed to work. Please see http://drupal.org/node/622922#comment-2235954.

(Sorry for my english, I'm French)
I've got this error message when I want to simulate the shipping

Une erreur HTTP AJAX s’est produite.
Code de statut HTTP : 500
Informations de débogage ci-dessous.
Chemin : /drupal/drupal-7.12/ ?q=system/ajax
StatusText : Internal Server Error
ResponseText :

I've try to apply your patch but I've obtain this error message

root@sylvain:/var/www/drupal/drupal-7.12/includes# patch -p1 --dry-run -i drupal.ajax-docs.patch
patching file ajax.inc
Hunk #1 FAILED at 26.
Hunk #2 FAILED at 53.
2 out of 2 hunks FAILED -- saving rejects to file ajax.inc.rej

Anyone could help me please ?

Write your first ajax form submit.....

$items['testForm'] = array(
		'title'           => t('testForm'),
		'page callback'   => 'drupal_get_form',
		'page arguments'  => array('node_list_add_to_list_form'),
		'access callback' => TRUE,
		'type'            => MENU_CALLBACK,
);
//////////////////////////////////////////////////////////////

function node_list_add_to_list_form($form, &$form_state, $nid) {
  
  // Wrapper for entire form
  $form['#prefix'] = '<div id="all-fields">';
  $form['#suffix'] = '</div>';  
  
  // Test button
  $form['test'] = array(
    '#type' => 'textarea',
   
     
  );   
  
  $form['submit'] = array(
    '#type' => 'submit',
    '#value' => t('Submit'),
  		'#ajax' => array(
  				'callback' => 'node_list_add_to_list_form_submit_commands',
  				
  		),
  );      
  
  return $form;
}
//////////////////////////////////////////////////////////////


function node_list_add_to_list_form_submit_commands($form, $form_state) {

	if (!form_get_errors()) {
		$commands = node_list_add_to_list_form_submit($form_state);
		return array('#type' => 'ajax', '#commands' => $commands);
	}

	// Reload form if it didn't pass validation.
	return $form;
}


//////////////////////////////////////////////////////////////////////////////////


function node_list_add_to_list_form_submit($form, $form_state) {
  
	ctools_include('ajax');
	ctools_include('modal');
	
	//write your INSER/UPDATE DB function.
	
	//message
	$title = t('Message added successfully');
	$message .= 'Message added successfully ';
	drupal_set_message(check_plain(t($message)));
	$commands[] = ctools_modal_command_display($title, theme('status_messages'));
	
	print ajax_render($commands);
	exit;
}

Thanks
Shantanu Karmakar