Notice Wim's comment on http://drupal.org/node/367567#comment-1345762. The issue is that D6 CCK and now D7 core FieldAPI takes the following 4 lines of code to return an AHAH response:

$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));

And line 2 of above is pretty nasty. Furthermore:

  1. Any AHAH callback that needs to return settings and status messages along with content needs to run the same lines of code.
  2. I think the order of the above steps is wrong. In theory, rendering the element may result in additional status messages and javascript settings being created, so I think it would make more sense to first render the element, then theme the status messages, and only then capture the javascript settings.
  3. Other modules (not just the one running the AHAH callback) may want to add more to the returned json object besides "status" and "data". Not only does the above code not make that easy; it actually make it impossible because drupal_json() prints directly to the browser and the next line of code after the above is usually "exit;" in order to prevent modules like devel from adding more content to the bottom.

For D6, I created the ahah_reponse module to address the above issues. It's a simple module that just adds a "ahah_response" theme hook, allowing the above code to be replaced with:

print theme('ahah_response', drupal_render($field_form));

Is there community interest in making something like this part of core for D7? I'm happy to create the patch file for it if there is. Before I do though, I'd like to request:

  1. If you think this is a good idea, please add a comment indicating that.
  2. Please review the existing ahah_reponse module for D6, and comment on whether you'd like a different implementation. Specifically, do you like "ahah_response" and "ahah_response_settings_js" as the names of the theme hooks?
  3. Would adding these hooks to drupal_common_theme() and the default tpl file to modules/system be appropriate?

Comments

quicksketch’s picture

Standardized return JSON would be a great feature. I'm not particularly sure if it should be a theme function however, AHAH returns aren't really display-related, the JSON delivery mechanism is almost pure-functionality, everything returned should have been passed through the theme system already.

Regardless, I'd like to see this standardized, especially as we start adding more items to the returned JSON. I've posted a patch in #507108: Allow AHAH to reset Drupal.settings after loading response data that returns the Drupal JavaScript settings back to the calling page, making the returned JavaScript have 3 keys instead of 2. And ultimately it would be good to separate messages from output, so you can return a "status" => FALSE, but send along a message that would be displayed.

So I think the standard return should look something like this:

// Get the current JavaScript added to the page.
$js = drupal_add_js();

drupal_json(array(
  'status' => $status,
  'messages' => theme('status_messages'),
  'output' => $output,
  'settings' => call_user_func_array('array_merge_recursive', $js['settings']['data']),
));

Later on we might also add "css" and "js" keys, and have the originating page auto-load the files as necessary.

sun’s picture

robloach’s picture

Issue tags: +JavaScript, +ahah

Subscribing!

Nick Lewis’s picture

Feel free to bite off my head. Isn't a giant javascript super global variable the definition of evil? The very fact that it needs to be returned in such a fashion points to a deep structural flaw imho.*

I'm very interested in this topic. Its pretty bad at this point. I'd say its easier to hand write a form using the good old [input tags and name="a_form_name_that_actually_appears_omfg" required="sign... nay" /] than mucking about with formapi and ahah. Frankly - I write old fashioned forms quiet often because its predictable behavior, regarding a form that doesn't need formapi's features (e.g. alter, built in security) vs. who knows how long it will take, is it even possible, and how far into form.inc must i dig to find the key?

I think '#ahah" is flawed, personally - though if it can become easier than writing out form by hand I'm all for an improved version of it. Personally, I think the solution is:
1. A special function, lets call it:

function drupal_rerender($fields, $form_id, $return_whole_form = false) {
   // rough chicken scratch flow:
  // lawl -- and it must not yet be rendered 
  $form = get_whatever_form_cache_will_make_the_form_not_error_on_validation($form_id);
  // leave it to the the programmer to make sure he doesn't write  $fields['#node']['#type'] = 'im_stoopid';
  $form = array_merge($fields, $form); 
  write_form_to_whatever_cache_or_thing_it_needs_to_to_work($form);
  return $form;
} 

This requires a $form_id, an array of fields to add to the form, and some javascript that actually figures out out how to return it to the form. In my humble opinion, the formapi should require no more... and jquery is so much easier than drupal that i question drupal's need to abstract away javascript as long as it offers features to actually use javascripts + formapi without taking a train to programming hell.

Good work with the #ahah module, btw -- though again, it seems mad that the module has spawned like 5 ahah_whatever helper modules all of which appear to merely be nothing but workarounds that allow flexifield to work... [flexifield is awesome btw]

Note:
*BasePath is useful enough in Drupal.settings -- and arguably defining unchanging constants on a page is too... but having a $.ajax response replace the whole bloody thnng with what (it appears at least) PHP thinks it should be is pure madness.

quicksketch’s picture

Isn't a giant javascript super global variable the definition of evil? The very fact that it needs to be returned in such a fashion points to a deep structural flaw imho.

Yes, the basic principle of using a single giant global variable is terrible. Which is why we have #360081: Pass Settings Locally to JS Behaviors now, so that behaviors don't need to ever check that global variable any more. However there's still the need to get the settings onto the page somehow from PHP. We could potentially now move all these global variables into a local scope and hide them from the rest of the page.

This really isn't an issue to debate whether FAPI is worthwhile or not, but to make #ahah easier (and hopefully more worthwhile) to use. It's already a lot easier in D7, see the poll module implementation before and after. I think this proposal will further simplify things by simply letting the user say "give me this markup" and pass that to the function to deal with standard things like messages and settings.

effulgentsia’s picture

@quicksketch: I can see your point about why this shouldn't go through the theming layer. However, I would like contrib modules to have a hook into the response object before it gets sent to the browser, so that if we don't get keys like css and js into core, a contrib module could add them.

What do you think of this approach?

function drupal_ahah_response($output, $status = TRUE) {
  $js = drupal_add_js();
  $ahah_response = array(
    'status' => $status,
    'messages' => theme('status_messages'),
    'output' => $output,
    'settings' => call_user_func_array('array_merge_recursive', $js['settings']['data']),
  );
  drupal_alter('ahah_response', $ahah_response);
  drupal_json($ahah_response);
}
effulgentsia’s picture

Or, perhaps even:

function drupal_ahah_response($id, $output, $status = TRUE) {
  $js = drupal_add_js();
  $ahah_response = array(
    'id' => $id,
    'status' => $status,
    'messages' => theme('status_messages'),
    'output' => $output,
    'settings' => call_user_func_array('array_merge_recursive', $js['settings']['data']),
  );
  drupal_alter('ahah_response_' . $id, $ahah_response);
  drupal_alter('ahah_response', $ahah_response);
  drupal_json($ahah_response);
}
quicksketch’s picture

Passing in an ID and using drupal_alter() both sound like great ideas to me. However the ID-specific alter in #7 seems a bit much, it'd be the equivalent of calling drupal_alter('node_load_' . $nid), if you will. I know we do something like that for preprocess functions, but generally I think just having an ID will be enough for most purposes.

function drupal_ahah_response($id, $output, $status = TRUE) {
  $js = drupal_add_js();
  $ahah_response = array(
    'id' => $id,
    'status' => $status,
    'messages' => theme('status_messages'),
    'output' => $output,
    'settings' => call_user_func_array('array_merge_recursive', $js['settings']['data']),
  );
  drupal_alter('ahah_response', $ahah_response);
  drupal_json($ahah_response);
}
merlinofchaos’s picture

I would like to interject.

In CTools ajax.inc, my responses look like this:

  $commands = array();
  $commands[] = ctools_ajax_command_replace('#id', $html);
  $commands[] = ctools_ajax_command_settings($settings);
  $commands[] = ctools_ajax_command_whatever($data);
  ctools_ajax_render($commands); // this performs an exit.

I've been *extremely* happy with this format. If it were converted to Drupal, the function names would be shorter:

  $commands = array();
  $commands[] = ajax_command_replace('#id', $html);
  $commands[] = ajax_command_settings($settings);
  $commands[] = ajax_command_whatever($data);
  ajax_render($commands); // this performs an exit.

This is a lot more readable and a lot more flexible than what we already have. The responder on the javascript side just executes all of the commands, uncaring as to what they are. If we wanted some *required* commands, we could always do:

  $commands = ajax_commands($id, $messages, $output);
  $commands[] = ajax_command_settings($settings);
  ajax_render($commands);

I've been tempted to try to merge my stuff with ahah.js, but right now there is a fundamental difference in what I do vs what ahah.js does, which is that ahah.js has a very very particular response requirement, where mine leaves it up to the server to decide. I'd be willing to try to work on a patch for this, as better ajax tools is one of my goals.

quicksketch’s picture

I think merging in some of the cTools AJAX functionality would be very welcome. I agree right now our current mechanisms are very tied to FAPI, which I know has been one of your long-standing complaints about the system. However, I think the FAPI integration is super-handy in a lot of situations. Considering the better properties such as $element['#ahah']['callback'], I think we'll see dramatically reduced learning curve for D7 #ahah usage. Compare:

http://api.lullabot.com/poll_choice_js/6
http://api.lullabot.com/poll_choice_js/7

So generally I'd like to keep the current FAPI integration through #ahah. However, that doesn't mean that we can't leverage the approach used by cTools, which is more flexible and not tied to FAPI at all. Instead we could fully implement the cTools functionality, but just have the #ahah property make the appropriate ajax_command_x() calls.

In D7 with the use of #ahah['callback'], we'll ultimately have a single menu callback for ALL #ahah properties. This has a tremendous advantage in that it doesn't require every module to re-implement a "ahah/my_module" path for every use of #ahah. It also makes it possible for us to take this standard syntax and obey the #ahah properties rather easily, since we actually have access to the original #ahah definition.

Starting with the current universal callback function:
http://api.lullabot.com/form_ahah_callback/7

We could enhance it so that it works something more like this:

  // Get the callback function from the clicked button.
  $ahah = $form_state['clicked_button']['#ahah'];
  $callback = $ahah['callback'];
  if (drupal_function_exists($callback)) {
    $html = $callback($form, $form_state);
  }

  $commands = array();
  $commands[] = ajax_command_replace($ahah['target'], $html);
  $commands[] = ajax_command_settings($settings);
  ajax_render($commands);

In other words, we yank out all the Drupal.settings.ahah entirely from the JavaScript side. We always assume that all AHAH enabled buttons call the same menu entry, and then on the server side we take the FAPI properties out of the #ahah property and generate the matching cTools AJAX commands.

This approach let's us keep the convenience of #ahah while allowing other modules the opportunity to use cTools' more flexible approach, completely independent of FAPI.

merlinofchaos’s picture

So, extrapolating from quicksketch's snippet above, poll_choice_js becomes this:

<?php
function poll_choice_js($form, $form_state) {
  $choice_form = $form['choice_wrapper']['choice'];

  // Prevent duplicate wrappers.
  unset($choice_form['#prefix'], $choice_form['#suffix']);
  return drupal_render($choice_form);
}

Even smaller! I think this is all doable, and it will give us a superior framework for client/server interaction that will allow the onus of the work to be server side (which I think most will find preferable, being as how the PHP experts vastly outnumber the JS experts in our community) and still allows very simple interactions when used for the constructions in FAPI that we currently enjoy.

quicksketch’s picture

Yes, that's right! I would enjoy this change even more than our current system. It would make returning snippets of the form trivial (well, after #370537: Allow suppress of form errors (hack to make "more" buttons work properly) gets finished :-/), and even better than "standardizing" the return, we'd make it drastically simpler. I'll be happy to review changes as this progresses.

Frando’s picture

Huge +1 from my side to everything said here by quicksketch and merlinofchaos. The CTools ajax responder is much more flexible than what we have, so let's try to get it in core and make #ahah use it. Sounds amazing.

Roi Danton’s picture

+subscription. With ctools_ajax_render in core ctools multistep form should follow.

merlinofchaos’s picture

I created a new patch that includes everything quicksketch adn I spoke about here: http://drupal.org/node/544418

I made it separate since there's a couple of things in this issue I didn't address (in particular, standard handling of messages). That can be integrated pretty easily either in that patch or shortly afterward.

sun’s picture

Status: Active » Closed (duplicate)

Thanks for taking the time to report this issue.

However, marking as duplicate of #544418: Integrate CTools AJAX framework with Drupal to extend (and replace) existing ahah framework.

effulgentsia’s picture

Agreed. #544418: Integrate CTools AJAX framework with Drupal to extend (and replace) existing ahah framework solves everything I was hoping for and more. Except the status messages, which I created a new issue for: #551694: Standardize the return of status messages in AJAX callbacks. Thank you, merlinofchaos (and others), for the outstanding contribution!

davidshaw’s picture

The patch above looks more comprehensive.
My solution is a hack... but I simply wanted to not show any status messages when the AHAH callback ran.
Commenting out the line which outputs it just made the status messages pile up and show after the final real form submit:

// commenting out didnt work. they pile up and show later
// $output = theme('status_messages');  

But i did work out that runing them through the theme() function and assigning them to a bogus variable made them not appear now and not appear later either!

// assigning to a bogus unused variable clears them out and never appear.
$bogus = theme('status_messages');  
$output = drupal_render($form);
  // Final rendering callback.
  print drupal_json(array('status' => TRUE, 'data' => $output));
  exit();

Dave Shaw
Tribe Rising