From a3af02fcf3397895f2394a5d72304e744afb4140 Mon Sep 17 00:00:00 2001 From: Tahsin Islam Date: Mon, 16 Aug 2021 14:12:08 -0400 Subject: [PATCH 1/2] All other loaded videos pause when one video is playing --- .../obojobo-chunks-youtube/youtube-player.js | 20 +++++ .../youtube-player.test.js | 82 ++++++++++++++++++- 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/packages/obonode/obojobo-chunks-youtube/youtube-player.js b/packages/obonode/obojobo-chunks-youtube/youtube-player.js index 961dc03b7c..7be3c1877e 100644 --- a/packages/obonode/obojobo-chunks-youtube/youtube-player.js +++ b/packages/obonode/obojobo-chunks-youtube/youtube-player.js @@ -10,6 +10,8 @@ const { uuid } = Common.util // They'll all get called once it loads const instanceCallbacksForYouTubeReady = [] +const loadedVideos = [] + // single global hangler that notifies all registered YouTubePlayer Components const onYouTubeIframeAPIReadyHandler = () => { // call every registered callback when ready @@ -22,6 +24,7 @@ class YouTubePlayer extends React.Component { this.playerId = `obojobo-draft--chunks-you-tube-player-${uuid()}` this.player = null this.loadVideo = this.loadVideo.bind(this) + this.onStateChange = this.onStateChange.bind(this) } componentDidMount() { @@ -79,8 +82,13 @@ class YouTubePlayer extends React.Component { playerVars: { start: startTime, end: endTime + }, + events: { + onStateChange: this.onStateChange } }) + + loadedVideos.push(this.player) } shouldVideoUpdate(prevProps) { @@ -97,6 +105,18 @@ class YouTubePlayer extends React.Component { render() { return
} + + onStateChange(playerState) { + switch (playerState.data) { + case 1: // video playing + for (let i = 0; i < loadedVideos.length; i++) { + if (loadedVideos[i].getVideoUrl() !== playerState.target.getVideoUrl()) { + loadedVideos[i].pauseVideo() + } + } + break + } + } } export default YouTubePlayer diff --git a/packages/obonode/obojobo-chunks-youtube/youtube-player.test.js b/packages/obonode/obojobo-chunks-youtube/youtube-player.test.js index 3985a3ab4d..288c26369f 100644 --- a/packages/obonode/obojobo-chunks-youtube/youtube-player.test.js +++ b/packages/obonode/obojobo-chunks-youtube/youtube-player.test.js @@ -26,7 +26,9 @@ describe('YouTubePlayer', () => { // Mock YouTube's Iframe Player object window.YT = { Player: jest.fn(() => ({ - cueVideoById: jest.fn() + cueVideoById: jest.fn(), + getVideoUrl: jest.fn(), + pauseVideo: jest.fn() })) } @@ -60,13 +62,16 @@ describe('YouTubePlayer', () => { }) expect(spy).toHaveBeenCalledTimes(4) + spy.mockRestore() }) test("YouTubePlayer doesn't update if content hasn't changed", () => { window.YT = { Player: jest.fn(() => ({ destroy: jest.fn(), - cueVideoById: jest.fn() + cueVideoById: jest.fn(), + getVideoUrl: jest.fn(), + pauseVideo: jest.fn() })) } @@ -86,5 +91,78 @@ describe('YouTubePlayer', () => { expect(spy).toHaveBeenCalledTimes(1) expect(component.instance().player.destroy).not.toHaveBeenCalled() + + spy.mockRestore() + }) + + test('Pause other players when a player is unpaused', () => { + const player1 = { + destroy: jest.fn(), + cueVideoById: jest.fn(), + getVideoUrl: () => 'url-1', + pauseVideo: jest.fn() + } + const player2 = { + destroy: jest.fn(), + cueVideoById: jest.fn(), + getVideoUrl: () => 'url-2', + pauseVideo: jest.fn() + } + + const mockContent1 = { + videoId: 'mockIDzz', + startTime: 1, + endTime: 2, + playerState: 2 + } + jest.mock('obojobo-document-engine/src/scripts/common/util/uuid', () => { + return () => 'mockId' + }) + const mockContent2 = { + videoId: 'mockIDzz2', + startTime: 1, + endTime: 2, + playerState: 2 + } + + window.YT = { + Player: jest.fn(() => player1), + test: 123 + } + const component1 = mount() + + window.YT = { + Player: jest.fn(() => player2), + test: 456 + } + const component2 = mount() + + component1.instance().onStateChange({ + data: 1, + target: player1 + }) + expect(player1.pauseVideo).not.toHaveBeenCalled() + expect(player2.pauseVideo).toHaveBeenCalledTimes(1) + + component1.instance().onStateChange({ + data: 1, + target: player2 + }) + expect(player2.pauseVideo).toHaveBeenCalledTimes(1) + expect(player1.pauseVideo).toHaveBeenCalledTimes(1) + + component2.instance().onStateChange({ + data: 1, + target: player1 + }) + expect(player1.pauseVideo).toHaveBeenCalledTimes(1) + expect(player2.pauseVideo).toHaveBeenCalledTimes(2) + + component2.instance().onStateChange({ + data: 1, + target: player2 + }) + expect(player1.pauseVideo).toHaveBeenCalledTimes(2) + expect(player2.pauseVideo).toHaveBeenCalledTimes(2) }) }) From ee6008ec3fab46d0d966c7c7280a466f6c8a183b Mon Sep 17 00:00:00 2001 From: Zachary Berry Date: Wed, 18 Aug 2021 20:29:46 -0400 Subject: [PATCH 2/2] Now pauses videos based on their unique youtube API id, also removes API player data on unmount --- .../obojobo-chunks-youtube/youtube-player.js | 21 +++++++++++++------ .../youtube-player.test.js | 8 +++---- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/obonode/obojobo-chunks-youtube/youtube-player.js b/packages/obonode/obojobo-chunks-youtube/youtube-player.js index 7be3c1877e..c00d550f99 100644 --- a/packages/obonode/obojobo-chunks-youtube/youtube-player.js +++ b/packages/obonode/obojobo-chunks-youtube/youtube-player.js @@ -10,7 +10,7 @@ const { uuid } = Common.util // They'll all get called once it loads const instanceCallbacksForYouTubeReady = [] -const loadedVideos = [] +const loadedVideos = {} // single global hangler that notifies all registered YouTubePlayer Components const onYouTubeIframeAPIReadyHandler = () => { @@ -25,6 +25,10 @@ class YouTubePlayer extends React.Component { this.player = null this.loadVideo = this.loadVideo.bind(this) this.onStateChange = this.onStateChange.bind(this) + + this.state = { + youTubePlayerId: null + } } componentDidMount() { @@ -36,6 +40,10 @@ class YouTubePlayer extends React.Component { } } + componentWillUnmount() { + delete loadedVideos[this.state.youTubePlayerId] + } + componentDidUpdate(prevProps) { // Don't update this component if the video properties haven't changed if (!this.shouldVideoUpdate(prevProps)) { @@ -87,8 +95,9 @@ class YouTubePlayer extends React.Component { onStateChange: this.onStateChange } }) + this.setState({ youTubePlayerId: this.player.id }) - loadedVideos.push(this.player) + loadedVideos[this.player.id] = this.player } shouldVideoUpdate(prevProps) { @@ -109,11 +118,11 @@ class YouTubePlayer extends React.Component { onStateChange(playerState) { switch (playerState.data) { case 1: // video playing - for (let i = 0; i < loadedVideos.length; i++) { - if (loadedVideos[i].getVideoUrl() !== playerState.target.getVideoUrl()) { - loadedVideos[i].pauseVideo() + Object.values(loadedVideos).forEach(video => { + if (video.id !== playerState.target.id) { + video.pauseVideo() } - } + }) break } } diff --git a/packages/obonode/obojobo-chunks-youtube/youtube-player.test.js b/packages/obonode/obojobo-chunks-youtube/youtube-player.test.js index 288c26369f..906b8b3d31 100644 --- a/packages/obonode/obojobo-chunks-youtube/youtube-player.test.js +++ b/packages/obonode/obojobo-chunks-youtube/youtube-player.test.js @@ -20,6 +20,8 @@ describe('YouTubePlayer', () => { const tree = component.html() expect(tree).toMatchSnapshot() + + component.unmount() }) test('YouTubePlayer updates video on content change', () => { @@ -27,7 +29,6 @@ describe('YouTubePlayer', () => { window.YT = { Player: jest.fn(() => ({ cueVideoById: jest.fn(), - getVideoUrl: jest.fn(), pauseVideo: jest.fn() })) } @@ -70,7 +71,6 @@ describe('YouTubePlayer', () => { Player: jest.fn(() => ({ destroy: jest.fn(), cueVideoById: jest.fn(), - getVideoUrl: jest.fn(), pauseVideo: jest.fn() })) } @@ -99,13 +99,13 @@ describe('YouTubePlayer', () => { const player1 = { destroy: jest.fn(), cueVideoById: jest.fn(), - getVideoUrl: () => 'url-1', + id: 1, pauseVideo: jest.fn() } const player2 = { destroy: jest.fn(), cueVideoById: jest.fn(), - getVideoUrl: () => 'url-2', + id: 2, pauseVideo: jest.fn() }