The module allows users to apply minimalist scrollbars to sections of content. It has been designed with ease-of-use in mind and allows designers to add scrollbars in just a few steps, without having to manually load plugin script files and css stylesheets. It is dependent on the Libraries API and loads two libraries. The mCustomScrollbar jQuery Plugin is the primary library, with the Mouse Wheel library available as an option.

Users can apply vertical scrollbars site-wide to blocks and nodes, or can specify CSS attributes to receive either vertical or horizontal scrollbars. Features include, a choice of 8 scrollbar themes, inertia/easing, autohide/show, touch scroll, dragger resize among others.

To my knowledge this is the first implementation of the mCustomScollbar jQuery plugin within a Drupal Module. In addition, a good deal of time has been spent exposing many of the features of this jQuery plugin to the user through Drupal's adminstration/configuration section, in order to make it availabe to novice developers/users.


Project Page:

https://drupal.org/sandbox/knooq/2087687

Git:

git clone http://git.drupal.org/sandbox/knooq/2087687.git neat_scrollbar


Reviews of other projects (1)

https://drupal.org/node/1924716#comment-7110488
https://drupal.org/node/1884178#comment-7017392
https://drupal.org/node/1923524#comment-7098122

Reviews of other projects (2)

https://drupal.org/node/2089507#comment-7863559
https://drupal.org/node/2081395#comment-7861657
https://drupal.org/node/2084559#comment-7862693


Comments

davidmac’s picture

Added "PAReview: review bonus" tag.

davidmac’s picture

Review of the 7.x-1.x branch:
http://pareview.sh/pareview/httpgitdrupalorgsandboxknooq2087687git
No current issues in relation to the pareview.sh/code sniff tests.

klausi’s picture

Assigned:Unassigned» cweagans
Status:Needs review» Reviewed & tested by the community
Issue tags:-PAReview: review bonus

manual review:

  1. neat_scrollbar_help(): do not split up translatable sentences in the middle.
  2. neat_scrollbar_preprocess_page(): do not use "jQuery(document).ready()", use Drupal behaviors instead. See https://drupal.org/node/756722#behaviors .
  3. all the checkboxes are missing a title on the settings page and look empty?

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.

Assigning to cweagans as he might have time to take a final look at this.

davidmac’s picture

Thanks @klausi.

Changes made.
re-ran pareview.sh/code sniff - no issues.
http://pareview.sh/pareview/httpgitdrupalorgsandboxknooq2087687git

davidmac’s picture

Issue summary:View changes

minor typing corrections

davidmac’s picture

Added PAReview: review bonus Tag.
please see 'Reviews of other projects (2)' above.

cweagans’s picture

Sorry for the delay in getting to this. Looking now.

cweagans’s picture

Assigned:cweagans» Unassigned
Status:Reviewed & tested by the community» Needs work
/**
* Implements hook_uninstall().
*/
function neat_scrollbar_uninstall() {
  // Remove persistent variables.
  variable_del('neat_scrollbar_mousewheel');
  variable_del('neat_scrollbar_theme');
  variable_del('neat_scrollbar_vertical_css');
  variable_del('neat_scrollbar_horizontal_css');
  variable_del('neat_scrollbar_content_node');
  variable_del('neat_scrollbar_content_page');
  variable_del('neat_scrollbar_default_stylesheet');
  variable_del('neat_scrollbar_switch');
  variable_del('neat_scrollbar_buttons');
  variable_del('neat_scrollbar_content_blocks');
  variable_del('neat_scrollbar_inertia');
  variable_del('neat_scrollbar_dragger');
  variable_del('neat_scrollbar_browser_resize');
  variable_del('neat_scrollbar_content_resize');
  variable_del('neat_scrollbar_touch');
  variable_del('neat_scrollbar_autoscroll');
  variable_del('neat_scrollbar_autohide');
  variable_del('neat_scrollbar_valid_vert_css');
  variable_del('neat_scrollbar_valid_horz_css');
}

This will not block your application, but I'd like to point out that exporting all this stuff is a PITA with Features. This would be much nicer if it were all combined into one variable (for instance, "neat_scrollbar_settings") that just contained an array with all the settings.

/**
* Implements hook_preprocess_page().
*/
function neat_scrollbar_preprocess_page() {

  // Retrieve the state of persistent variables.
  $content_nodes = variable_get('neat_scrollbar_content_node', 0);
  $content_blocks = variable_get('neat_scrollbar_content_blocks', 0);
  $valid_vert_css = variable_get('neat_scrollbar_valid_vert_css');
  $valid_horz_css = variable_get('neat_scrollbar_valid_horz_css');
  $on_off_switch = variable_get('neat_scrollbar_switch', 0);
  $scrollbar_theme = variable_get('neat_scrollbar_theme', 'dark');
  $stylesheet = variable_get('neat_scrollbar_default_stylesheet', 1);
  $inertia = variable_get('neat_scrollbar_inertia', 550);
  $button_status = variable_get('neat_scrollbar_buttons', 1);
  $dragger_status = variable_get('neat_scrollbar_dragger', 1);
  $mousewheel_status = variable_get('neat_scrollbar_mousewheel', 0);
  $touch_status = variable_get('neat_scrollbar_touch', 0);
  $browser_resize_status = variable_get('neat_scrollbar_browser_resize', 1);
  $content_resize_status = variable_get('neat_scrollbar_content_resize', 0);
  $autoscroll_status = variable_get('neat_scrollbar_autoscroll', 0);
  $autohide = variable_get('neat_scrollbar_autohide', 0);
  $current_path = current_path();

  // Return string for javascript use, based on status of persistent variables.
  $buttons = ($button_status) ? 'true' : 'false';
  $dragger = ($dragger_status) ? 'true' : 'false';
  $mousewheel = ($mousewheel_status) ? 'true' : 'false';
  $touch = ($touch_status) ? 'true' : 'false';
  $browser_resize = ($browser_resize_status) ? 'true' : 'false';
  $content_resize = ($content_resize_status) ? 'true' : 'false';
  $autoscroll = ($autoscroll_status) ? 'true' : 'false';
  $hide = ($autohide) ? 'true' : 'false';

  // Load the primary library as defined with hook_libraries_info above.
  if (neat_scrollbar_loaded()) {
    libraries_load('neat_scrollbar');
  }

  // Check availability and load mousewheel plugin library, if required.
  if (!empty($mousewheel_status)) {
    neat_scrollbar_mousewheel_loaded();
  }
  if ((($library = libraries_load('mousewheel')) && !empty($library['loaded'])) && !empty($mousewheel_status)) {
    libraries_load('mousewheel');
  }

  // Only add JS & CSS if library exists and module's settings are switched on.
  if ((($library = libraries_load('neat_scrollbar')) && !empty($library['loaded'])) && empty($on_off_switch)) {

    // Load the Neat Scrollbar base stylesheet, for site-wide content options.
    if (!empty($stylesheet)) {
      drupal_add_css(drupal_get_path('module', 'neat_scrollbar') . '/css/neat_scrollbar_default.css', array('every_page' => TRUE));
    }

    // Add inline jQuery based on choices from options/settings (vertical).
    if (!path_is_admin($current_path) && (!empty($valid_vert_css) || (!empty($content_nodes) && empty($content_blocks)) || (empty($content_nodes) && !empty($content_blocks)) || (!empty($content_nodes) && !empty($content_blocks)))) {
      drupal_add_js('
      (function ($) {
        Drupal.behaviors.neatScrollbarVert = {
          attach: function (context, settings) { 
            $(".neat-scrollbar,.neat-scrollbar-blocks,' . $valid_vert_css . '").mCustomScrollbar({
              theme: "' . $scrollbar_theme . '",
              scrollButtons:{enable:' . $buttons . '},
              horizontalScroll: false,
              mouseWheel: ' . $mousewheel . ',
              scrollInertia: ' . $inertia . ',
              autoDraggerLength: ' . $dragger . ',
              contentTouchScroll: ' . $touch . ',
              autoHideScrollbar: ' . $hide . ',
              advanced:{
              updateOnContentResize: ' . $content_resize . ',
              updateOnBrowserResize: ' . $browser_resize . ',
              autoScrollOnFocus: ' . $autoscroll . ',
             }
            });
          }
        };
      })(jQuery);
      ', array('type' => 'inline', 'scope' => 'footer', 'weight' => 50)
      );
    }
    // Add inline jQuery based on choices from options/settings (horizontal).
    if (!path_is_admin($current_path) && !empty($valid_horz_css)) {
      drupal_add_js('
      (function ($) {
        Drupal.behaviors.neatScrollbarHorz = {
          attach: function (context, settings) { 
            $("' . $valid_horz_css . '").mCustomScrollbar({
              theme: "' . $scrollbar_theme . '",
              scrollButtons:{enable:' . $buttons . '},
              horizontalScroll: true,
              mouseWheel: ' . $mousewheel . ',
              scrollInertia: ' . $inertia . ',
              autoDraggerLength: ' . $dragger . ',
              contentTouchScroll: ' . $touch . ',
              autoHideScrollbar: ' . $hide . ',
              advanced:{
              updateOnContentResize: ' . $content_resize . ',
              updateOnBrowserResize: ' . $browser_resize . ',
              autoScrollOnFocus: ' . $autoscroll . ',
             }
            });
          }
        };
      })(jQuery);
      ', array('type' => 'inline', 'scope' => 'footer', 'weight' => 50)
      );
    }
  }
}

Please move this into a separate Javascript file, use #attached instead of drupal_add_js(), and then use Drupal.settings to pass the information to your script.

Other than that, this looks good. Back to CNW for the second issue. I'll keep an eye on the issue. Please update this issue when you've fixed that issue, and I'll come back and take another look.

cweagans’s picture

Sorry, let me clarify about #attached: This is for your separate Javascript file as well as for your drupal_add_css() call.

Also, you can use #attached for loading libraries: https://drupal.org/node/1831222#comment-6841028, so you don't have to do hardly any work in hook_preprocess_page().

davidmac’s picture

Status:Needs work» Needs review

Thank you for your advice.

I have reworked this module to address the following:
Implement Drupal.settings, via #attached and pass PHP variables to scripts that way.
Use #attached to load the main Library as opposed to libraries_load().
Use #attached to load css & js files as opposed to drupal_add_css() & drupal_add_js().
Reduced the number of persistent variables in the Form and .install file from 19 to 4 in order to make it more manageable with modules such as Features.

cweagans’s picture

Status:Needs review» Fixed

Thanks for your contribution, davidmac!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

davidmac’s picture

many thanks @cweagans, @klausi.

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary:View changes

Inserted second round of 'Reviews of Other Projects'