[D7] Ajax page load with progress bar

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxAlons2155421git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

While giving GIT clone command, provide the non-maintainer command : "git clone --branch 7.x-1.x http://git.drupal.org/sandbox/Alons/2155421.git ajax_page_load_with_progress_bar"

PAReview Passed:
http://pareview.sh/pareview/httpgitdrupalorgsandboxAlons2155421git

Manual Review:
1. I didn't see progress bar on top of the page. Maybe because page loaded fast
2. It will be great if we can configure other settings like speed, opacity in the future
3. Better use standard drupal_set_message here and display that message immediately after installation of the module.
Also are you sure that we should use t() for t('http://ricostacruz.com/nprogress/') ?

  if (!_ajax_page_load_library_path()) {
    $options = array('attributes' => array('target' => '_blank'));
    $url = l(t('http://ricostacruz.com/nprogress/'), 'http://ricostacruz.com/nprogress/', $options);
    $message = t('You need to install the jQuery nprogress plugin. Create a directory in sites/all/libraries called nprogress, and then copy nprogress.js into it. You can find the plugin at !url.', array('!url' => $url));
    $form['ajax_page_load_nolibrary'] = array(
      '#markup' => '<div style="color: red">' . $message . '</div>',
    );
  }

Hi

Further to the above comments, I've also reviewed your module, followed the instructions to install the nprogress library, then tested it worked by

1. setting selector to ul.main-menu on the administration page.
2. Create some content and add them to the main menu
3. Change from one page to another page in the main menu

I did see the progress spinner at the top of the page, as well as the progress bar, but it seemed to show them mostly after the content had already been replaced.

Either way, only issue I could see was something mentioned in the Tips for ensuring a smooth review (https://drupal.org/node/1187664#issues), your module doesn't remove the Drupal variables it defines.

Thanks

• Improve project page.
• Improve readme.
• Don't give files execution rights.
• Loading javascript and css is both in info and init.

Hi,

Here are my reviews after installing the module:

1.

function ajax_page_load_check_library() {
  if (!_ajax_page_load_library_path()) {
    $options = array('attributes' => array('target' => '_blank'));
    $path = 'http://ricostacruz.com/nprogress/';
    $url = l($path, $path, $options);
    $message = t('You need to install the jQuery nprogress plugin. Create a directory in sites/all/libraries called nprogress, and then copy nprogress.js into it. You can find the plugin at !url.$
    drupal_set_message($message, 'error');
  }
}

Theres nothing wrong with the above codes, but i would like to recommend that instead of using the author's website url why not use the github link since its in that site where we will be downloading the plugin .

2. I think its standard to have a separate folder for your css file

3. When i tested a link from my main menu ajax page loads ok, but if it loads my timeline page(using jquery plugin timeliner) it breaks it. You can check it from here: demo.fourmindstech.com

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

This application is not fixed since you don't have the git vetted user role yet? See the workflow: https://www.drupal.org/node/532400

Code looks pretty good. I see you have a css directory with a css file in it. You have the same css file in the root of your project and the code references the file in the root. You should either update your code to reference the css file in the css directory or delete the css directory.

One stylistic change I would suggest is adding a @return and @parameter information to the function documentation.

A nice to have is for this project to add the required ngprogress library upon installation or activation of the module.

Forgot to change the status.

I like this module! PAReview is showing some new issues though - http://pareview.sh/pareview/httpgitdrupalorgsandboxalons2155421git

Definitely need to remove the duplicate CSS file, I'd get rid of the one in the root directory and keep the css directory, so it matches the js setup.

My main issue though is with the opacity animations - it's fading out and then in again on the original page, then the new page loads. Should really only fade out on the original page and fade in on the new page, which can be achieved by changing:

var progressDone = function() {
  $('body').removeClass('load').animate({opacity: 1}, 500); NProgress.done();
}

to:

var progressDone = function() {
  NProgress.done(); 
  $('body').removeClass('load').animate({opacity: 1}, 500);
}

I also think the opacity reference on body.load in the css file is redundant. It seems to cause a flicker. Setting the opacity in the JS is enough.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

Please update the description of this article with instructions to install nprogress.js.

Review template.

Automated Review

Found some minor warnings http://pareview.sh/pareview/httpgitdrupalorgsandboxalons2155421git , but they can be avoided.

Manual Review

Individual user account

Yes: Follows

No duplication

Yes: Does not cause.

Master Branch

Yes: Follows. No Master Branch.

Licensing

Yes: Follows.

3rd party assets/code

Yes: Follows, no 3rd party code.

README.txt/README.md

No: Not follows the template https://www.drupal.org/node/2181737

Code long/complex enough for review

Yes: Follows

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

1. (+) Remove from .info, since JS and CSS initialized in ajax_page_load_page_build().

stylesheets[all][] = ajax_page_load.css
scripts[] = js/ajax_page_load.js

2. Add .make file and\or drush command to load nprogress.js JS library.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

Automated Review
Go through the issue mentioned in the below url http://pareview.sh/pareview/httpgitdrupalorgsandboxAlons2155421git
Manual Review
Individual user account
Yes: Follows
No duplication
Yes: Does not cause.
Master Branch
Yes: Follows. No Master Branch.
Licensing
Yes: Follows.
3rd party assets/code
Yes: Follows, no 3rd party code.
README.txt/README.md
No: Not follows the template https://www.drupal.org/node/2181737
Secure code
[Yes: Meets the security requirements. / No: List of security issues identified.]
Coding style & Drupal API usage
1. Just a recommendation
It good if you implement hook_install() It is recommended to always implement hook_install(). Here you can find an example.
Also implement hook_help() its good to implement for a professional developer.

@Devaraj johnson

1. Just a recommendation
It good if you implement hook_install() It is recommended to always implement hook_install(). Here you can find an example.

I saw recommendations like this in several other modules reviews. Looks like you are not investigated this module code clearly. It doesn't need hook_install() implementation.
Please avoid giving unreasonable recommendations, or describe a reason or provide a link with description.

Also, as I see, README.txt was fixed before your comment.
(Katherine committed on 7/23/15 at 3:05pm, and your comment (Devaraj johnson) dated 7/23/2015 at 11:28pm).

So, either provide a reasons why you think README.txt doesn't match with template, or remove your negative comment about README.txt.

Automated Review

No errors found: http://pareview.sh/pareview/httpgitdrupalorgsandboxalons2155421git

Note that perfect adherence to Drupal Coding Standard is NOT a reason to block an application, except for total disregard of them. However, modules should follow them as closely as possible.

Manual Review

Individual user account

Yes: Follows

No duplication

Does not cause

Master Branch

Yes: Follows

Licensing

Yes: Follows

3rd party assets/code

Yes: Follows

README.txt/README.md

Yes: Follows

Code long/complex enough for review

Yes: Follows

Secure code

Yes: Meets the security requirements

Coding style & Drupal API usage

[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

1. (*) Major finding, needs work
2. (+) Release blocker
3. Just a recommendation
   ajax_page_load.module line 63 - Place title into t()

   $items['admin/config/user-interface/ajax-page-load'] = array(
       'title' => 'Configure "Ajax page load with progress bar"',
       'page callback' => 'drupal_get_form',
       'page arguments' => array('ajax_page_load_admin_form'),
       'access arguments' => array('administer Ajax Page Load settings'),
       'type' => MENU_NORMAL_ITEM,
     );
   

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

@smakisog, No need to wrap menu items title in t(), since Drupal do this for you.

Module looks good for after second review, but one thing need to be done before release:

1. (*) Mark active link with active class.
   Currently, when I click on some link in menu, module shows me the content of requested page but previous link keep staying active. This may confuse a user.
2. (+) This is not a release blocker, but I strongly recommend to add drush command and .make file to download and install Nprogress library.
   Check the attached file as starting point.
3. Expand the available settings for library as on https://github.com/rstacruz/nprogress (template, easing, parent) (Not a release blocker)
4. Add the ability to define custom .css file. (Not a release blocker)

Feature requests:

1. Developer API: ability for developers to customize settings depend on context.
2. Per page settings or per sections settings (probably integration with context)

Fix (*) and I think module could be marked as RTBC.

Hi

I've also reviewed your module.
I have followed the instructions to install the nprogress library and then tested it.

But I did not see the progress spinner at the top of the page, as well as the progress bar. :(

May be my contents are a bit less and the page is loading ast, but as a suggestion you may provivide some descrriptive text in the help sections instead of mentiong the same configuration links.
Also it seems that your module doesn't remove the Drupal variables it defines.
So, you may have a look on that too.
And I have seen some new issues logged by "http://pareview.sh/pareview/httpgitdrupalorgsandboxalons2155421git".
So, please fix these issues and good luck :).

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.