From 98b1a52f128e1d20d0202266d5e8aa090b2fdbc7 Mon Sep 17 00:00:00 2001 From: Tim Mendoza Date: Tue, 9 Mar 2021 19:51:10 -0700 Subject: [PATCH 1/8] Update useScreenShareToggle to stop screen track on room disconnect --- .../EndCallButton/EndCallButton.test.tsx | 21 ------------------- .../Buttons/EndCallButton/EndCallButton.tsx | 5 +---- .../useScreenShareToggle.test.tsx | 20 +++++++++++++++--- .../useScreenShareToggle.tsx | 17 ++++++++++++++- 4 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/components/Buttons/EndCallButton/EndCallButton.test.tsx b/src/components/Buttons/EndCallButton/EndCallButton.test.tsx index faf7769e0..c066582e5 100644 --- a/src/components/Buttons/EndCallButton/EndCallButton.test.tsx +++ b/src/components/Buttons/EndCallButton/EndCallButton.test.tsx @@ -8,7 +8,6 @@ const mockVideoContext = { disconnect: jest.fn(), }, isSharingScreen: false, - toggleScreenShare: jest.fn(), removeLocalAudioTrack: jest.fn(), removeLocalVideoTrack: jest.fn(), }; @@ -22,7 +21,6 @@ describe('End Call button', () => { beforeAll(() => { jest.clearAllMocks(); - mockVideoContext.isSharingScreen = true; wrapper = shallow(); wrapper.simulate('click'); }); @@ -35,28 +33,9 @@ describe('End Call button', () => { expect(mockVideoContext.removeLocalVideoTrack).toHaveBeenCalled(); }); - it('should toggle screen sharing off', () => { - expect(mockVideoContext.toggleScreenShare).toHaveBeenCalled(); - }); - it('should disconnect from the room ', () => { expect(mockVideoContext.room.disconnect).toHaveBeenCalled(); }); }); - - describe('while not sharing screen', () => { - let wrapper; - - beforeAll(() => { - jest.clearAllMocks(); - mockVideoContext.isSharingScreen = false; - wrapper = shallow(); - wrapper.simulate('click'); - }); - - it('should not toggle screen sharing', () => { - expect(mockVideoContext.toggleScreenShare).not.toHaveBeenCalled(); - }); - }); }); }); diff --git a/src/components/Buttons/EndCallButton/EndCallButton.tsx b/src/components/Buttons/EndCallButton/EndCallButton.tsx index d34e60daf..a14ca7b99 100644 --- a/src/components/Buttons/EndCallButton/EndCallButton.tsx +++ b/src/components/Buttons/EndCallButton/EndCallButton.tsx @@ -20,12 +20,9 @@ const useStyles = makeStyles((theme: Theme) => export default function EndCallButton(props: { className?: string }) { const classes = useStyles(); - const { room, isSharingScreen, toggleScreenShare, removeLocalAudioTrack, removeLocalVideoTrack } = useVideoContext(); + const { room, removeLocalAudioTrack, removeLocalVideoTrack } = useVideoContext(); const handleClick = () => { - if (isSharingScreen) { - toggleScreenShare(); - } removeLocalAudioTrack(); removeLocalVideoTrack(); room!.disconnect(); diff --git a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx index a994c71db..bff4903a5 100644 --- a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx +++ b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx @@ -7,9 +7,8 @@ const mockLocalParticipant = new EventEmitter() as any; mockLocalParticipant.publishTrack = jest.fn(() => Promise.resolve('mockPublication')); mockLocalParticipant.unpublishTrack = jest.fn(); -const mockRoom = { - localParticipant: mockLocalParticipant, -} as any; +const mockRoom = new EventEmitter() as any; +mockRoom.localParticipant = mockLocalParticipant; const mockOnError: ErrorCallback = () => {}; @@ -71,6 +70,21 @@ describe('the useScreenShareToggle hook', () => { expect(result.current[0]).toEqual(false); }); + it('should correctly stop screen sharing when disconnected from the room', async () => { + const { result, waitForNextUpdate } = renderHook(() => useScreenShareToggle(mockRoom, mockOnError)); + expect(mockTrack.onended).toBeUndefined(); + result.current[1](); + await waitForNextUpdate(); + expect(result.current[0]).toEqual(true); + act(() => { + mockRoom.emit('disconnected'); + }); + expect(mockLocalParticipant.unpublishTrack).toHaveBeenCalledWith(mockTrack); + expect(mockLocalParticipant.unpublishTrack).toHaveBeenCalledWith(mockTrack); + expect(mockTrack.stop).toHaveBeenCalled(); + expect(result.current[0]).toEqual(false); + }); + describe('onended function', () => { it('should correctly stop screen sharing when called', async () => { const localParticipantSpy = jest.spyOn(mockLocalParticipant, 'emit'); diff --git a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx index 9a3b3b66d..43a80a279 100644 --- a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx +++ b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx @@ -1,4 +1,4 @@ -import { useState, useCallback, useRef } from 'react'; +import { useState, useCallback, useRef, useEffect } from 'react'; import { LogLevels, Track, Room } from 'twilio-video'; import { ErrorCallback } from '../../../types'; @@ -61,5 +61,20 @@ export default function useScreenShareToggle(room: Room | null, onError: ErrorCa } }, [isSharing, shareScreen, stopScreenShareRef, room]); + useEffect(() => { + if (room) { + const handleDisconnect = () => { + if (isSharing) { + stopScreenShareRef.current(); + } + }; + + room.on('disconnected', handleDisconnect); + return () => { + room.off('disconnected', handleDisconnect); + }; + } + }, [room, isSharing]); + return [isSharing, toggleScreenShare] as const; } From 5fd914eae19cf6419555ada043d34911bbd630b2 Mon Sep 17 00:00:00 2001 From: Tim Mendoza Date: Tue, 9 Mar 2021 20:05:49 -0700 Subject: [PATCH 2/8] Rename useHandleRoomDisconnectionError to useHandleRoomDisconnection --- .../useHandleRoomDisconnection.test.tsx} | 12 ++++++------ .../useHandleRoomDisconnection.ts} | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) rename src/components/VideoProvider/{useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors.test.tsx => useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx} (79%) rename src/components/VideoProvider/{useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors.ts => useHandleRoomDisconnection/useHandleRoomDisconnection.ts} (82%) diff --git a/src/components/VideoProvider/useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors.test.tsx b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx similarity index 79% rename from src/components/VideoProvider/useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors.test.tsx rename to src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx index 926a1e0aa..d6711b719 100644 --- a/src/components/VideoProvider/useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors.test.tsx +++ b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx @@ -1,15 +1,15 @@ import { act, renderHook } from '@testing-library/react-hooks'; import EventEmitter from 'events'; -import useHandleRoomDisconnectionErrors from './useHandleRoomDisconnectionErrors'; +import useHandleRoomDisconnection from './useHandleRoomDisconnection'; import { Room } from 'twilio-video'; -describe('the useHandleRoomDisconnectionErrors hook', () => { +describe('the useHandleRoomDisconnection hook', () => { let mockRoom: any = new EventEmitter(); it('should do nothing if the room emits a "disconnected" event with no error', () => { const mockOnError = jest.fn(); - renderHook(() => useHandleRoomDisconnectionErrors(mockRoom, mockOnError)); + renderHook(() => useHandleRoomDisconnection(mockRoom, mockOnError)); act(() => { mockRoom.emit('disconnected', 'disconnected'); }); @@ -18,7 +18,7 @@ describe('the useHandleRoomDisconnectionErrors hook', () => { it('should react to the rooms "disconnected" event and invoke onError callback if there is an error', () => { const mockOnError = jest.fn(); - renderHook(() => useHandleRoomDisconnectionErrors(mockRoom, mockOnError)); + renderHook(() => useHandleRoomDisconnection(mockRoom, mockOnError)); act(() => { mockRoom.emit('disconnected', 'disconnected', 'mockError'); }); @@ -27,7 +27,7 @@ describe('the useHandleRoomDisconnectionErrors hook', () => { it('should tear down old listeners when receiving a new room', () => { const originalMockRoom = mockRoom; - const { rerender } = renderHook(() => useHandleRoomDisconnectionErrors(mockRoom, () => {})); + const { rerender } = renderHook(() => useHandleRoomDisconnection(mockRoom, () => {})); expect(originalMockRoom.listenerCount('disconnected')).toBe(1); act(() => { @@ -41,7 +41,7 @@ describe('the useHandleRoomDisconnectionErrors hook', () => { }); it('should clean up listeners on unmount', () => { - const { unmount } = renderHook(() => useHandleRoomDisconnectionErrors(mockRoom, () => {})); + const { unmount } = renderHook(() => useHandleRoomDisconnection(mockRoom, () => {})); unmount(); expect(mockRoom.listenerCount('disconnected')).toBe(0); }); diff --git a/src/components/VideoProvider/useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors.ts b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.ts similarity index 82% rename from src/components/VideoProvider/useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors.ts rename to src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.ts index 78f5e16ed..d7945e2e1 100644 --- a/src/components/VideoProvider/useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors.ts +++ b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.ts @@ -3,7 +3,7 @@ import { useEffect } from 'react'; import { Callback } from '../../../types'; -export default function useHandleRoomDisconnectionErrors(room: Room | null, onError: Callback) { +export default function useHandleRoomDisconnection(room: Room | null, onError: Callback) { useEffect(() => { if (room) { const onDisconnected = (_: Room, error: TwilioError) => { From e49580e55936d2f66bacf1bef77bfcac5c24573f Mon Sep 17 00:00:00 2001 From: Tim Mendoza Date: Wed, 10 Mar 2021 09:25:41 -0700 Subject: [PATCH 3/8] Update end call button to only disconnect from room --- .../EndCallButton/EndCallButton.test.tsx | 29 +++---------------- .../Buttons/EndCallButton/EndCallButton.tsx | 10 ++----- 2 files changed, 6 insertions(+), 33 deletions(-) diff --git a/src/components/Buttons/EndCallButton/EndCallButton.test.tsx b/src/components/Buttons/EndCallButton/EndCallButton.test.tsx index c066582e5..940dd6afe 100644 --- a/src/components/Buttons/EndCallButton/EndCallButton.test.tsx +++ b/src/components/Buttons/EndCallButton/EndCallButton.test.tsx @@ -7,35 +7,14 @@ const mockVideoContext = { room: { disconnect: jest.fn(), }, - isSharingScreen: false, - removeLocalAudioTrack: jest.fn(), - removeLocalVideoTrack: jest.fn(), }; jest.mock('../../../hooks/useVideoContext/useVideoContext', () => () => mockVideoContext); describe('End Call button', () => { - describe('when it is clicked', () => { - describe('while sharing screen', () => { - let wrapper; - - beforeAll(() => { - jest.clearAllMocks(); - wrapper = shallow(); - wrapper.simulate('click'); - }); - - it('should stop local audio tracks', () => { - expect(mockVideoContext.removeLocalAudioTrack).toHaveBeenCalled(); - }); - - it('should stop local video tracks', () => { - expect(mockVideoContext.removeLocalVideoTrack).toHaveBeenCalled(); - }); - - it('should disconnect from the room ', () => { - expect(mockVideoContext.room.disconnect).toHaveBeenCalled(); - }); - }); + it('should disconnect from the room when clicked', () => { + const wrapper = shallow(); + wrapper.simulate('click'); + expect(mockVideoContext.room.disconnect).toHaveBeenCalled(); }); }); diff --git a/src/components/Buttons/EndCallButton/EndCallButton.tsx b/src/components/Buttons/EndCallButton/EndCallButton.tsx index a14ca7b99..d5d9bf76e 100644 --- a/src/components/Buttons/EndCallButton/EndCallButton.tsx +++ b/src/components/Buttons/EndCallButton/EndCallButton.tsx @@ -20,16 +20,10 @@ const useStyles = makeStyles((theme: Theme) => export default function EndCallButton(props: { className?: string }) { const classes = useStyles(); - const { room, removeLocalAudioTrack, removeLocalVideoTrack } = useVideoContext(); - - const handleClick = () => { - removeLocalAudioTrack(); - removeLocalVideoTrack(); - room!.disconnect(); - }; + const { room } = useVideoContext(); return ( - ); From 59f2e1e3c72f67f88743bed0cd6d771d4d324007 Mon Sep 17 00:00:00 2001 From: Tim Mendoza Date: Wed, 10 Mar 2021 09:26:09 -0700 Subject: [PATCH 4/8] Undo changes to useScreenShareToggle --- .../useScreenShareToggle.test.tsx | 15 --------------- .../useScreenShareToggle/useScreenShareToggle.tsx | 15 --------------- 2 files changed, 30 deletions(-) diff --git a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx index bff4903a5..1760e2a7e 100644 --- a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx +++ b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx @@ -70,21 +70,6 @@ describe('the useScreenShareToggle hook', () => { expect(result.current[0]).toEqual(false); }); - it('should correctly stop screen sharing when disconnected from the room', async () => { - const { result, waitForNextUpdate } = renderHook(() => useScreenShareToggle(mockRoom, mockOnError)); - expect(mockTrack.onended).toBeUndefined(); - result.current[1](); - await waitForNextUpdate(); - expect(result.current[0]).toEqual(true); - act(() => { - mockRoom.emit('disconnected'); - }); - expect(mockLocalParticipant.unpublishTrack).toHaveBeenCalledWith(mockTrack); - expect(mockLocalParticipant.unpublishTrack).toHaveBeenCalledWith(mockTrack); - expect(mockTrack.stop).toHaveBeenCalled(); - expect(result.current[0]).toEqual(false); - }); - describe('onended function', () => { it('should correctly stop screen sharing when called', async () => { const localParticipantSpy = jest.spyOn(mockLocalParticipant, 'emit'); diff --git a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx index 43a80a279..7cf95ddd2 100644 --- a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx +++ b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx @@ -61,20 +61,5 @@ export default function useScreenShareToggle(room: Room | null, onError: ErrorCa } }, [isSharing, shareScreen, stopScreenShareRef, room]); - useEffect(() => { - if (room) { - const handleDisconnect = () => { - if (isSharing) { - stopScreenShareRef.current(); - } - }; - - room.on('disconnected', handleDisconnect); - return () => { - room.off('disconnected', handleDisconnect); - }; - } - }, [room, isSharing]); - return [isSharing, toggleScreenShare] as const; } From f3d5fc95d61da3570e1b057fb9aab3180b07a7f1 Mon Sep 17 00:00:00 2001 From: Tim Mendoza Date: Wed, 10 Mar 2021 09:28:55 -0700 Subject: [PATCH 5/8] Update useHandleRoomDisconnection hook to stop all tracks on disconnect --- src/components/VideoProvider/index.test.tsx | 14 ++- src/components/VideoProvider/index.tsx | 16 ++- .../useHandleRoomDisconnection.test.tsx | 111 ++++++++++++++++-- .../useHandleRoomDisconnection.ts | 17 ++- 4 files changed, 141 insertions(+), 17 deletions(-) diff --git a/src/components/VideoProvider/index.test.tsx b/src/components/VideoProvider/index.test.tsx index e6b4192f3..1706452df 100644 --- a/src/components/VideoProvider/index.test.tsx +++ b/src/components/VideoProvider/index.test.tsx @@ -5,7 +5,7 @@ import { Room, TwilioError } from 'twilio-video'; import { VideoProvider } from './index'; import useLocalTracks from './useLocalTracks/useLocalTracks'; import useRoom from './useRoom/useRoom'; -import useHandleRoomDisconnectionErrors from './useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors'; +import useHandleRoomDisconnection from './useHandleRoomDisconnection/useHandleRoomDisconnection'; import useHandleTrackPublicationFailed from './useHandleTrackPublicationFailed/useHandleTrackPublicationFailed'; import useVideoContext from '../../hooks/useVideoContext/useVideoContext'; @@ -17,10 +17,11 @@ jest.mock('./useLocalTracks/useLocalTracks', () => getLocalVideoTrack: () => {}, getLocalAudioTrack: () => {}, isAcquiringLocalTracks: true, + removeLocalAudioTrack: () => {}, removeLocalVideoTrack: () => {}, })) ); -jest.mock('./useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors'); +jest.mock('./useHandleRoomDisconnection/useHandleRoomDisconnection'); jest.mock('./useHandleTrackPublicationFailed/useHandleTrackPublicationFailed'); describe('the VideoProvider component', () => { @@ -47,7 +48,14 @@ describe('the VideoProvider component', () => { dominantSpeaker: true, }); expect(useLocalTracks).toHaveBeenCalled(); - expect(useHandleRoomDisconnectionErrors).toHaveBeenCalledWith(mockRoom, expect.any(Function)); + expect(useHandleRoomDisconnection).toHaveBeenCalledWith( + mockRoom, + expect.any(Function), + expect.any(Function), + expect.any(Function), + false, + expect.any(Function) + ); expect(useHandleTrackPublicationFailed).toHaveBeenCalledWith(mockRoom, expect.any(Function)); }); diff --git a/src/components/VideoProvider/index.tsx b/src/components/VideoProvider/index.tsx index 76827f8fc..82c229cac 100644 --- a/src/components/VideoProvider/index.tsx +++ b/src/components/VideoProvider/index.tsx @@ -11,7 +11,7 @@ import { ErrorCallback } from '../../types'; import { SelectedParticipantProvider } from './useSelectedParticipant/useSelectedParticipant'; import AttachVisibilityHandler from './AttachVisibilityHandler/AttachVisibilityHandler'; -import useHandleRoomDisconnectionErrors from './useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors'; +import useHandleRoomDisconnection from './useHandleRoomDisconnection/useHandleRoomDisconnection'; import useHandleTrackPublicationFailed from './useHandleTrackPublicationFailed/useHandleTrackPublicationFailed'; import useLocalTracks from './useLocalTracks/useLocalTracks'; import useRoom from './useRoom/useRoom'; @@ -33,7 +33,6 @@ export interface IVideoContext { getLocalVideoTrack: (newOptions?: CreateLocalTrackOptions) => Promise; getLocalAudioTrack: (deviceId?: string) => Promise; isAcquiringLocalTracks: boolean; - removeLocalAudioTrack: () => void; removeLocalVideoTrack: () => void; isSharingScreen: boolean; toggleScreenShare: () => void; @@ -65,10 +64,18 @@ export function VideoProvider({ options, children, onError = () => {} }: VideoPr } = useLocalTracks(); const { room, isConnecting, connect } = useRoom(localTracks, onErrorCallback, options); + const [isSharingScreen, toggleScreenShare] = useScreenShareToggle(room, onError); + // Register onError callback functions. - useHandleRoomDisconnectionErrors(room, onError); + useHandleRoomDisconnection( + room, + onError, + removeLocalAudioTrack, + removeLocalVideoTrack, + isSharingScreen, + toggleScreenShare + ); useHandleTrackPublicationFailed(room, onError); - const [isSharingScreen, toggleScreenShare] = useScreenShareToggle(room, onError); return ( {} }: VideoPr getLocalAudioTrack, connect, isAcquiringLocalTracks, - removeLocalAudioTrack, removeLocalVideoTrack, isSharingScreen, toggleScreenShare, diff --git a/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx index d6711b719..a895f5032 100644 --- a/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx +++ b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx @@ -4,30 +4,118 @@ import EventEmitter from 'events'; import useHandleRoomDisconnection from './useHandleRoomDisconnection'; import { Room } from 'twilio-video'; +const mockOnError = jest.fn(); +const mockRemoveLocalAudioTrack = jest.fn(); +const mockRemoveLocalVideoTrack = jest.fn(); +const mockToggleScreenSharing = jest.fn(); + describe('the useHandleRoomDisconnection hook', () => { let mockRoom: any = new EventEmitter(); + beforeEach(jest.clearAllMocks); + it('should do nothing if the room emits a "disconnected" event with no error', () => { - const mockOnError = jest.fn(); - renderHook(() => useHandleRoomDisconnection(mockRoom, mockOnError)); + renderHook(() => + useHandleRoomDisconnection( + mockRoom, + mockOnError, + mockRemoveLocalAudioTrack, + mockRemoveLocalVideoTrack, + false, + mockToggleScreenSharing + ) + ); act(() => { - mockRoom.emit('disconnected', 'disconnected'); + mockRoom.emit('disconnected', mockRoom); }); expect(mockOnError).not.toHaveBeenCalled(); }); it('should react to the rooms "disconnected" event and invoke onError callback if there is an error', () => { const mockOnError = jest.fn(); - renderHook(() => useHandleRoomDisconnection(mockRoom, mockOnError)); + renderHook(() => + useHandleRoomDisconnection( + mockRoom, + mockOnError, + mockRemoveLocalAudioTrack, + mockRemoveLocalVideoTrack, + false, + mockToggleScreenSharing + ) + ); act(() => { - mockRoom.emit('disconnected', 'disconnected', 'mockError'); + mockRoom.emit('disconnected', mockRoom, 'mockError'); }); expect(mockOnError).toHaveBeenCalledWith('mockError'); }); + it('should remove local tracks when the "disconnected" event is emitted', () => { + const mockOnError = jest.fn(); + renderHook(() => + useHandleRoomDisconnection( + mockRoom, + mockOnError, + mockRemoveLocalAudioTrack, + mockRemoveLocalVideoTrack, + false, + mockToggleScreenSharing + ) + ); + act(() => { + mockRoom.emit('disconnected', mockRoom, 'mockError'); + }); + expect(mockRemoveLocalAudioTrack).toHaveBeenCalled(); + expect(mockRemoveLocalVideoTrack).toHaveBeenCalled(); + }); + + it('should not toggle screensharing when the "disconnected" event is emitted and isSharing is false', () => { + const mockOnError = jest.fn(); + renderHook(() => + useHandleRoomDisconnection( + mockRoom, + mockOnError, + mockRemoveLocalAudioTrack, + mockRemoveLocalVideoTrack, + false, + mockToggleScreenSharing + ) + ); + act(() => { + mockRoom.emit('disconnected', mockRoom, 'mockError'); + }); + expect(mockToggleScreenSharing).not.toHaveBeenCalled(); + }); + + it('should toggle screensharing when the "disconnected" event is emitted and isSharing is true', () => { + const mockOnError = jest.fn(); + renderHook(() => + useHandleRoomDisconnection( + mockRoom, + mockOnError, + mockRemoveLocalAudioTrack, + mockRemoveLocalVideoTrack, + true, + mockToggleScreenSharing + ) + ); + act(() => { + mockRoom.emit('disconnected', mockRoom, 'mockError'); + }); + expect(mockToggleScreenSharing).toHaveBeenCalled(); + }); + it('should tear down old listeners when receiving a new room', () => { const originalMockRoom = mockRoom; - const { rerender } = renderHook(() => useHandleRoomDisconnection(mockRoom, () => {})); + const { rerender } = renderHook(() => + useHandleRoomDisconnection( + mockRoom, + mockOnError, + mockRemoveLocalAudioTrack, + mockRemoveLocalVideoTrack, + false, + mockToggleScreenSharing + ) + ); expect(originalMockRoom.listenerCount('disconnected')).toBe(1); act(() => { @@ -41,7 +129,16 @@ describe('the useHandleRoomDisconnection hook', () => { }); it('should clean up listeners on unmount', () => { - const { unmount } = renderHook(() => useHandleRoomDisconnection(mockRoom, () => {})); + const { unmount } = renderHook(() => + useHandleRoomDisconnection( + mockRoom, + mockOnError, + mockRemoveLocalAudioTrack, + mockRemoveLocalVideoTrack, + false, + mockToggleScreenSharing + ) + ); unmount(); expect(mockRoom.listenerCount('disconnected')).toBe(0); }); diff --git a/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.ts b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.ts index d7945e2e1..7afe6756f 100644 --- a/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.ts +++ b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.ts @@ -3,13 +3,26 @@ import { useEffect } from 'react'; import { Callback } from '../../../types'; -export default function useHandleRoomDisconnection(room: Room | null, onError: Callback) { +export default function useHandleRoomDisconnection( + room: Room | null, + onError: Callback, + removeLocalAudioTrack: () => void, + removeLocalVideoTrack: () => void, + isSharingScreen: boolean, + toggleScreenShare: () => void +) { useEffect(() => { if (room) { const onDisconnected = (_: Room, error: TwilioError) => { if (error) { onError(error); } + + removeLocalAudioTrack(); + removeLocalVideoTrack(); + if (isSharingScreen) { + toggleScreenShare(); + } }; room.on('disconnected', onDisconnected); @@ -17,5 +30,5 @@ export default function useHandleRoomDisconnection(room: Room | null, onError: C room.off('disconnected', onDisconnected); }; } - }, [room, onError]); + }, [room, onError, removeLocalAudioTrack, removeLocalVideoTrack, isSharingScreen, toggleScreenShare]); } From b444dba5159f6bcf256d7531aa8cd04513f6b156 Mon Sep 17 00:00:00 2001 From: Tim Mendoza Date: Wed, 10 Mar 2021 09:36:59 -0700 Subject: [PATCH 6/8] Fix linting issues --- .../useHandleRoomDisconnection.test.tsx | 4 ---- .../useScreenShareToggle/useScreenShareToggle.tsx | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx index a895f5032..18fe28c36 100644 --- a/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx +++ b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx @@ -32,7 +32,6 @@ describe('the useHandleRoomDisconnection hook', () => { }); it('should react to the rooms "disconnected" event and invoke onError callback if there is an error', () => { - const mockOnError = jest.fn(); renderHook(() => useHandleRoomDisconnection( mockRoom, @@ -50,7 +49,6 @@ describe('the useHandleRoomDisconnection hook', () => { }); it('should remove local tracks when the "disconnected" event is emitted', () => { - const mockOnError = jest.fn(); renderHook(() => useHandleRoomDisconnection( mockRoom, @@ -69,7 +67,6 @@ describe('the useHandleRoomDisconnection hook', () => { }); it('should not toggle screensharing when the "disconnected" event is emitted and isSharing is false', () => { - const mockOnError = jest.fn(); renderHook(() => useHandleRoomDisconnection( mockRoom, @@ -87,7 +84,6 @@ describe('the useHandleRoomDisconnection hook', () => { }); it('should toggle screensharing when the "disconnected" event is emitted and isSharing is true', () => { - const mockOnError = jest.fn(); renderHook(() => useHandleRoomDisconnection( mockRoom, diff --git a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx index 7cf95ddd2..9a3b3b66d 100644 --- a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx +++ b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.tsx @@ -1,4 +1,4 @@ -import { useState, useCallback, useRef, useEffect } from 'react'; +import { useState, useCallback, useRef } from 'react'; import { LogLevels, Track, Room } from 'twilio-video'; import { ErrorCallback } from '../../../types'; From b34bb3257bdf06efe843cd41e838d4b5b6dc5fae Mon Sep 17 00:00:00 2001 From: Tim Mendoza Date: Wed, 10 Mar 2021 09:42:58 -0700 Subject: [PATCH 7/8] Formatting and comment --- src/components/VideoProvider/index.tsx | 2 +- .../useHandleRoomDisconnection.test.tsx | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/VideoProvider/index.tsx b/src/components/VideoProvider/index.tsx index 82c229cac..8d29a1402 100644 --- a/src/components/VideoProvider/index.tsx +++ b/src/components/VideoProvider/index.tsx @@ -66,7 +66,7 @@ export function VideoProvider({ options, children, onError = () => {} }: VideoPr const [isSharingScreen, toggleScreenShare] = useScreenShareToggle(room, onError); - // Register onError callback functions. + // Register callback functions to be called on room disconnect. useHandleRoomDisconnection( room, onError, diff --git a/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx index 18fe28c36..742c6ad03 100644 --- a/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx +++ b/src/components/VideoProvider/useHandleRoomDisconnection/useHandleRoomDisconnection.test.tsx @@ -1,8 +1,7 @@ import { act, renderHook } from '@testing-library/react-hooks'; +import { Room } from 'twilio-video'; import EventEmitter from 'events'; - import useHandleRoomDisconnection from './useHandleRoomDisconnection'; -import { Room } from 'twilio-video'; const mockOnError = jest.fn(); const mockRemoveLocalAudioTrack = jest.fn(); From b3185abdd239853f933205d66afd40d8cdca3861 Mon Sep 17 00:00:00 2001 From: Tim Mendoza Date: Wed, 10 Mar 2021 09:43:59 -0700 Subject: [PATCH 8/8] Undo change to useScreenShareToggle --- .../useScreenShareToggle/useScreenShareToggle.test.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx index 1760e2a7e..a994c71db 100644 --- a/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx +++ b/src/components/VideoProvider/useScreenShareToggle/useScreenShareToggle.test.tsx @@ -7,8 +7,9 @@ const mockLocalParticipant = new EventEmitter() as any; mockLocalParticipant.publishTrack = jest.fn(() => Promise.resolve('mockPublication')); mockLocalParticipant.unpublishTrack = jest.fn(); -const mockRoom = new EventEmitter() as any; -mockRoom.localParticipant = mockLocalParticipant; +const mockRoom = { + localParticipant: mockLocalParticipant, +} as any; const mockOnError: ErrorCallback = () => {};