JavaScript image toggling click

Question

I working on image toggling, first click should hide the image and second click should display the image again. I got some thing working, but the issue is, on first click it is doing nothing and on second click toggling gets started. Hope there is some thing wrong in my code. Please advise.

<img src="img/1.jpg" width="449" height="600" class="one" id="one" style="opacity=1">
<img src="img/2.jpg" width="450" height="600" class="two">

function toggle(obj) {
    var el = document.getElementById("one");
    
    if ( el.style.opacity != 0 ) {
        el.style.opacity = 0;
    }
    else {
        el.style.opacity = 1;
    }
}

one.addEventListener("click", toggle, false);

Using opacity to show hide elements? Why not just use visibility? — epascarello
– epascarello, Commented Dec 15, 2010 at 16:08
@epascarello when the element is invisible you can't click it again. — user447356
– user447356, Commented Dec 16, 2010 at 13:17

Community · Accepted Answer · 2017-05-23 12:26:55Z

5

style="opacity=1" should be style="opacity:1". The rule, as you've written it, is invalid and will be ignored, so the first click on the element will set the opacity to 1 (which is the default anyway).

nb, you could refactor your function to look like this:

function toggle(obj) {
    var el = document.getElementById("one");

    el.style.opacity = +!+el.style.opacity;
    // or, using bitwise XOR assignment to keep @Raynos happy
    // el.style.opacity ^= 1;
}

Working demo: http://jsfiddle.net/AndyE/se29y/

This converts the current value from a string, e.g. "1" to a number, e.g. 1, then negates the "truthiness" of that number, !1 === false, then converts the resulting boolean back to a number again before it is assigned to opacity. This means each click toggles the value to its opposite.

Of course, as @casablanca's answer points out, visibility is more appropriate (and more widely supported) for in-place hiding of elements, but a hidden element cannot be clicked to be shown again (thanks @Shadow Wizard).

edited May 23, 2017 at 12:26

CommunityBot

11 silver badge

answered Dec 15, 2010 at 16:05

Andy E

346k86 gold badges482 silver badges452 bronze badges

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

4 Comments

Andy E Over a year ago

@Raynos: no! JSLint will hurt my feelings if I do.

user447356 Over a year ago

visibility is more appropriate but as the requirement is clicking the element itself it can't be used: hidden element can't be clicked, I just checked it out now.

Andy E Over a year ago

@Shadow Wizard: that's something I was unsure of myself. Thanks for clarifying and doing something that I was far, far too lazy to do myself.

user447356 Over a year ago

sure, guess that's the reason the OP here used opacity in the first place. :)

casablanca · Accepted Answer · 2010-12-15 16:06:08Z

3

Don't use opacity to show/hide elements -- use visibility instead with the values visible or hidden.

answered Dec 15, 2010 at 16:06

casablanca

70.9k8 gold badges139 silver badges155 bronze badges

2 Comments

karim79 Over a year ago

Argh, I was just typing this :)

bart Over a year ago

Or display: none if you want to completely remove it from the DOM. Otherwise other elements will still move aside for the hidden element.

Raynos · Accepted Answer · 2010-12-15 16:47:12Z

1

function toggle(obj) {
    var el = document.getElementById("one");

    el.style.opacity = el.style.opacity^1;
}

Because converting to integer twice in a single expression is stupid.

edited Dec 15, 2010 at 16:47

answered Dec 15, 2010 at 16:29

Raynos

170k57 gold badges359 silver badges399 bronze badges

2 Comments

Andy E Over a year ago

Douglas Crockford is going to show up at your house unannounced, and beat you to death with a sign that says 'Problem at line 4 character 40: Unexpected use of '^'.'

Raynos Over a year ago

@AndyE Pedantic people use a logical negation and two arithmetic addition to EMULATE a binary XOR just to satisfy crockford

user447356 · Accepted Answer · 2010-12-16 13:53:07Z

Unfortunately and unsurprisingly, each browser has its own implementation of opacity.. so here is cross browser (IE, Chrome, Firefox) version of the original code:

function Toggle(element) {
    var curOpacity;
    if (typeof element.style.filter != "undefined") {
        curOpacity = parseInt(element.style.filter.split("=")[1]);
        if (isNaN(curOpacity))
            curOpacity = 100;
        element.style.filter = "alpha (opacity=" + (100 - curOpacity) + ")";
    }
    else {
        curOpacity = element.style.opacity || 1;
        element.style.opacity = +!+curOpacity;
    }
}

To apply this on all images at page load:

window.onload = function() {
    var images = document.getElementsByTagName("img");
    for (var i = 0; i < images.length; i++)
        images[i].onclick = function() { Toggle(this); };
}

Working test case with cute cats: http://jsfiddle.net/2sXVQ/2/ :)

Collectives™ on Stack Overflow

JavaScript image toggling click

4 Answers 4

4 Comments

2 Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

4 Comments

2 Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related