[D7] Media Preview Slider

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

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.

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:

README.txt or README.md is missing, see the guidelines for in-project documentation.

@bcwald
There are some errors and warning still in the module which should be resolved.
http://pareview.sh/pareview/httpgitdrupalorgsandboxbrian142222597git
Please resolve these.

Looks like its just README.md having a few lines more than 80 char. Kind of an annoying requirement but I'd suggest adding in some breaks there to make the bot and minions of bots happy. :)

README.md
1 | WARNING | Line exceeds 80 characters; contains 144 characters
13 | WARNING | Line exceeds 80 characters; contains 219 characters

Git clone command is incorrect, as it includes your username.

description = "Provides UX imporvement by adding a jqueryUI slider for preview media library for easier image selection.</em>"

- improvement
- jQuery UI
- HTML in the module description?!

scripts[]      = js/slider.js

Surplus whitespace.

/**
 * Implements hook_init().
 */
function media_preview_slider_init() {
  drupal_add_library('system', 'ui.slider');
}
/**

Need a blank line between two functions.

Also, why are you loading this on every page? What's the point?

Also, consider using Libraries API for this instead.

(function ($) {
    Drupal.behaviors.media_preview_sizer = {
        attach: function (context, settings) {
            //add in slider support for image sizing in the library
            //find the exposed filters and add the slider inline with them

Indentation should be 2 spaces in JS too, AFAIK. Also, comments should have capitals and full stops and a space after the //. Same applies to comments in the CSS.

Re: loading this on every page? What's the point?

Is it possible to lazy load this only when the Media Library view is loaded? It seems one might have to patch Media module to make that work. This might be why it is included on every page.

Nice module page.
As mentioned by joachim, please correct your git url to http://git.drupal.org/sandbox/Brian14/2222597.git.
And using Libraries API would be the better approach, I think.

Mandar

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.

Still looking to get this approved. What else needs to happen?

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. http://pareview.sh/pareview/httpgitdrupalorgsandboxbrian142222597git reported number of issues that need to be address.

Manual Review

README.txt/README.md
(*) No: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review
No: Follows the guidelines for project length and complexity. Added PAReview: Single project promote tag.

hook_help() is missing in your module.

The starred items (*) are fairly big issues and warrant going back to Needs Work.
Great work so far.

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.

Re-opening on behalf of the maintainer. We will look into getting these last things fixed.

Now passing Automated review. http://pareview.sh/pareview/httpgitdrupalorgsandboxbrian142222597git

> Is it possible to lazy load this only when the Media Library view is loaded? It seems one might have to patch Media module to make that work. This might be why it is included on every page.

If you mean a view as produced by Views module, then use hook_views_pre_render().

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.