Losing reference to a jQuery stored select element

Question

I am somehow losing reference to a select element on the page. I'm working with legacy code and I'm trying to rebuild a portion of it. There is other code besides the code I am posting here that changes the value of the select element I am trying to work with.

I create a new object for each visualizer div element on the page. I select all of the select elements with class master_select and store them in masterSelects like this:

masterSelects = wrapper.find( ".master_select" );

Then whenever any select box on the page changes I call updatePreview and it should be able to make changes to the masterSelects.

When the page is first loaded, updatePreview is manually fired and I call:

masterSelects.css('background-color', 'blue')

which works and changes the bg-color of these select boxes as would be expected. However, if I change a select option and updatePreview is called by the event, using this:

masterSelects.css('background-color', 'orange');

fails to update the bg-color. I printed out the masterSelects object to the console log and it still contains the information I would expect. It's like the elements I want to change on the page are no longer the ones I have saved a reference to. However, trying to re-select it also doesn't work. Could the other legacy code somehow be breaking my reference to these elements despite the console log printing out the expected information? I'm sorry I haven't posted that code, it's a 1000 lines and is a mess since I didn't write it, I'm going to start weeding through it, but I wanted to know if this is even possible before I start.

var Visualizer = (function() {
    /*********************************
     * Internal Variables
     *********************************/
    var wrapper,
        selects,
        masterSelects;

    /*********************************
     * Constructor
     *********************************/
    function Visualizer( newElement ) {
        wrapper = $(newElement);
        selects = wrapper.find( "select" );
        masterSelects = wrapper.find( ".master_select" );
    };

    /*********************************
     * Internal Methods
     *********************************/
    var updatePreview = function( isManual ) {
        // Default isManual to false if it is not already true
        if (isManual != true) isManual = false;

        if(isManual) {
            masterSelects.css('background-color', 'blue');
            console.log(masterSelects);
        }

        if(!isManual) {
            // Even tried reselecting the masterSelects, but it didn't work
            // masterSelects = wrapper.find( ".master_select" );
            masterSelects.css('background-color', 'orange');
            console.log(masterSelects);
        }
    };

    // Bind on change events to the select boxes inside the wrapper div
    var bindEvents = function() {
        selects.change( updatePreview );
    };

    /*********************************
     * Public Methods
     *********************************/
    // Initialize the Visualizer object
    Visualizer.prototype.init = function() {
        bindEvents();
        updatePreview( true );
    };

    // Return the Visualizer object
    return Visualizer;
})();

/*********************************
 * Usage
 *********************************/
$( document ).ready( function() {
    $( "div.visualizer-wrapper" ).each( function( key, value ) {
        var vis = new Visualizer( value );
        vis.init();
    });
});

Oh and this code worked on another similar page that has slightly different other legacy code.

When you say _" whenever any select box on the page changes"_ what do you mean exactly? meaning when the user selects a different option in the select dropdown? Please explain this better. — CodeGodie, Mar 03 '15 at 16:01
@CodeGodie Yeah that's exactly what I mean. Look at the bindEvents function. — sage88, Mar 03 '15 at 16:02
At the point where you execute `masterSelects.css('background-color', 'orange');`, try `$(".master_select").css('background-color', 'orange');` instead. Does it work? — Roamer-1888, Mar 03 '15 at 16:06
Uh, that code is horrible. All those variables are static("global"), while they should be instance properties. No wonder why it doesn't work with multiple instances. — Bergi, Mar 03 '15 at 16:07
@Bergi, I agree. Its probably the cause why it conflicts with the external functions — CodeGodie, Mar 03 '15 at 16:09
@Bergi that may be my problem this is my first time attemping object oriented javascript, can you point me in the direction of a better way to make helper functions that aren't global but that I can reference easily? If you give an answer toward that effect and it works I'd definitely accept it. — sage88, Mar 03 '15 at 16:12
As written `Visualizer` is a hybrid of namespace and constructor. The pattern won't work because `wrapper`, `selects` and `masterSelects` will be "shared" by each instance of `new Visualizer()`!! In other words, they will be overwritten. — Roamer-1888, Mar 03 '15 at 16:13
@Roamer-1888 I just looked at the last called instance on the page and you're definitely correct, it's just getting overwritten (wish I'd toggled that last instance open before...). I'm going to poke around and see if I can find a better way to make instance variables and functions. — sage88, Mar 03 '15 at 16:20
@sage88: Just don't use any "internal" stuff, put the values as instance properties (object members) and the methods on the prototype. If you really need to use them, place everything inside the constructor. (Maybe [this answer](http://stackoverflow.com/a/13418980/1048572) helps to get you started) — Bergi, Mar 03 '15 at 16:21
@Bergi yes, something like that should work. Anything that gives each instance its own private vars. — Roamer-1888, Mar 03 '15 at 16:23
@Bergi Thanks for the link to your other answer, I'll be working through it. — sage88, Mar 03 '15 at 16:31
@Bergi Think I could get you to check out the answer I posted below to see if I have the right idea now? — sage88, Mar 03 '15 at 16:44

sage88 · Accepted Answer · 2015-03-03T16:51:09.780

Thanks Bergi for the help. This is how I interpreted your advice and I'll post it as the answer for anyone else who comes along. Please let me know if I'm still doing something wrong here.

/*********************************
 * Constructor
 *********************************/
function Visualizer( newElement ) {
    /*********************************
     * Internal Variables
     *********************************/
    var wrapper,
        selects,
        masterSelects;

    wrapper = $(newElement);
    selects = wrapper.find( "select" );
    masterSelects = wrapper.find( ".master_select" );

    /*********************************
     * Internal Methods
     *********************************/
    this.updatePreview = function( isManual ) {
        // Default isManual to false if it is not already true
        if (isManual != true) isManual = false;

        if(isManual) {
            masterSelects.css('background-color', 'blue');
            console.log(masterSelects);
        }

        if(!isManual) {
            masterSelects = wrapper.find( ".master_select" );
            masterSelects.css('background', 'transparent');
            masterSelects.css('background-color', 'orange');
            //$(".master_select").css('background-color', 'orange');
            console.log(masterSelects);
        }
    };

    // Bind on change events to the select boxes inside the wrapper div
    this.bindEvents = function() {
        selects.change( this.updatePreview.bind(this) );
    };
}

/*********************************
 * Public Methods
 *********************************/
// Initialize the Visualizer object
Visualizer.prototype.init = function() {
    this.bindEvents();
    this.updatePreview( true );
}

/*********************************
 * Usage
 *********************************/
$( document ).ready( function() {
    $( "div.visualizer-wrapper" ).each( function( key, value ) {
        var vis = new Visualizer( value );
        vis.init();
    });
});

That seems fine. Only [one minor obstacle](https://stackoverflow.com/questions/20279484/how-to-access-the-correct-this-context-inside-a-callback): You should use `selects.change( this.updatePreview.bind(this) )` in case you're going to the `this` keyword (e.g. for a method call) inside the `updatePreview` method. — Bergi, Mar 03 '15 at 16:48
@Bergi You're a mind reader, that was the exact problem I'd run into before. Thanks again for the help! I'm going to edit the answer to reflect that change. — sage88, Mar 03 '15 at 16:50

score 0 · Answer 2 · answered Mar 03 '15 at 17:21

This would work well phrased as a jQuery pugin.

(function($){
    // **********************************
    // ***** Start: Private Members *****
    var pluginName = 'visualizer';
    // ***** Fin: Private Members *****
    // ********************************

    // *********************************
    // ***** Start: Public Methods *****
    var methods = {
        init : function(options) {
            //"this" is a jquery object on which this plugin has been invoked.
            return this.each(function(index) {
                var $this = $(this);
                var data = $this.data(pluginName);
                // If the plugin hasn't been initialized yet
                if (!data) {
                    var settings = {
                        init_bgColor: 'blue',
                        alt_bgColor: 'orange'
                    };
                    if(options) { $.extend(true, settings, options); }
                    var masterSelects = $this.find( ".master_select" );
                    $this.find( "select" ).on('change', function(event) {
                        if(event.originalEvent) {
                            masterSelects.css('background-color', settings.init_bgColor);
                        } else {
                            masterSelects.css('background-color', settings.alt_bgColor);
                        }
                    });
                    $this.data(pluginName, {
                        target : $this,
                        settings: settings,
                        masterSelects: masterSelects
                    });
                }
            });
        },
    };
    // ***** Fin: Public Methods *****
    // *******************************

    // *****************************
    // ***** Start: Supervisor *****
    $.fn[pluginName] = function( method ) {
        if ( methods[method] ) {
            return methods[method].apply( this, Array.prototype.slice.call( arguments, 1 ));
        } else if ( typeof method === 'object' || !method ) {
            return methods.init.apply( this, arguments );
        } else {
            $.error( 'Method ' + method + ' does not exist in jQuery.' + pluginName );
        }
    };
    // ***** Fin: Supervisor *****
    // ***************************
})( jQuery );

This is more long-winded that strictly necessary but I've left in all the hooks for additional plugin methods should they be required.

Usage :

$( document ).ready( function() {
    $( "div.visualizer-wrapper" ).visualizer();
});

Or, to override the default color scheme :

$( document ).ready( function() {
    $( "div.visualizer-wrapper" ).visualizer({
        init_bgColor: 'green',
        alt_bgColor: 'red'
    });
});

Losing reference to a jQuery stored select element

2 Answers2