I want to amalgamate these two functions into one, Any ideas?

  function toggleVisibility() {

        document.getElementById('regStart').style.visibility = 'hidden';
        document.getElementById('regPage').style.visibility = 'visible';

        document.getElementById('facebook').src = "/images/js project/FacebookGray.png"
        document.getElementById('google').src = "/images/js project/GoogleGray.png"
        document.getElementById('winlive').src = "/images/js project/WinLiveGray.png"
        document.getElementById('yahoo').src = "/images/js project/YahooGray.png"

function toggleSave() {

    document.getElementById('regStart').style.visibility = 'visible';
    document.getElementById('regPage').style.visibility = 'hidden';

    document.getElementById('facebook').src = "/images/js project/FacebookDepressed.ico"
    document.getElementById('google').src = "/images/js project/GoogleDepressed.ico"
    document.getElementById('winlive').src = "/images/js project/WinLiveDepressed.ico"
    document.getElementById('yahoo').src = "/images/js project/YahooDepressed.ico"

  • \$\begingroup\$ The functions are poorly named. A toggle can switch between two states in either direction, which neither does. Is that the purpose of merging, to make them into a genuine toggle? If so, why is one called ToggleVisibility and the other ToggleSave? All it seems to do is toggle visibility of two different sets of elements. \$\endgroup\$
    – itsbruce
    Feb 16, 2015 at 3:47
  • \$\begingroup\$ You're absolutely right, basically when I made toggleVisibility I was intending on it being a true toggle, but I'm super new to JS and couldn't figure out how to formulate the if condition effectively. My first instinct was to set the condition based on the current state of regStart but I couldn't really find anything that explained the syntax of how to do that. I tried some other things but i think most of them were pretty.... stupid. Anyways, I am in fact hoping to turn these into a single toggle, any ideas? \$\endgroup\$
    – dave
    Feb 16, 2015 at 3:53

1 Answer 1

function toggleVisibility() {
    var temp = document.getElementById('regStart').style.visibility
    document.getElementById('regStart').style.visibility = document.getElementById('regPage').style.visibility;
    document.getElementById('regPage').style.visibility = temp;

    var suffix = "Gray.png";
    if ( 'hidden' === temp ) {
        suffix = "Depressed.ico";

    document.getElementById('facebook').src = "/images/js project/Facebook" + suffix;
    document.getElementById('google').src = "/images/js project/Google" + suffix;
    document.getElementById('winlive').src = "/images/js project/WinLive" + suffix;
    document.getElementById('yahoo').src = "/images/js project/Yahoo" + suffix;

The first insight is that you want to swap the visibility settings for regStart and regPage. So use the normal swap format with a temp variable.

The second insight is that your image paths have the same start for both cases of each image. So the beginning can be a constant value for each image.

The third insight is that your image paths have the same ending for all images in a particular case. So based on the value of temp (arbitrarily chosen as the shortest variable name), change the ending.

I also normalized the indentation. It was inconsistent in your original functions.


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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