jQuery code returns 'Object expected' error

Question

I have the following jQuery code. It is writing two event listeners like this ok? The following code throws an.error

$(document).ready(function () {
    $('tr').hover(
        function () {
            $(this).css('background-color', '#FFFF99');
            $(this).contents('td').css({
                'border': '1px solid red', 
                'border-left': 'none', 
                'border-right': 'none' 
            });
            $(this).contents('td:first').css('border-left', '1px solid red');
            $(this).contents('td:last').css('border-right', '1px solid red');
        });

        $('#radioButtonImageSourceContainerId input:radio').click(function () {
            if ($(this).val() === 'fromFile') {
                addVisibility("from-file");
                removeVisibility("from-url");
            } 
            else if ($(this).val() === 'fromUrl') {
                removeVisibility("from-file");
                addVisibility("from-url");
            }
        })
    });

The other 2 functions are

function addVisibility(elemId) {
    document.getElementById(elemId).style.display = "";
    if (document.getElementById(elemId).style.visibility == "hidden") {
        document.getElementById(elemId).style.visibility = "visible";
    }
}

function removeVisibility(elemId) {
    document.getElementById(elemId).style.display = "";
    if (document.getElementById(elemId).style.visibility == "visible") {
        document.getElementById(elemId).style.visibility = "hidden";
    }
}

RightSaidFred · Accepted Answer · 2011-12-10 17:18:54Z

3

Check to make sure you found an element first.

function addVisibility(elemId) {
    var el = document.getElementById(elemId);
    if( el )
        el.style.display = "";
        if (el.style.visibility == "hidden") {
            el.style.visibility = "visible";
        }
    } else {
        console.log( 'NO ELEMENT WAS FOUND' );
    }
}

...and cache your DOM selection instead of repeating it.

Another issue is these lines:

$(this).contents('td').css({...
$(this).contents('td:first')...
$(this).contents('td:last')...

The contents() method returns text nodes as well. You should use find or children instead.

$(this).find('td').css({...
$(this).children('td').css({...

A better way to rewrite this part of it (with jQuery) would IMO be this:

$(document).ready(function () {
    $('tr').hover(function () {
        $(this).addClass( 'hilite_row' );
        var tds = $(this.cells).addClass( 'hilite_cell' );
        tds.first().addClass( 'hilite_left_cell' );
        tds.last().addClass( 'hilite_right_cell' );
    });

    $('#radioButtonImageSourceContainerId input:radio').click(function () {
        var value = $(this).val(),
            visib = ['hidden','visible'],
            file = visib[ +(value === 'fromFile') ],
            url = visib[ +(value === 'fromUrl') ];
        if( file === url ) return;
        $("#from-file").css('visibility', file);
        $("#from-url").css('visibility', url);
    })
});

Then define the style for the classes in your CSS.

Or even better, use the :hover pseudo-selector and do it all in CSS, though IE6 won't work.

edited Dec 10, 2011 at 17:18

answered Dec 10, 2011 at 16:41

RightSaidFred

11.3k38 silver badges36 bronze badges

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

4 Comments

Cᴏʀʏ Over a year ago

find() doesn't look at just the immediate children, it searches recursively. children() is the way to go here, it only goes one level down the DOM.

RightSaidFred Over a year ago

@Cory: Yes, unless there are nested tables that are meant to be found. Given OP's misuse of contents() (and lack of markup), I wasn't sure. But yes, most likely children is intended.

RightSaidFred Over a year ago

...that said, some people still prefer to use find(), because it can actually be faster when it is able to use getElementsByTagName. Ultimately it should be var tds = $(this).find('td')...; tds.first()...; tds.last()...; (Or the same using children. The point being to cache instead of reselect.)

RightSaidFred Over a year ago

...Or the absolute best way would be var tds = $(this.cells).

Cᴏʀʏ · Accepted Answer · 2011-12-10 16:50:09Z

Just a tip for you: your table row hover is doing a tiny bit of extra work with contents(). contents() looks for text nodes and comments which you can safely ignore; find() would work as well but it travels recursively through the DOM instead of just through immediate children. I would change it to children() instead:

$('tr').hover(function () {
    $(this).css('background-color', '#FFFF99');
    $(this).children('td').css({ 
        'border': '1px solid red', 
        'border-left': 'none', 
        'border-right': 'none' 
    });
    $(this).children('td:first').css('border-left', '1px solid red');
    $(this).children('td:last').css('border-right', '1px solid red');
 },
 function() {
     // mouseout - you were missing this
 });

Collectives™ on Stack Overflow

jQuery code returns 'Object expected' error

2 Answers 2

4 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related