0
\$\begingroup\$

I'm making a web application for the administration of a restaurant kind of type. The purpose of this application is to make customers, and the orders of those customers, which has products in it, etc...

This application is written in PHP combined with jQuery.

I have many tabs, such as:

  • Displaying the orders
  • A screen to make a new order (same div as displaying the list from the orders)
  • A screen to show you all products, which you can click on to put them inside of a orders.
  • A screen where more order information appears whenever clicking on it.

On this moment, I'm using this kind of snippets to manage this:

if (orderListScreen.css('visibility', 'hidden')) {
    orderListScreen.css("visibility", "visible");
    newOrderScreen.css("visibility", "hidden");
}

if (availableProductsScreen.css("visibility", "hidden")) {
    availableProductsScreen.css("visibility", "hidden");
    orderInformationScreen.css("visibility", "visible");
}
  • Note: This is one block of code.
  • Note: the orderListScreen and all the other variables refer to different div elements.

Problem

  • This is very confusing to work with.
  • This takes me more time to actually think which one is active and which one is not.

My own try

I've made a small function which also switches between the visibility of the elements, but I still don't like it; it makes it too confusing.

function switchTab(activate, deactivate) {
    var _activate = $(activate);
    var _deactivate = $(deactivate);

    if (_activate.css("visibility", "hidden")) {
        _activate.css("visibility", "visible");
        _deactivate.css("visibility", "hidden");
    }
}
\$\endgroup\$
  • \$\begingroup\$ What do you expect that if statement to do? Are you sure that's what you want? It doesn't seem right... \$\endgroup\$ – elclanrs Aug 11 '14 at 23:47
  • \$\begingroup\$ @elclanrs Checking if it actually is hidden? But you're right, it probably isn't right. \$\endgroup\$ – Bas Aug 11 '14 at 23:51
  • \$\begingroup\$ I feel that this question is slightly off-topic, since it is more about looking for an alternative, rather then a review of the code. PS: to the OP: have you considered using jQ's $.toggle()? \$\endgroup\$ – Elias Van Ootegem Aug 12 '14 at 11:58
  • \$\begingroup\$ @EliasVanOotegem I was'nt ment to make you guys actually search for an alternative for the visibility it was just a try of my own. Wich didnt work out that well. Also, no i didnt, do you mean that that i'll give an .active class to the div that currently is active? \$\endgroup\$ – Bas Aug 12 '14 at 12:55
  • \$\begingroup\$ @Bas: No, with $.toggle you can do something like: var elem = $('#foo'); elem.toggle(); elem.toggle(); the first toggle call will hide the element (or show it, in case it was hidden), the second will do the opposite. It all depends on the element being visible or not \$\endgroup\$ – Elias Van Ootegem Aug 12 '14 at 13:36
1
\$\begingroup\$

The if statement doesn't do what you think it does.

Optional details about the .css() function:

The .css() function is overloaded in a number of ways. It can take one string, two strings, or an object.

In the first form, it returns the current value of the specified property. This will probably not be exactly as you specified it. For example, $("body").css("background-color", "red").css("background-color") returns "rgb(255, 0, 0)" for me.

In the second, it sets the specified property (first string) to the given value, and returns the jQuery object. In the third, it sets the specified properties (given as keys of the object) to their respective values. This form also returns the jQuery object, for more method chaining.

As you can see, if you're intent on using the .css() function to check a value, the first form is what you want.

A better way of checking visibility would be to use the .is() function in conjunction with the :visible selector.

Here's my revision. I took the liberty of renaming the parameters. I also used the common practice of naming jQuery variables using Hungarian notation.

function switchTab(toActivate, toDeactivate) {
    var $toActivate = $(toActivate);
    var $toDeactivate = $(toDeactivate);

    if (!$toActivate.is(":visible")) {
        $toActivate.css("visibility", "visible");
        $toDeactivate.css("visibility", "hidden");
    }
}

Another way:

If there's some common selector between the tabs (for example, if they all have the class .tab) and you're fine with display: none; instead of visibility: hidden;, you can simplify to the following:

function switchTab(toActivate) {
    $(".tab").hide();
    $(toActivate).show();
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for your time :) ill look at it tommorow when i get back on my computer \$\endgroup\$ – Bas Aug 12 '14 at 1:16
  • \$\begingroup\$ Thanks for the optimization on my own try, but i still dont like the idea with toggling the visibility's manually. Is there another way to do this? \$\endgroup\$ – Bas Aug 13 '14 at 14:00
  • \$\begingroup\$ @Bas I've listed an alternative in an edit. \$\endgroup\$ – Schism Aug 13 '14 at 14:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.