Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Poster hidden when item index > 0 #260

Merged
merged 2 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/auto-advance.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ const setup = (player, delay) => {
player.playlist.autoadvance_.timeout = player.setTimeout(() => {
reset(player);
player.off('play', cancelOnPlay);
player.playlist.next();
// Poster should be suppressed when auto-advancing
player.playlist.next(true);
}, delay * 1000);
};

Expand Down
6 changes: 4 additions & 2 deletions src/play-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ const clearTracks = (player) => {
*
* @param {Object} item
* A source from the playlist.
* @param {boolean} [suppressPoster]
* Should the native poster be suppressed? Defaults to false.
*
* @return {Player}
* The player that is now playing the item
*/
const playItem = (player, item) => {
const playItem = (player, item, suppressPoster = false) => {
const replay = !player.paused() || player.ended();

player.trigger('beforeplaylistitem', item.originalValue || item);
Expand All @@ -38,7 +40,7 @@ const playItem = (player, item) => {
player.playlist.currentPlaylistItemId_ = item.playlistItemId_;
}

player.poster(item.poster || '');
player.poster(suppressPoster ? '' : item.poster || '');
player.src(item.sources);
clearTracks(player);

Expand Down
24 changes: 9 additions & 15 deletions src/playlist-maker.js
Original file line number Diff line number Diff line change
Expand Up @@ -290,11 +290,13 @@ export default function factory(player, initialList, initialIndex = 0) {
*
* @param {number} [index]
* If given as a valid value, plays the playlist item at that index.
* @param {boolean} [suppressPoster]
* Should the native poster be suppressed? Defaults to false.
*
* @return {number}
* The current item index.
*/
playlist.currentItem = (index) => {
playlist.currentItem = (index, suppressPoster) => {
// If the playlist is changing, only act as a getter.
if (changing) {
return playlist.currentIndex_;
Expand All @@ -310,19 +312,9 @@ export default function factory(player, initialList, initialIndex = 0) {
playlist.currentIndex_ = index;
playItem(
playlist.player_,
list[playlist.currentIndex_]
list[playlist.currentIndex_],
suppressPoster
);

// When playing multiple videos in a playlist the videojs PosterImage
// will be hidden using CSS. However, in some browsers the native poster
// attribute will briefly appear while the new source loads. Prevent
// this by hiding every poster after the first play list item. This
// doesn't cover every use case for showing/hiding the poster, but
// it will significantly improve the user experience.
if (index > 0) {
player.poster('');
}

return playlist.currentIndex_;
}

Expand Down Expand Up @@ -604,18 +596,20 @@ export default function factory(player, initialList, initialIndex = 0) {
/**
* Plays the next item in the playlist.
*
* @param {boolean} [suppressPoster]
* Should the native poster be suppressed? Defaults to false.
* @return {Object|undefined}
* Returns undefined and has no side effects if on last item.
*/
playlist.next = () => {
playlist.next = (suppressPoster = false) => {
dzianis-dashkevich marked this conversation as resolved.
Show resolved Hide resolved
if (changing) {
return;
}

const index = playlist.nextIndex();

if (index !== playlist.currentIndex_) {
const newItem = playlist.currentItem(index);
const newItem = playlist.currentItem(index, suppressPoster);

return list[newItem].originalValue || list[newItem];
}
Expand Down
10 changes: 4 additions & 6 deletions test/playlist-maker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,22 +167,20 @@ QUnit.test('playlist.currentItem() works as expected', function(assert) {
assert.equal(playlist.currentItem(-Infinity), 2, 'cannot change to an invalid item');
});

QUnit.test('playlist.currentItem() shows the poster for the first video', function(assert) {
QUnit.test('playlist.currentItem() shows a poster by default', function(assert) {
const player = playerProxyMaker();
const playlist = playlistMaker(player, videoList);

playlist.currentItem(0);
assert.notEqual(player.poster(), '', 'poster is shown for playlist index 0');
});

QUnit.test('playlist.currentItem() hides the poster for all videos after the first', function(assert) {
QUnit.test('playlist.currentItem() will hide the poster if suppressPoster param is true', function(assert) {
const player = playerProxyMaker();
const playlist = playlistMaker(player, videoList);

for (let i = 1; i <= playlist.lastIndex(); i++) {
playlist.currentItem(i);
assert.equal(player.poster(), '', 'poster is hidden for playlist index ' + i);
}
playlist.currentItem(1, true);
assert.equal(player.poster(), '', 'poster is suppressed');
});

QUnit.test('playlist.currentItem() returns -1 with an empty playlist', function(assert) {
Expand Down