-2

I have a jquery function where I repeat the .red way too many times.

I would like to replace them for $(this) instead, but I don't know the correct way to do it apparently.

$(".red").ready(function () {
    $(this).addClass("active");
});

http://jsfiddle.net/LGDz2/

This is a simplified version of the code and how I'd want it to behave, hope it makes sense to you guys.

The idea here is to be able to replace that initial .red and have it apply on all the lines of code I add inside the function.

Regards.

EDIT: I was trying to simplify it to the maximum because I use to get lost in complex explainations and because I'm kind of embarrased posting my crappy code lol, here is the code I want to optimize:

function showRed() {
    $(".red").show();
    if (position == 0) {
        $(".red").css({opacity : "0", left: "0"}).transition({opacity:1}, 500, ezin);
    } else if (position < 1){
        $(".red").css({left : "100%", });
    } else if (position > 1) {
        $(".red").css({left : "-100%", });
    }
    position = 1;
    goodbye();
    $(".red").css({scale: "1"}).transition({left:0}, 500, ezin).addClass("active");
}

function showGreen() {

    $(".green").ready(function(){
        $(".green").show();
        if (position == 0) {
            $(".green").css({opacity : "0", left: "0"}).transition({opacity:1}, 500, ezin);
        } else if (position < 2){
            $(".green").css({left : "100%", });
        } else if (position > 2) {
            $(".green").css({left : "-100%", });
        }
        position = 2;
        goodbye();
        $(".green").css({scale: "1"}).transition({left:0}, 500, ezin).addClass("active");
    });

}

What I want is to optimize all this so I don't have to write this code for every single slide. I'm using jQuery transit by the way, transition is pretty much like animate in here.

It's the same for showBlue too.

And this is how it works: http://dlacrem.16mb.com/dlatest/positions.html

Dlacrem
  • 71
  • 3
  • 10
  • 4
    What are you trying to accomplish? – tymeJV Apr 02 '14 at 15:19
  • So what are you asking here? – mituw16 Apr 02 '14 at 15:19
  • Elements don't have a `.ready()` event. Only the `document` does. What are you trying to do? When do you want to add the `active` class? – gen_Eric Apr 02 '14 at 15:19
  • Perhaps you should show us the code where you are using ".red" too many times and we can help you optimise it. Showing us code that doesn't do what you want or make sense isn't going to help us to help you. :) – Chris Apr 02 '14 at 15:22
  • This is a very simplified version of my code, the thing is that I have multiple colors, and each color has multiple things to do, addClass, animate, etc, what I'd want is to be able to change just one selector, and have it apply to all the animations and changes. – Dlacrem Apr 02 '14 at 15:23
  • @Dlacrem Maybe you could post a less simplified code which makes sense, then... – A. Wolff Apr 02 '14 at 15:27
  • 1
    You don't have to show *all* the code necessarily but as others have pointed out you shouldn't be calling `.ready` in this way so I assume your original code doesn't either. – Chris Apr 02 '14 at 15:31
  • Alright, posted the real code, hope it makes more sense for you this time. – Dlacrem Apr 02 '14 at 15:32
  • @Dlacrem: Definitely makes more sense. I've got an answer below. I wasn't sure how you were calling these functions so my answer covered both options I could think of. :) – Chris Apr 02 '14 at 15:40

5 Answers5

2

You need to use:

$(document).ready(function () { 
    $(".red").addClass("active");
});

Above code will add class active to your div with class red on page load.

The .ready() method can only be called on a jQuery object matching the current document

Felix
  • 37,892
  • 8
  • 43
  • 55
2

Looking at your code the first thing you would want to do as a step by step on how to do it yourself is refactor out all your uses of $(".green") into a local variable.

This would give:

function showGreen() {
    var element = $(".green");
    element.show();
    if (position == 0) {
        element.css({opacity : "0", left: "0"}).transition({opacity:1}, 500, ezin);
    } else if (position < 2){
        element.css({left : "100%", });
    } else if (position > 2) {
        element.css({left : "-100%", });
    }
    position = 2;
    goodbye();
    element.css({scale: "1"}).transition({left:0}, 500, ezin).addClass("active");
}

This is the same sort of refactoring that you might do when you would want to remove any kind of variable.

What you do after that depends on how you are calling these functions. Standard refactoring would then involve just promoting it to a function parameter and observing that at this stage showGreen and showRed would be the same (assuming there are no minor differences in properties I've not noticed).

function show(element) {
    element.show();
    if (position == 0) {
        element.css({opacity : "0", left: "0"}).transition({opacity:1}, 500, ezin);
    } else if (position < 2){
        element.css({left : "100%", });
    } else if (position > 2) {
        element.css({left : "-100%", });
    }
    position = 2;
    goodbye();
    element.css({scale: "1"}).transition({left:0}, 500, ezin).addClass("active");
}

you could then call this as show($('.red')) or show($('.green')).

Alternatively if you want to call it more like:

$('.red').click(showRed);

Then you could change the function to start

var element = $(this)

and that should do what you want.

Chris
  • 27,210
  • 6
  • 71
  • 92
  • I think this is what I need, will try it in a couple of hours and let you know, thanks for the efforts trying to understand what I mean :D I have such a hard time describing what I want to do. – Dlacrem Apr 02 '14 at 15:46
  • 1
    you should remove `element.ready` – A. Wolff Apr 02 '14 at 15:53
  • @A.Wolff: Well spotted. I was concentrating more on the refactoring than the code tidying. :) Of course its worth noting that while the `.ready` shouldn't be here if it is there because the OP hasn't got the call to this function in a ready then it might be necessary to add one where this function is getting called. – Chris Apr 02 '14 at 16:07
  • The functions are being called by window.location.hash changes. if (window.location.hash == ""){ showRed(); } – Dlacrem Apr 02 '14 at 16:13
  • In that case the first method I suggested should work I hope. Let me know if there is anything you don't understand. – Chris Apr 02 '14 at 16:30
  • Alright, so I created an empty variable called "element", then created another called "hello", added all the conditionals and animations there from showGreen and replaced the green selector for element, and then, on showGreen, showRed and showBlue, I change the element variable for green, red and blue accordingly, and it works perfectly. Now I'm going to try creating another variable for position so I can add those conditionals too. Thank you so much I feel like I've made a lot of progress in my learning just with this. – Dlacrem Apr 03 '14 at 10:15
  • @Dlacrem: That's good to here. I always try to explain my thought process behind something like this rather than just giving you the result to make it easier for future similar problems. "Teach a man to fish" and all that. :) – Chris Apr 03 '14 at 10:54
1

Are you just looking for something like that ?

$(document).ready(function () {
    var $red = $('.red');
    $red.addClass("active");
    $red.somethingelse();
});
Raphaël Althaus
  • 59,727
  • 6
  • 96
  • 122
1

Something like this?:

$(document).ready(function () { 
    function updateRed($class) {
        $class.addClass("active");  
    }
    var $red = $('.red');
    updateRed($red);
});

http://jsfiddle.net/LGDz2/2/

Barbara Laird
  • 12,599
  • 2
  • 44
  • 57
1

Your issue is that in your code this is referring to document object. That's because ready pseudo event is bound to document, unregarding any oject used to bound this pseudo event.

So indeed, looks like you want:

$(document).ready(function(){
    var $reds = $('.red'); 
    // then you can use $reds inside this ready handler scope
});
A. Wolff
  • 74,033
  • 9
  • 94
  • 155