fix(FEC-7355): don't show poster when autoplaying is on #158
Conversation
src/player.js
Outdated
| this.configure(config); | ||
| (this.config.playback && !this.config.playback.autoplay) && this._appendPosterEl(); |
There was a problem hiding this comment.
This is the poster DOM placeholder, you want to avoid setting the poster source, not the entire DOM element
There was a problem hiding this comment.
@OrenMe you don't even want to avoid setting the poster source but only avoid to show it (i.e. posterManager.show())
|
@odedhutzler if change media seems like a related change, IMO it is, then do it in same PR |
src/player.js
Outdated
| this.configure(config); | ||
| (this.config.playback && !this.config.playback.autoplay) && this._appendPosterEl(); |
There was a problem hiding this comment.
@odedhutzler you can't rely on config.playback.autoplay in this manner since it's can be true but the browser disallow autoplay
| @@ -346,8 +346,6 @@ export default class Player extends FakeEventTarget { | |||
| } | |||
| if (this._selectEngineByPriority()) { | |||
| this._appendEngineEl(); | |||
| this._posterManager.setSrc(this._config.metadata.poster); | |||
There was a problem hiding this comment.
You want to set the source anyway, whether if we will hide it or show it later.
src/utils/poster-manager.js
Outdated
| * @param {string} posterUrl - the poster image URL | ||
| * @returns {void} | ||
| */ | ||
| setAndShow(posterUrl: ?string): void { |
There was a problem hiding this comment.
First, this method is redundant (see my comment above)
Second, the name of it is too generic - set and show what?
src/player.js
Outdated
| @@ -1280,13 +1278,16 @@ export default class Player extends FakeEventTarget { | |||
| this.dispatchEvent(new FakeEvent(CustomEvents.FALLBACK_TO_MUTED_AUTOPLAY)); | |||
| } else { | |||
| Player._logger.warn("Autoplay failed, pause player"); | |||
| this._posterManager.setAndShow(this._config.metadata.poster); | |||
There was a problem hiding this comment.
call only this._posterManager.show()
src/player.js
Outdated
| this.load(); | ||
| this.ready().then(() => this.pause()); | ||
| this.dispatchEvent(new FakeEvent(CustomEvents.AUTOPLAY_FAILED)); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| } else { | ||
| this._posterManager.setAndShow(this._config.metadata.poster); |
There was a problem hiding this comment.
call only this._posterManager.show()
odedhutzler
changed the title
Description of the Changes
When autoplay is on, the poster might be shown for few miliseconds, which looks buggy.
If autoplay is on & the browser is capable of doing it, I am hiding the poster element .
CheckLists