[D7] Ajax links API

Welcome,

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Make sure to do manual reviews of other project by reading through their source code and reporting issues that you find. Posting results of automated reviews is not enough. If you did that don't forget to add the "PAreview: review bonus" tag as described in #1410826: [META] Review bonus.

Regarding the errors: "@param $var" should be "@param string $var" where string is the parameter type. Same for "@return" vs. "@return int". See http://drupal.org/node/1354#functions

You can keep the short function name, I think that is a valid exception.

review bonus links

Your "manual review"'s are only pointing out single issues, most of them are minor others are simply wrong or just links to automated reports

sorry, removing review bonus, it would not be fair to others who really make efforts for their reviews

review bonus links

manual review:

1. l_ajax(): do not call theme_ajax_links_api() directly, use theme('ajax_links_api', ...) and hook_theme() so that others can easily override it.
2. ajax_links_api_menu(): why is the test menu path unprotected? I think it should also use an administrative permission.
3. ajax_links_api_footer(): remove that function, you can add scripts to the footer with the scope option on drupal_add_js().
4. ajax_links_api_preprocess_page(): I think you can use hook_theme() to register your template.

Otherwise this looks nearly ready. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Thank your for your reviews. When finishing your review comment also set the issue status either to "needs work" (you found some problems with the project) or "reviewed & tested by the community" (you found no flaws) as indicated in #1410826: [META] Review bonus.

With preprocessing for example

project description

nothing that's the normal behaviour.
Create your new branch while beeing in 6.x-1.x, make your changes that are required for your port, commit and push the branch

this way anyone can see what and how it was ported and it's easier to detect error that came due to the port.

review bonus links

I do not understand why I should copy template files to my theme.
Many user use some of the default themes which should not be changed any way (as they are in Drupal core). I should use your module with such theme also without killing kittens ;)
You have settings form with defining selector. So why also changing theme?

In your .install file you have

/**
 * @file
 * Make any links or create new links with ajax.
 */

That is not what file does. Of course it is not a bug, but probably better comment would be: "Delete unnecessary variables." or "Uninstall function for AJAX links API module."

In your .module file:

 /**
 * Implements hook_init().
 */
function ajax_links_api_init() {
  drupal_add_js(drupal_get_path('module', 'ajax_links_api') . '/files/ajax_links_api.js');
  drupal_add_css(drupal_get_path('module', 'ajax_links_api') . '/files/ajax_links_api.css');
}

Why not to add these files into .info file? They will be properly aggregated then.

Do not use t() function in hook_menu.

Your .js function:
Try to use Drupal.behaviors instead of document.ready.

I think l_ajax() function is a good name, even if does not follow coding standards. I think that such API should not provide lengthy function names, it is simple and clean.

You can also add configure option to your .info file. It will display small icon on the admin/modules list.

With overlay enabled I cannot use your module properly. Link is appearing in overlay (demo page) rather than in my #content. You should provide information about how your module works with core overlay module.

If i do not have an access to the page it only shows loader and do not show access denied page for me. I do not know if it is something specific with my Drupal installation, but I think I should mention it.

I'm not sure if this issues should block your application into RTBC, I've also did a review of 7.x branch.
But for now it requires changing core in some situations, it does not like core overlay module and uses t() in hook_menu(), so I'm changing to need work.
If someone think that it should be needs review - feel free to change.

Review of the 7.x-1.x branch:

• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

• ajax_links_api.info: why do you need your CSS/JS on every single page request? I think you should only add it when you actually output an ajax link, no?

But otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

You were adding css and js file on every single request through hook-init(). It makes your files no aggregated when user enables aggregation so if you need your files on EVERY page you shoud add them to .info file.

Although I should have asked if you really need your files on every request before suggeting moving it into info .file. But it is not an application blocker, so I did only little harm ;)

I do not have an access to your repository right now (using mobile phone), so I'm not able to give you full and right answer based on code, just talking about some theory for now.

Imagine situation where ajax link is only on one page - adding your files to every page might not be so good idea, because all other pages will not use your style and scripts. It would be better to add them only to pages that have your link.
But if you are sure that you really need your files on every request - I suggest .info file for proper aggregation.

I can look at your code later and we can work out the best solution if you will be having problems decide what option choose or how to implement it.

It appears that you have been granted the 'create full projects' permission:

Once their first application has been successfully approved, then an applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue.

Hi, i want to use this module but he is loading all css, js files after an ajax request is something wrong with how i use this module, because i just want to load at least just css,js files for loaded content and not for the entire page. Ty for the feedback.

This issue is not the right place for feedback or support requests.

Please create a support issue in the projects issue queue instead and leave closed issues closed.

explaining why page-ajax is required

Hi,

I'm using ajax_links_api and every page is loading via ajax, except for the homepage. I can see response in "network" section of firebug, but it's not loading.

How can I debug that?

Thanks

Hello,

Ajax links API module and is working fine the problem is that is affecting other modules in the page

http://isafe.org/?q=content/design-home-page

I'm using a view to work as a search, in this view I'm not using Ajax

below that view I'm using the Ajax links API module to display some pages, the problem is when I click on a button to display the page the view above this section gets affected

I'm not very good with javascript and I'm very new in Drupal I'm using Drupal 7

Thank you for your help