Reputation: 263
I am working on a video slider, in which following video starts just before the previous ends - in a loop. At the same time i need to update a progress bar of each one. The code below works, but Chrome is burning my macbook. Am I doing sth wrong? How can I improve the performance? There are free slides, and videos has no loop="true" attribute. I just start the slider by playing the first video.
videos.on('timeupdate', function () {
var progress = this.currentTime / this.duration * 100 + '%';
$(this).closest('.slide').find('.js-video-progress').width(progress);
if ((this.currentTime * 1000 + 1000) > this.duration * 1000) {
if ($(this).closest('.slide').next('.slide').length) {
$(this).closest('.slide').next('.slide').addClass('is-swiped').find('video')[0].play();
} else {
$('.slide').eq(0).addClass('is-swiped').find('video')[0].play();
}
}
});
Upvotes: 0
Views: 126
Reputation: 136698
timeupdate
may fire at an high rate (possibly even faster than your screen refresh rate), so whatever you do in there must be as light as possible.
jQuery's methods do a lot of things every time you call them, and always return new objects which will soon pollute all your memory, forcing the GarbageCollector to come in every time. So don't call twice the same function and instead store the result in a variable.
Here is a slightly better approach storing each reused object in the handler itself:
videos.on('timeupdate', function () {
var time = this.currentTime, // it's a getter function, store
var dur = this.duration;
var progress = time / duration * 100 + '%';
var $this = $(this); // store
var $slide = $this.closest('.slide');
var $progress = $slide.find('.js-video-progress');
var $slide_next;
$progress.width(progress);
if ((time * 1000 + 1000) > dur * 1000) {
$slide_next = $slide.next('.slide');
if ($slide_next.length) {
$slide_next
.addClass('is-swiped')
.find('video')[0]
.play();
} else {
$slide.eq(0)
.addClass('is-swiped')
.find('video')[0]
.play();
}
}
});
But personally, I think would even go deeper, at the risk of being verbose, and store all the needed objects in a static object that I would just lookup in the handlers:
videos.each(function attach_data() {
var $this = $(this);
var $slide = $this.closest('.slide');
var $progress = $slide.find('.js-video-progress');
var $video = $slide.eq(0).find('video');
var $slide_next = $slide.next('.slide');
var $next_video = $slide_next.find('video');
$this.data('$tree', {
slide: $slide.eq(0),
progress: $progress,
video: $video,
slide_next: $slide_next,
video_next: $video_next
})
})
.on('timeupdate', function () {
var time = this.currentTime;
var dur = this.duration;
var progress = time / duration * 100 + '%';
// retrieve our stored object
var $tree = $(this).data('$tree');
$tree.progress.width(progress);
if ((time * 1000 + 1000) > dur * 1000) {
if ($tree.slide_next.length) {
$tree.slide_next.addClass('is-swiped');
$tree.next_video[0].play(); // you may want to add a check the <video> really exists
} else {
$tree.slide.addClass('is-swiped');
$tree.video[0].play();
}
}
});
Upvotes: 1