Skip to content

Stop screensharing on disconnect #452

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

Merged
merged 8 commits into from
Mar 11, 2021
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
50 changes: 4 additions & 46 deletions src/components/Buttons/EndCallButton/EndCallButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,56 +7,14 @@ const mockVideoContext = {
room: {
disconnect: jest.fn(),
},
isSharingScreen: false,
toggleScreenShare: jest.fn(),
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();
mockVideoContext.isSharingScreen = true;
wrapper = shallow(<EndCallButton />);
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 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(<EndCallButton />);
wrapper.simulate('click');
});

it('should not toggle screen sharing', () => {
expect(mockVideoContext.toggleScreenShare).not.toHaveBeenCalled();
});
});
it('should disconnect from the room when clicked', () => {
const wrapper = shallow(<EndCallButton />);
wrapper.simulate('click');
expect(mockVideoContext.room.disconnect).toHaveBeenCalled();
});
});
13 changes: 2 additions & 11 deletions src/components/Buttons/EndCallButton/EndCallButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,10 @@ const useStyles = makeStyles((theme: Theme) =>

export default function EndCallButton(props: { className?: string }) {
const classes = useStyles();
const { room, isSharingScreen, toggleScreenShare, removeLocalAudioTrack, removeLocalVideoTrack } = useVideoContext();

const handleClick = () => {
if (isSharingScreen) {
toggleScreenShare();
}
removeLocalAudioTrack();
removeLocalVideoTrack();
room!.disconnect();
};
const { room } = useVideoContext();

return (
<Button onClick={handleClick} className={clsx(classes.button, props.className)} data-cy-disconnect>
<Button onClick={() => room!.disconnect()} className={clsx(classes.button, props.className)} data-cy-disconnect>
Disconnect
</Button>
);
Expand Down
14 changes: 11 additions & 3 deletions src/components/VideoProvider/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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', () => {
Expand All @@ -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));
});

Expand Down
18 changes: 12 additions & 6 deletions src/components/VideoProvider/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -33,7 +33,6 @@ export interface IVideoContext {
getLocalVideoTrack: (newOptions?: CreateLocalTrackOptions) => Promise<LocalVideoTrack>;
getLocalAudioTrack: (deviceId?: string) => Promise<LocalAudioTrack>;
isAcquiringLocalTracks: boolean;
removeLocalAudioTrack: () => void;
removeLocalVideoTrack: () => void;
isSharingScreen: boolean;
toggleScreenShare: () => void;
Expand Down Expand Up @@ -65,11 +64,19 @@ export function VideoProvider({ options, children, onError = () => {} }: VideoPr
} = useLocalTracks();
const { room, isConnecting, connect } = useRoom(localTracks, onErrorCallback, options);

// Register onError callback functions.
useHandleRoomDisconnectionErrors(room, onError);
useHandleTrackPublicationFailed(room, onError);
const [isSharingScreen, toggleScreenShare] = useScreenShareToggle(room, onError);

// Register callback functions to be called on room disconnect.
useHandleRoomDisconnection(
room,
onError,
removeLocalAudioTrack,
removeLocalVideoTrack,
isSharingScreen,
toggleScreenShare
);
useHandleTrackPublicationFailed(room, onError);

return (
<VideoContext.Provider
value={{
Expand All @@ -81,7 +88,6 @@ export function VideoProvider({ options, children, onError = () => {} }: VideoPr
getLocalAudioTrack,
connect,
isAcquiringLocalTracks,
removeLocalAudioTrack,
removeLocalVideoTrack,
isSharingScreen,
toggleScreenShare,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import { act, renderHook } from '@testing-library/react-hooks';
import { Room } from 'twilio-video';
import EventEmitter from 'events';
import useHandleRoomDisconnection from './useHandleRoomDisconnection';

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', () => {
renderHook(() =>
useHandleRoomDisconnection(
mockRoom,
mockOnError,
mockRemoveLocalAudioTrack,
mockRemoveLocalVideoTrack,
false,
mockToggleScreenSharing
)
);
act(() => {
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', () => {
renderHook(() =>
useHandleRoomDisconnection(
mockRoom,
mockOnError,
mockRemoveLocalAudioTrack,
mockRemoveLocalVideoTrack,
false,
mockToggleScreenSharing
)
);
act(() => {
mockRoom.emit('disconnected', mockRoom, 'mockError');
});
expect(mockOnError).toHaveBeenCalledWith('mockError');
});

it('should remove local tracks when the "disconnected" event is emitted', () => {
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', () => {
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', () => {
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,
mockOnError,
mockRemoveLocalAudioTrack,
mockRemoveLocalVideoTrack,
false,
mockToggleScreenSharing
)
);
expect(originalMockRoom.listenerCount('disconnected')).toBe(1);

act(() => {
mockRoom = new EventEmitter() as Room;
});

rerender();

expect(originalMockRoom.listenerCount('disconnected')).toBe(0);
expect(mockRoom.listenerCount('disconnected')).toBe(1);
});

it('should clean up listeners on unmount', () => {
const { unmount } = renderHook(() =>
useHandleRoomDisconnection(
mockRoom,
mockOnError,
mockRemoveLocalAudioTrack,
mockRemoveLocalVideoTrack,
false,
mockToggleScreenSharing
)
);
unmount();
expect(mockRoom.listenerCount('disconnected')).toBe(0);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { Room, TwilioError } from 'twilio-video';
import { useEffect } from 'react';

import { Callback } from '../../../types';

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);
return () => {
room.off('disconnected', onDisconnected);
};
}
}, [room, onError, removeLocalAudioTrack, removeLocalVideoTrack, isSharingScreen, toggleScreenShare]);
}

This file was deleted.

Loading