Javascript: Recursion, jQuery Error

Question

Working on a navigation menu script with jQuery. The script is being designed with recursion so that there is no hard coded limit to the number of levels the menu has.

I'll start with the code:

navigationMenu.prototype.reset = function ( ulElement, colorIndex, colors ) { //Color index should always be 1 when directly calling this function
var listItems = $(ulElement.children);
var numItems = listItems.length;
var targetWidth = (100 / numItems) + '%';

listItems.each( function ( x ) {
    var children = $(listItems[x].children);
    var xT = $(listItems[x]).prop('tagName');
    var subMenu = null;

    children.each( function ( y ) {
        var yT = $(children[y]).prop('tagName'); 

        if (yT == 'UL') {
            subMenu = $(children[y]);
        } else if (yT == 'A') {
            $(children[y]).css('background-color', colors[colorIndex-1]); //Offset by 1 to facilitate for 0 indexed arrays
            $(children[y]).hover( function () { //Set hover color to the opposite
               $(children[y]).css('background-color',colors[(3-colorIndex)-1]); //3-1 = 2 and 3-2 = 1, subtract 1 to facilitate 0 indexed arrays
            }, function() {
                $(children[y]).css('background-color',colors[colorIndex-1]); //3-1 = 2 and 3-2 = 1, subtract 1 to facilitate 0 indexed arrays
            }); //Rest of style SHOULD be handled by css (width 100%, text color, text align)
        }
    });

    if (subMenu !== null) { //Recurse
        navigationMenu.prototype.reset(subMenu, (3 - colorIndex), colors); //Not defined?
    }

    if (xT == 'LI') { //Format the element
        $(listItems[x]).css('width',targetWidth);
        $(listItems[x]).css('background-color', colors[colorIndex]);
    }
});
};

Next, The error:

Uncaught TypeError: Cannot read property 'firstChild' of null <-whitespace-> jquery-1.11.1.min.js:2

What concerns me is that the error does not seem to come directly from my code, rather, a function within the jQuery library; however, I'm placing good money on the fact that it is because of something I did wrong.

A live demo can be found here: http://proofoftheilluminati.com/test/test.html

For an idea of the final look of the menu you can see the top level with hover effect and a simple JS script that maths the link widths here: http://proofoftheilluminati.com/test/index.html

Script: http://proofoftheilluminati.com/test/scripts/menu.js

I'm hosting a freshly downloaded copy of jQuery version 1.11.1: http://proofoftheilluminati.com/test/scripts/jquery-1.11.1.min.js

What it should be doing: Top level list should be orange with black over effect second level list should be black with orange hover effect third level list should be same as first, etc.

Positioning is handled by external css file

What it is doing: Handles top level list correctly, seems to error before style second level list.

Please let me know if I left anything out. I try to be thorough.

Edit: The supplied code has a comment on the line that calls itself:

//Not defined?

This was left over from a previous error, I was having trouble getting it to recognize the recursive function call. I tried the following lines here and they would not allow the function to progress:

this.reset(subMenu, (3 - colorIndex), colors);
reset(subMenu, (3 - colorIndex), colors);
navigationMenu.reset(subMenu, (3 - colorIndex), colors);

Additionally, this function is called when the document is ready:

$(document).ready(function() {
    s = new navigationMenu('#NavMenu', '#884106', '#000000', -1);
});

Edit: modified code to use x/y instead of index and xT/yT instead of tag (removed nested variables with same name)

I felt odd using index twice. I also feel odd using different names for the same idea. — gNerb
– gNerb, Commented Nov 14, 2014 at 19:32
But you should use this instead of listItems[index] and children[index] — A. Wolff
– A. Wolff, Commented Nov 14, 2014 at 19:44
I don't intent to say your wrong or argue @A.Wolff but I feel that's a style choice. Tests show no difference in processing time/efficiency and using this doesn't resolve the issue. — gNerb
– gNerb, Commented Nov 14, 2014 at 19:51
One thing that is definitely a problem is the line navigationMenu.prototype.reset.call(me, subMenu, (3 - colorIndex), colors); in menu.js. That is calling the reset function with navigationMenu.prototype as this. You need to do var me=this as the first statement in reset() and change that line to me.reset(subMenu(3-colorIndex), colors). It doesn't fix the problem, but it's definitely wrong. — Ruan Mendes
– Ruan Mendes, Commented Nov 14, 2014 at 19:54

John S · Accepted Answer · 2014-11-14 20:05:09Z

1

When you first call navigationMenu.prototype.reset, I'm guessing ulElement is a DOM element, but when you call it recursively, you are passing it subMenu, which is a jQuery object. That will be a problem for the following line:

var listItems = $(ulElement.children);

Try changing the following line of code:

navigationMenu.prototype.reset(subMenu, (3 - colorIndex), colors);

To:

navigationMenu.prototype.reset(subMenu[0], (3 - colorIndex), colors);

I prefer to always prefix variables that refer to jQuery objects with "$" to keep them straight.

You could also use this inside the functions given to .each(). So instead of:

children.each(function(index) {
    var tag = $(children[index]).prop('tagName');

You could have:

children.each(function() {
    var $child = $(this),
        tag = $child.prop('tagName');

You could also consider using the jQuery .children() method, instead of the children DOM element property

edited Nov 14, 2014 at 20:05

answered Nov 14, 2014 at 19:46

John S

21.5k9 gold badges50 silver badges59 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

gNerb Over a year ago

interesting feedback, you are entirely correct in the difference, changing the first occurrence to a jQuery select causes the issue immediately rather then waiting for the second occurrence. As for the comment about "this," I don't prefer that method and my tests show there is no performance difference between the two.

gNerb Over a year ago

That's why I ask you guys. Your fix worked perfectly. Thanks!

Collectives™ on Stack Overflow

Javascript: Recursion, jQuery Error

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related