CODEWITHSUNDEEP
javascriptjquerycss
Aziz
Aziz

Reputation: 7793

Is it safe to wrap a function inside jQuery.removeClass()?

I'm working on a custom slider which has a dynamic video. Each time the slide changes, a new video is inserted into the DOM while the previous one gets deleted. I'm using CSS transition with a class to change video's position into view, so ideally I want the function to:

1) Add a CSS class to old video

2) remove() or detach() old video**

3) Add a CSS class to new video

The code I have so far is

function videoDetach() {

    // restore loading spinner
    $('.spinnerMsg').removeClass("nospin");

    // move video out of screen and bring the next one in
    $('#bgIn video:first').removeClass( function (){
        $('#bgIn video:first').addClass("outview");
        setTimeout(function(){ 
            $('#bgIn video:first').detach();
            $('#bgIn video:last').addClass("inview");
        },250);
    });
}

This code works as expected but I am not sure if it's safe or correct, what is the best practice for accomplishing such a task?

** not sure which one to choose, each video will be inserted back as the slider repeats itself so detach() seems appealing, but is it ok to store multiple videos in the DOM which have the same id? also will remove(); force the users to redownload the entire video?

All feedback appreciated.

UPDATE

tweaked code following T.J's answer as it was a bit unstable by either detaching all videos or allowing duplicates, the final code looks like this:

function videoDetach() {

    // restore loading spinner
    $('.spinnerMsg').removeClass("nospin");

    // move current video out of screen
    $('#bgIn video').addClass("outview");

    setTimeout(function(){ 
        $('#bgIn video.outview').detach(); // detach old video
        $('#bgIn video').addClass("inview"); // bring new video into view
    },250);
}

Upvotes: 0

Views: 146

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1075119

The removeClass call that you're passing the function into is effectively a no-op, you could remove it entirely without changing what happens:

function videoDetach() {

    // restore loading spinner
    $('.spinnerMsg').removeClass("nospin");

    // move video out of screen and bring the next one in
    $('#bgIn video:first').addClass("outview");
    setTimeout(function(){ 
        $('#bgIn video:first').detach();
        $('#bgIn video:last').addClass("inview");
    },250);
}

If you give a function to removeClass (or most of jQuery's other setter operations), jQuery will call that function once for each DOM element in the set; it expects the function to return something useful. In the case of removeClass, it expects the function to return the class to remove from that element.

The only reason it's working is that jQuery will either call the function exactly once (if there's an element matching #bgIn video:first) or not at all, and it happens that the code in the function only does something if there is an element matching #bgIn video:first. If you'd done that removeClass on a jQuery set containing multiple elements, it would have called your function repeatedly; if you'd called it on a set with no elements in it at all, your function would never have been called. If the code inside operated on different elements than the ones selected for the removeClass, the result might have seemed chaotic.

what is the best practice for accomplishing such a task?

Just running the code directly, as above, not using side-effects of operations incorrectly.

not sure which one to choose, each video will be inserted back as the slider repeats itself so detach() seems appealing, but is it ok to store multiple videos in the DOM which have the same id?

If the elements are detached, they aren't in the DOM; it's fine to have an element that exists, but is not in the DOM, that has the same id as an element in the DOM. But based on your code, you wouldn't, because you're detaching the video element (which doesn't have an id as far as your code demonstrates), not the #bgIn element.

also will remove(); force the users to redownload the entire video?

No, the browser's cache is not element-specific.

Upvotes: 2

Related Questions