Posted by nedjo on September 29, 2009 at 5:22pm
| Project: | Views |
| Version: | 7.x-3.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | merlinofchaos |
| Status: | closed (fixed) |
Issue Summary
#343542: Enable optional auto-refreshing views using AJAX adds some new complexity to ajax_view.js. In preparation for D7, there's a move to start using CTools.
To ease the upgrade, we should look at rewriting ajax_view.js to duplicate what CTools is doing.
Comments
#1
For 2.x, I think we should duplicate CTools ajax.inc and ajax-responder.js, and in 3.x actually rely on CTools for this functionality.
#2
subscribe
#3
+1 for creating a dependency on CTools for 3.x and share the code base for stuff like this.
#4
I've taken a first look at requirements.
Currently we have a separate menu callback for AJAX requests and special handling in the JS to parse out view attributes, which are passed along with the request as post variables.
The basic changes needed look to be:
1. Use a single callback for nojs and ajax requests. Rewrite to test for 'ajax' and conditionally use ctools_ajax_render(). The most obvious place to put this would be views_page() (which we would then have to add access tests to and expose to the menu system). Something like:
<?phpctools_include('ajax');
// Load the view
if ($view = views_get_view($name)) {
$output = $view->execute_display($display_id, $args);
if ($type == 'ajax') {
$commands = array();
$commands[] = ctools_ajax_command_replace('#some-div', $return);
ctools_ajax_render($commands);
}
else {
ctools_add_js('ajax-responder');
return $output;
}
}
?>
2. Append /nojs to all views links and to the target of exposed filter forms.
3. Also append to these links and target the minimum data to identify the request. This is what I'm not so sure of. Currently we're posting a lot of data to views_ajax: view_name, view_display_id, view_args, view_path, view_dom_id, pager_element. Append all this to URLs? Ugly. Do we need to register the current view path as an alias of this ugly real path?
4. Add ctools-use-ajax class to relevant links (views pager, table header, and summary attachment links) and add ajax properties to exposed filter forms.
Does this sound vaguely right? What do we do about passing the view identification information to a callback?
#5
I would wait until d7views is ready, because the js there should have been converted to the new ajax framework, which was created out of ctools ajax.
#6
Most of that work has already been done (except for preview which should not be difficult) and could potentially just be backported to D6.
#7
any updates here?
#8
Likely it would make sense to do #343542: Enable optional auto-refreshing views using AJAX first for D7 to provide the incentive for this considerable refactoring.
#9
Subscribing.
#10
subscribing
#11
This will not happen in d6, because it will not require ctools, but it should be done in d7.
#12
Here is a first proof of concept patch that uses D7 core AJAX API. There are still plenty of rough spots. One key problem: all AJAX requests are using HTTP POST (it is hardcoded into
misc/ajax.jsand there is a rather unsightly hack to accommodate this inviews_ajax(). Besides hacking values from$_POSTinto$_GET, the requests will also require more custom configuration to work with proxy caches like Varnish. One option would be to extendDrupal.ajaxto use GET or clean up the hack in place.Some testing and additional direction would be useful here before the next iteration.
#13
Second version. We can avoid
$_POSTrequest for Views AJAX by extendingDrupal.ajax. Unfortunately the wayDrupal.ajaxworks requires a bit more copy and paste than the couple of lines we need to change, but it is effective and works without needing changes to core.New patch uses HTTP GET, removes POST to GET vars hack, and extends
Drupal.ajaxinDrupal.Views.Ajaxto allow for GET requests.#14
Updated v2 patch to have more selective overrides (overriding
Drupal.ajaxinstances directly rather than doing prototype inheritance). Still faces shortcomings described by earl on IRC of not being able to update JS/CSS dynamically and take advantage of D7 AJAX APIajax_page_state.However making use of this portion of the API is problematic with GET because the information needed to be passed back to Drupal (
ajax_html_idsandajax_page_state) totally blows through the GET url length limit on most HTTP servers. The request payload can easily reach 5k-15k+ on even simple pages (as to whether the size itself is an issue with a POST request is another topic).For now here is the cleaned up patch for the GET approach, more to come...
#15
I've updated a small issue in the patch (the required
misc/ajax.jswas not getting included on all pages where it was needed). The second version of the patch can be used in conjunction with #956186: Ensure it is possible to use AJAX with GET requests which fixes the AJAX framework request size and also enables use of GET with page state asset updating etc.#16
Subscribing
#17
applied #15 and got
when using the pager of my view.
#18
Just made a minor change to the patch to fix #17 - changed views_add_js so that misc/progress.js and misc/ajax.js both get added when either ajax.js or ajax_view.js is being added.
#19
Assigning to merlinofchaos, as agreed on.
#20
Cleaned up a little and committed. This is no longer forcing $_GET, but instead copying $_POST to $_GET on the other side during AJAX requests.
#21
Automatically closed -- issue fixed for 2 weeks with no activity.