MDL-35590 block_navigation: fix remaining issues

blocks/navigation/amd/src/ajax_response_renderer.js

@@ -27,34 +27,42 @@ define(['jquery'], function($) {
     // Mappings for the different types of nodes coming from the navigation.
     // Copied from lib/navigationlib.php navigation_node constants.
     var NODETYPE = {
-        // @type int Root node = 0
+        // @type int Root node = 0.
         ROOTNODE : 0,
-        // @type int System context = 1
+        // @type int System context = 1.
         SYSTEM : 1,
-        // @type int Course category = 10
+        // @type int Course category = 10.
         CATEGORY : 10,
-        // @type int MYCATEGORY = 11
+        // @type int MYCATEGORY = 11.
         MYCATEGORY : 11,
-        // @type int Course = 20
+        // @type int Course = 20.
         COURSE : 20,
-        // @type int Course section = 30
+        // @type int Course section = 30.
         SECTION : 30,
-        // @type int Activity (course module) = 40
+        // @type int Activity (course module) = 40.
         ACTIVITY : 40,
-        // @type int Resource (course module = 50
+        // @type int Resource (course module = 50.
         RESOURCE : 50,
-        // @type int Custom node (could be anything) = 60
+        // @type int Custom node (could be anything) = 60.
         CUSTOM : 60,
-        // @type int Setting = 70
+        // @type int Setting = 70.
         SETTING : 70,
-        // @type int site administration = 71
+        // @type int site administration = 71.
         SITEADMIN : 71,
-        // @type int User context = 80
+        // @type int User context = 80.
         USER : 80,
-        // @type int Container = 90
+        // @type int Container = 90.
         CONTAINER : 90
     };
 
+    /**
+     * Build DOM.
+     *
+     * @method buildDOM
+     * @param {Object} rootElement the root element of DOM.
+     * @param {object} nodes jquery object representing the nodes to be build.
+     * @return
+     */
     function buildDOM(rootElement, nodes) {
         var ul = $('<ul></ul>');
         ul.attr('role', 'group');
@@ -149,8 +157,7 @@ define(['jquery'], function($) {
 
     return {
         render: function(element, nodes) {
-            // The first element of the response is the existing node
-            // so we start with processing the children.
+            // The first element of the response is the existing node so we start with processing the children.
             if (nodes.children && nodes.children.length) {
                 buildDOM(element, nodes.children);
             } else {

blocks/navigation/amd/src/nav_loader.js

@@ -22,13 +22,19 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 define(['jquery', 'core/ajax', 'core/config', 'block_navigation/ajax_response_renderer'],
-        function($, ajax, config, renderer) {
+    function($, ajax, config, renderer) {
+        var URL = config.wwwroot + '/lib/ajax/getnavbranch.php';
 
-    var URL = config.wwwroot + '/lib/ajax/getnavbranch.php';
-
-    function getBlockInstanceId(element) {
-        return element.closest('[data-block]').attr('data-instanceid');
-    }
+        /**
+         * Get the block instance id.
+         *
+         * @function getBlockInstanceId
+         * @param element
+         * @returns {*}
+         */
+        function getBlockInstanceId(element) {
+            return element.closest('[data-block]').attr('data-instanceid');
+        }
 
     return {
         load: function(element) {

blocks/navigation/amd/src/navblock.js

@@ -14,7 +14,7 @@
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
 /**
- * Load the navtree javscript
+ * Load the navigation tree javascript.
  *
  * @module     block_navigation/navblock
  * @package    core

blocks/navigation/amd/src/site_admin_loader.js

@@ -33,7 +33,7 @@ define(['jquery', 'core/ajax', 'core/config', 'block_navigation/ajax_response_re
             var promise = $.Deferred();
             var data = {
                 type: SITE_ADMIN_NODE_TYPE,
-                sesskey: config.sesskey,
+                sesskey: config.sesskey
             };
             var settings = {
                 type: 'POST',

blocks/navigation/block_navigation.php

@@ -107,7 +107,6 @@ class block_navigation extends block_base {
      * Gets Javascript that may be required for navigation
      */
     function get_required_javascript() {
-        global $CFG;
         parent::get_required_javascript();
         $this->page->requires->string_for_js('viewallcourses', 'moodle');
         $this->page->requires->js_call_amd('block_navigation/navblock', 'init', array());
@@ -119,6 +118,7 @@ class block_navigation extends block_base {
      * @return object $this->content
      */
     function get_content() {
+        global $CFG;
         // First check if we have already generated, don't waste cycles
         if ($this->contentgenerated === true) {
             return $this->content;

blocks/navigation/renderer.php

@@ -182,7 +182,8 @@ class block_navigation_renderer extends plugin_renderer_base {
             // Create the structure.
             $content = html_writer::tag('p', $content, $divattr);
             if ($isexpandable) {
-                $content .= $this->navigation_node($item->children, array('role' => 'group'), $expansionlimit, $options, $depth+1);
+                $content .= $this->navigation_node($item->children, array('role' => 'group'), $expansionlimit,
+                    $options, $depth + 1);
             }
             if (!empty($item->preceedwithhr) && $item->preceedwithhr===true) {
                 $content = html_writer::empty_tag('hr') . $content;

blocks/settings/renderer.php

@@ -43,6 +43,14 @@ class block_settings_renderer extends plugin_renderer_base {
         return $content;
     }
 
+    /**
+     * Build the navigation node.
+     *
+     * @param navigation_node $node the navigation node object.
+     * @param array $attrs list of attributes.
+     * @param int $depth the depth, default to 1.
+     * @return string the navigation node code.
+     */
     protected function navigation_node(navigation_node $node, $attrs=array(), $depth = 1) {
         $items = $node->children;
 
@@ -73,7 +81,8 @@ class block_settings_renderer extends plugin_renderer_base {
             $liexpandable = array();
             if ($isbranch) {
                 $liclasses[] = 'contains_branch';
-                if (!$item->forceopen || (!$item->forceopen && $item->collapse) || ($item->children->count()==0  && $item->nodetype==navigation_node::NODETYPE_BRANCH)) {
+                if (!$item->forceopen || (!$item->forceopen && $item->collapse) || ($item->children->count() == 0
+                        && $item->nodetype == navigation_node::NODETYPE_BRANCH)) {
                     $liexpandable = array('aria-expanded' => 'false');
                 } else {
                     $liexpandable = array('aria-expanded' => 'true');
@@ -90,7 +99,7 @@ class block_settings_renderer extends plugin_renderer_base {
                 $liclasses[] = 'current_branch';
             }
             $nodetextid = 'label_' . $depth . '_' . $number;
-            $liattr = array('class' => join(' ',$liclasses), 'tabindex' => '-1', 'role' => 'treeitem') + $liexpandable;
+            $liattr = array('class' => join(' ', $liclasses), 'tabindex' => '-1', 'role' => 'treeitem') + $liexpandable;
             // class attribute on the div item which only contains the item content
             $divclasses = array('tree_item');
             if ($isbranch) {
@@ -105,7 +114,7 @@ class block_settings_renderer extends plugin_renderer_base {
             if (!empty($item->id)) {
                 $divattr['id'] = $item->id;
             }
-            $content = html_writer::tag('p', $content, $divattr) . $this->navigation_node($item, array(), $depth+1);
+            $content = html_writer::tag('p', $content, $divattr) . $this->navigation_node($item, array(), $depth + 1);
             if (!empty($item->preceedwithhr) && $item->preceedwithhr===true) {
                 $content = html_writer::empty_tag('hr') . $content;
             }

blocks/upgrade.txt

 This files describes API changes in /blocks/* - activity modules,
 information provided here is intended especially for developers.
 
+=== 3.1 ===
+
+* The collapsed class was removed from the navigation block to make it compatible with aria.
+* New aria attributes were added on the navigation block [aria-expanded="false"].
+* The tree JS handling were moved from YUI to AMD module (Jquery).
+
 === 2.9 ===
 
 * The obsolete method preferred_width() was removed (it was not doing anything)

lib/amd/src/tree.js

@@ -15,7 +15,7 @@
 
 /**
  * Implement an accessible aria tree widget, from a nested unordered list.
- * Based on http://oaa-accessibility.org/example/41/
+ * Based on http://oaa-accessibility.org/example/41/.
  *
  * @module     tool_lp/tree
  * @package    core
@@ -27,14 +27,15 @@ define(['jquery'], function($) {
     var SELECTORS = {
         ITEM: '[role=treeitem]',
         GROUP: '[role=treeitem]:has([role=group]), [role=treeitem][data-requires-ajax=true]',
-        CLOSED_GROUP: '[role=treeitem]:has([role=group])[aria-expanded=false], [role=treeitem][data-requires-ajax=true][aria-expanded=false]',
+        CLOSED_GROUP: '[role=treeitem]:has([role=group])[aria-expanded=false], [role=treeitem]' +
+                '[data-requires-ajax=true][aria-expanded=false]',
         FIRST_ITEM: '[role=treeitem]:first',
         VISIBLE_ITEM: '[role=treeitem]:visible',
         UNLOADED_AJAX_ITEM: '[role=treeitem][data-requires-ajax=true][data-loaded=false][aria-expanded=true]'
     };
 
     /**
-     * Constructor
+     * Constructor.
      *
      * @param {String} selector
      * @param {function} selectCallback Called when the active node is changed.
@@ -44,7 +45,6 @@ define(['jquery'], function($) {
 
         this.treeRoot.data('activeItem', null);
         this.selectCallback = selectCallback;
-
         this.keys = {
             tab:      9,
             enter:    13,
@@ -60,11 +60,9 @@ define(['jquery'], function($) {
             asterisk: 106
         };
 
-        // Apply the standard default initialisation for all nodes, starting
-        // with the tree root.
+        // Apply the standard default initialisation for all nodes, starting with the tree root.
         this.initialiseNodes(this.treeRoot);
-        // Make the first item the active item for the tree so that it is
-        // added to the tab order.
+        // Make the first item the active item for the tree so that it is added to the tab order.
         this.setActiveItem(this.treeRoot.find(SELECTORS.FIRST_ITEM));
         // Create the cache of the visible items.
         this.refreshVisibleItemsCache();
@@ -72,8 +70,6 @@ define(['jquery'], function($) {
         this.bindEventHandlers();
     };
 
-    // Public variables and functions.
-
     /**
      * Find all visible tree items and save a cache of them on the tree object.
      *
@@ -83,16 +79,20 @@ define(['jquery'], function($) {
         this.treeRoot.data('visibleItems', this.treeRoot.find(SELECTORS.VISIBLE_ITEM));
     };
 
+    /**
+     * Get all visible tree items.
+     *
+     * @method getVisibleItems
+     */
     Tree.prototype.getVisibleItems = function() {
         return this.treeRoot.data('visibleItems');
-    }
+    };
 
     /**
-     * Mark the given item as active within the tree and fire the callback for
-     * when the active item is set.
+     * Mark the given item as active within the tree and fire the callback for when the active item is set.
      *
      * @method setActiveItem
-     * @param {object} a jquery object representing an item on the tree.
+     * @param {object} item jquery object representing an item on the tree.
      */
     Tree.prototype.setActiveItem = function(item) {
         var currentActive = this.treeRoot.data('activeItem');
@@ -117,11 +117,10 @@ define(['jquery'], function($) {
     };
 
     /**
-     * Determines if the given item is a group item (contains child tree items) in
-     * the tree.
+     * Determines if the given item is a group item (contains child tree items) in the tree.
      *
      * @method isGroupItem
-     * @param {object} a jquery object representing an item on the tree.
+     * @param {object} item jquery object representing an item on the tree.
      * @returns {bool}
      */
     Tree.prototype.isGroupItem = function(item) {
@@ -134,14 +133,13 @@ define(['jquery'], function($) {
      * on items.
      *
      * @method initialiseNodes
-     * @param {object} a jquery object representing an item on the tree.
+     * @param {object} node jquery object representing a node.
      */
     Tree.prototype.initialiseNodes = function(node) {
         this.removeAllFromTabOrder(node);
         this.setAriaSelectedFalseOnItems(node);
 
-        // Get all ajax nodes that have been rendered as expanded but
-        // haven't loaded the child items yet.
+        // Get all ajax nodes that have been rendered as expanded but haven't loaded the child items yet.
         var thisTree = this;
         node.find(SELECTORS.UNLOADED_AJAX_ITEM).each(function() {
             var unloadedNode = $(this);
@@ -155,18 +153,17 @@ define(['jquery'], function($) {
      * Removes all child DOM elements of the given node from the tab order.
      *
      * @method removeAllFromTabOrder
-     * @param {object} a jquery object representing an item on the tree.
+     * @param {object} node jquery object representing a node.
      */
     Tree.prototype.removeAllFromTabOrder = function(node) {
         node.find('*').attr('tabindex', '-1');
     };
 
     /**
-     * Find all child tree items from the given node and set the aria selected
-     * attribute to false.
+     * Find all child tree items from the given node and set the aria selected attribute to false.
      *
      * @method setAriaSelectedFalseOnItems
-     * @param {object} a jquery object representing an item on the tree.
+     * @param {object} node jquery object representing a node.
      */
     Tree.prototype.setAriaSelectedFalseOnItems = function(node) {
         node.find(SELECTORS.ITEM).attr('aria-selected', 'false');
@@ -179,13 +176,13 @@ define(['jquery'], function($) {
      */
     Tree.prototype.expandAllGroups = function() {
         this.expandAllChildGroups(this.treeRoot);
-    }
+    };
 
     /**
      * Find all child group nodes from the given node and expand them.
      *
      * @method expandAllChildGroups
-     * @param {object} a jquery object representing an item on the tree.
+     * @param {object} node jquery object representing a node.
      */
     Tree.prototype.expandAllChildGroups = function(node) {
         var thisTree = this;
@@ -199,12 +196,13 @@ define(['jquery'], function($) {
     };
 
     /**
-     * Expand a collapsed group. Handles expanding nodes that are ajax loaded (marked
-     * with a data-requires-ajax attribute).
+     * Expand a collapsed group.
+     *
+     * Handles expanding nodes that are ajax loaded (marked with a data-requires-ajax attribute).
      *
      * @method expandGroup
-     * @param {Object} item is the jquery id of the parent item of the group
-     * @return {Object} a promise that is resolved when the group has been expanded
+     * @param {Object} item is the jquery id of the parent item of the group.
+     * @return {Object} a promise that is resolved when the group has been expanded.
      */
     Tree.prototype.expandGroup = function(item) {
         var promise = $.Deferred();
@@ -239,15 +237,14 @@ define(['jquery'], function($) {
         } else {
             promise.resolve();
         }
-
         return promise;
     };
 
     /**
-     * Performes the necessary DOM changes to display a group item.
+     * Perform the necessary DOM changes to display a group item.
      *
      * @method finishExpandingGroup
-     * @param {Object} item is the jquery id of the parent item of the group
+     * @param {Object} item is the jquery id of the parent item of the group.
      */
     Tree.prototype.finishExpandingGroup = function(item) {
         // Find the first child node.
@@ -266,7 +263,7 @@ define(['jquery'], function($) {
      * Collapse an expanded group.
      *
      * @method collapseGroup
-     * @param {Object} item is the jquery id of the parent item of the group
+     * @param {Object} item is the jquery id of the parent item of the group.
      */
     Tree.prototype.collapseGroup = function(item) {
         // If the item is already collapsed then do nothing.
@@ -274,11 +271,9 @@ define(['jquery'], function($) {
             return;
         }
 
+        // Get and collapse the group.
         var group = item.children(SELECTORS.GROUP);
-
-        // Collapse the group.
         group.hide().attr('aria-hidden', 'true');
-
         item.attr('aria-expanded', 'false');
 
         // Update the list of visible items.
@@ -289,7 +284,7 @@ define(['jquery'], function($) {
      * Expand or collapse a group.
      *
      * @method toggleGroup
-     * @param {Object} item is the jquery id of the parent item of the group
+     * @param {Object} item is the jquery id of the parent item of the group.
      */
     Tree.prototype.toggleGroup = function(item) {
         if (item.attr('aria-expanded') === 'true') {
@@ -303,7 +298,7 @@ define(['jquery'], function($) {
      * Handle a key down event - ie navigate the tree.
      *
      * @method handleKeyDown
-     * @param {Object} item is the jquery id of the parent item of the group
+     * @param {Object} item is the jquery id of the parent item of the group.
      * @param {Event} e The event.
      */
     Tree.prototype.handleKeyDown = function(item, e) {
@@ -394,21 +389,17 @@ define(['jquery'], function($) {
 
                     next.focus();
                 }
+
                 e.stopPropagation();
                 return false;
             }
             case this.keys.asterisk: {
                 // Expand all groups.
-
-                var thisObj = this;
-
                 this.expandAllGroups();
-
                 e.stopPropagation();
                 return false;
             }
         }
-
         return true;
     };
 
@@ -416,7 +407,7 @@ define(['jquery'], function($) {
      * Handle a click (select).
      *
      * @method handleClick
-     * @param {Object} item is the jquery id of the parent item of the group
+     * @param {Object} item The jquery id of the parent item of the group.
      * @param {Event} e The event.
      */
     Tree.prototype.handleClick = function(item, e) {
@@ -439,10 +430,10 @@ define(['jquery'], function($) {
     };
 
     /**
-     * Handle a focus event
+     * Handle a focus event.
      *
      * @method handleFocus
-     * @param {Object} item item is the jquery id of the parent item of the group
+     * @param {Object} item The jquery id of the parent item of the group.
      * @param {Event} e The event.
      */
     Tree.prototype.handleFocus = function(item, e) {