0

I'm having problem with updating my App object properties value! I'm trying to update the 'visible' property from false to true whenever I'm calling methods within my App object!

Now I can't figure out why I can't update the values!!! Could somebody please help me?

Here is the code;

var app = app || {};

app.layout = {

    // SIDEBAR
    'sidebar': function(a) {
        var options = {
            'settings': {
                'object': $('.sidebar-left'),
                'visible': false,
                'type': 'wide'
            },
            'open': function() {
                if(options.isActive() === false) {
                    options.settings.visible = true;
                    options.settings.object.addClass('active');
                }
            },
            'close': function() {
                if(options.isActive() === true) {
                    options.settings.visible = false;
                    options.settings.object.removeClass('active');

                }
            },
            'isActive': function() {
                if(options.settings.visible === true) {
                    return true;
                } else {
                    return false;
                }
            }
        }
        return options[a];
    }

    // HEADER
    // CONTENT
    // FOOTER
}

The idea behind this little App API is that I don't need to go and manually check whether a container is visible and change the UI manually, I could just call the app.layout methods and that should do the job...

As you can see, I'm trying to update the 'visible' property value when the two methods of app.layout.sidebar('open')(); and app.layout.sidebar('close')(); are called!

Thank you


Here is the updated version:

http://jsfiddle.net/Farzad/ndb1490v/3/

Hope it helps those that are looking to integrate this to their app, as it makes it really easy to keep track of your UI without needing you to manually check every time...

However if anybody knows any better approach then make an update to the jsFiddle version and link back in the comments section :)

Thank you guys, specially those who being helpful Cheers

ZAD
  • 569
  • 2
  • 7
  • 23
  • Are you getting some error in console? – Rajesh Nov 03 '15 at 13:58
  • 2
    Every time you call `app.layout.sidebar()` you're creating a brand new "options" object, in which the "visible" property is initialized to `false`. – Pointy Nov 03 '15 at 14:00
  • 2
    Your `options`object is redefined each time you call the `sidebar`function ; hence you reset the values. http://stackoverflow.com/questions/500431/what-is-the-scope-of-variables-in-javascript may help – pistou Nov 03 '15 at 14:00
  • 1
    Side note: You're doing a lot of `===` comparisons with booleans. That's almost never necessary. For instance, unless you're going to store somethign other than a boolean in `options.settings.visible`, then your entire `isActive` could more readably be written: `isActive: function() { return options.settings.visible; }`. Similarly, since `isActive` only ever returns `true` or `false`, you don't need `if(options.isActive() === true)`, just use `if(options.isActive())` Similarly, `if(options.isActive() === false)` becomes `if(!options.isActive())`. – T.J. Crowder Nov 03 '15 at 14:01
  • No, no error in console... I'm also able to change the value in the console, but nothing changes! – ZAD Nov 03 '15 at 14:01
  • @T.J.Crowder Wow, thanks for the explanation, I'll give that a go... thanks – ZAD Nov 03 '15 at 14:04

1 Answers1

4

Your code is recreating the options object every time sidebar is called, with false as the hardcoded value of visible:

app.layout = {

    // SIDEBAR
    'sidebar': function(a) {
        var options = {
            'settings': {
                'object': $('.sidebar-left'),
                'visible': false,                 // <=== Note
                'type': 'wide'
            },
            // ...
        return options[a];
    }
}

So any call you make like this:

app.layout.sidebar('open')();

...will always see options.settings.visible as false.

I can't see any reason for the complexity in your layout object, and so can't really recommend an appropriate change, other than to make sure options is not recreated each time.

If you're happy to use this kind of call instead:

app.layout.sidebar.open();

...then you can make things much less complicated:

var app = app || {};

app.layout = (function() {
    var options = {
        'settings': {
            'object': $('.sidebar-left'),
            'visible': false,
            'type': 'wide'
        }
    };
    return {
        sidebar: {
            'open': function() {
                if(options.isActive() === false) {
                    options.settings.visible = true;
                    options.settings.object.addClass('active');
                }
            },
            'close': function() {
                if(options.isActive() === true) {
                    options.settings.visible = false;
                    options.settings.object.removeClass('active');

                }
            },
            'isActive': function() {
                if(options.settings.visible === true) {
                    return true;
                } else {
                    return false;
                }
            }
        }
        // HEADER, e.g.
        ,
        header: {
            // ...
        }
        // CONTENT
        // FOOTER
    };
})();

Note that you need to make sure that code runs after the .sidebar-left element exists.


Side note: The quotes you've used in the options object property keys are all optional, you could just leave them off:

var options = {
    settings: {
        object: $('.sidebar-left'),
        visible: false,
        type: 'wide'
    }
};

Same goes for the ones defining sidebar. The only time you need quotes around property names in JavaScript is if the name is not a invalid IdentifierName (for instance, has spaces in it, or starts with a digit but isn't a number).

T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • Yes, absolutely... thank you so much :) Very helpful, massive +1 – ZAD Nov 03 '15 at 14:38
  • Here is a jsFiddle version, http://jsfiddle.net/Farzad/ndb1490v/3/, one thing that I don't love about it yet is that I'm repeating myself for sidebarLeft and sidebarRight, they are both the same but with different classes... now my question is that can I somehow make this better without repeating my codes... thanks – ZAD Nov 04 '15 at 11:45
  • 1
    @FarzadCyrus: That's a different question, I suggest posting it. :-) Put the example *in* the question using Stack Snippets (the `<>` button), say you're not happy about how you've had to repeat things, and ask how not to repeat yourself. – T.J. Crowder Nov 04 '15 at 11:50