Callback instead of StopVideo

[mcschneider]

UPDATE: As I am new to GitHub I needed to clean up this request.

I think the videoStop function should not be included, to give us more control. Single Responsibility.

By moving it to a callback function I can now decide how to handle it. The four 2 new callback functions are called before and after each show or hide of a tab after a tab is displayed and after it is hidden. The tab is passed to the callback. Only if the tab was actually active will it be passed. Before, all videos in all tabs were reset by the stopVideo function which can create more requests than necessary.

The old callBack functions are only needed if there are multiple tab-contents for the same tab and something needs to happen right before or after everything.

I am sorry that the other pull request is included.

[cferdinandi]

Can you walk me through why you've made such drastic changes (including removing the stopVideo function), and how this differs from you last pull request?

[cferdinandi]

Let me give this one a little thought. I like the additional structure of the callbacks and the checks around adding classes. I'm not totally convinced on removing the stopVideo function, though. For non-developers, that's a helpful "built-in" functionality, and I can't fathom why anyone would want a video to keep playing after the content area is closed.

One additional thought: It might be helpful to submit unrelated changes as individual pull requests (for example: additional callbacks as one, the check on classes as another, and the removal of stopVideos as a third). That provides me with more options on how to move forward with the request.

[cferdinandi]

One additional note: That second options merge is not redundant. It allows users to call the toggleTab function in their own scripts, while providing the flexibility of passing in their own options of just using the defaults.

[cferdinandi]

See here: https://github.com/cferdinandi/tabby#use-tabby-events-in-your-own-scripts

[mcschneider]

I am developing a page where I put a slider with many videos in a tab. Its a little tricky to get it to work across browsers. One of the problems was that your version refreshes every single video in that slider. That's why I had to remove it. I am very sorry I submitted all these changes as one pull request. I merged the changes to my master before creating a new branch. I am very new to social coding and Git.

Maybe you could add the stopVideo function to the tutorial page inside the sample code:

tabby.init({
    callbackAfterHide: function (tab) {
var iframe = tab.querySelector( 'iframe');
        var video = tab.querySelector( 'video' );
        if ( iframe !== null ) {
            var iframeSrc = iframe.src;
            iframe.src = iframeSrc;
        }
        if ( video !== null ) {
            video.pause();
        }
} // Function that's run after the old tab is hidden
});

[mcschneider]

Sorry about the options removal. Got it.

[mcschneider]

Another option could be to set a flag 'stopVideo' that is true by default. If its true the stopVideo function could be called from the _hideOtherTabs() function only on the last active tab. What do you think? I still think that the stopVideo function belongs somewhere else where it can be called by Slider and Houdini without duplicating the code.

[cferdinandi]

I am very sorry I submitted all these changes as one pull request. I merged the changes to my master before creating a new branch. I am very new to social coding and Git.

No worries at all! I really appreciate all of the contributions you've been making. Slider is in need of a bigger overhaul, when I have the time.

I still think that the stopVideo function belongs somewhere else where it can be called by Slider and Houdini without duplicating the code.

Sold. When you put it like that, you're right. It belongs outside, referenced by callback.

[mcschneider]

I cleaned this up a little. Hope it's easier for you now to merge.

[cferdinandi]

Love the approach here. I'll be modifying the callback naming conventions to be a bit more obvious:

• callbackBeforeShow
• callbackAfterShow
• callbackBeforeHide
• callbackAfterHide

I'll also be updating the documentation and version numbers to reflect the changes.

[cferdinandi]

After revisting the code, I decided to maintain the original callback structure and keep the _stopVideos function in. However, I did make two additions that should address your concerns at least partially:

1. The script runs a check to see if the tab content area is active before stopping the video. This prevents unnecessary src resets.
2. The callback functions now expose the toggle and tabID variables, which allows for more control over how they're used.

Ultimately, I favor simplicity over robustness, and plug-and-play over DIY. That's sometimes at odds with how other devs approach these things, so no hard feelings if you're thrilled with the change in direction.

Thanks again for all of your scripting help.