Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I had a jQuery program that displays some hover-over messages when touching an icon, in order to make it disappear the user needs to click again in the icon.

am.homepage.Index.prototype.touchableTitleBinding = function () {
    var that = this;
    $('.' + this.bindingClassForTouchableTitle).on('touchend', function (e) {
        e.stopPropagation();
        $('.' + that.toggleClassForTouchableTitle).not(this).toggleClass(that.toggleClassForTouchableTitle);
        $(this).toggleClass(that.toggleClassForTouchableTitle);
    });
}

The variables are passed to the .js file from an mvc view. and the touchableBinding funciton is called on dom load.

Thing is that now I'd like to make it disappear whenever you click outside any of those icons, so I wrote this:

am.homepage.Index.prototype.hideTouchableTitleBinding = function () {
    var that = this,
        classForTouchabletitle = '.' + this.bindingClassForTouchableTitle;

    am.personExpenditure.Index.prototype.hideTouchableTitleBinding = function () {
        var that = this,
            classForTouchabletitle = '.' + this.bindingClassForTouchableTitle;

        $('*:not(' + classForTouchabletitle + ')').on('touchend', function (e) {
        e.stopPropagation();
        $('.' + that.toggleClassForTouchableTitle).removeClass(that.toggleClassForTouchableTitle);
    });
}

Is it too heavy to select all the elements apart from the ones with toggleClassForTouchableTitle? Is it worth binding like this?

am.homepage.Index.prototype.hideTouchableTitleBinding = function() {
    var that = this,
        classForTouchabletitle = '.' + this.bindingClassForTouchableTitle;

    $(body).on('touchend', '*:not(' + classForTouchabletitle + ')', function(e) {
        e.stopPropagation();
        $('.' + that.toggleClassForTouchableTitle).removeClass(that.toggleClassForTouchableTitle);
    });
};

Is it another way of doing it more elegant/performant?

At the end I've changed to this

am.homepage.Index.prototype.touchableTitleBinding = function() {
    var that = this,
        removeTouchableTitleClass = function (e) {
            e.stopPropagation();
            $('.' + that.toggleClassForTouchableTitle).removeClass(that.toggleClassForTouchableTitle);
        };


    $('.' + this.bindingClassForTouchableTitle).on('touchend', function(e) {
        e.stopPropagation();
        $('.' + that.toggleClassForTouchableTitle).not(this).toggleClass(that.toggleClassForTouchableTitle);

        //If we've displayed the title for one icon we rebind to hide it when clicking outside
        //just once, if we've hide it we stop listening to touch other elements for dealing with this
        $('body').off('touchend', ':not(' + that.toggleClassForTouchableTitle + ')');
        if ($(this).hasClass(that.toggleClassForTouchableTitle)) {
            $(this).removeClass(that.toggleClassForTouchableTitle);
        } else {
            $(this).addClass(that.toggleClassForTouchableTitle);
            $('body').on('touchend', ':not(' + that.toggleClassForTouchableTitle + ')', removeTouchableTitleClass);
        }

    });
};

In which case only once after displaying the tile will listen for the touch in any other part of the screen. I think this is probably slightly less readable but more performant

share|improve this question

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.