Skip to content

Commit

Permalink
Change room type (#434)
Browse files Browse the repository at this point in the history
* Change type of room so that it can be null

* Fix tests

* Remove unused eventemitter import

* Delete unused useHandleOnDisconnect hook

* Update useDominantSpeaker so it returns null when there is no room

* Update more tests

* Update more tests

* Fix test

* Fix lint issue

* Remove more references to onDisconnect
  • Loading branch information
timmydoza authored Feb 25, 2021
1 parent ed2f029 commit c118c98
Show file tree
Hide file tree
Showing 28 changed files with 168 additions and 196 deletions.
2 changes: 1 addition & 1 deletion src/components/Buttons/EndCallButton/EndCallButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default function EndCallButton(props: { className?: string }) {
}
removeLocalAudioTrack();
removeLocalVideoTrack();
room.disconnect();
room!.disconnect();
};

return (
Expand Down
5 changes: 2 additions & 3 deletions src/components/MainParticipant/MainParticipant.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ import useVideoContext from '../../hooks/useVideoContext/useVideoContext';

export default function MainParticipant() {
const mainParticipant = useMainParticipant();
const {
room: { localParticipant },
} = useVideoContext();
const { room } = useVideoContext();
const localParticipant = room!.localParticipant;
const [selectedParticipant] = useSelectedParticipant();
const screenShareParticipant = useScreenShareParticipant();

Expand Down
5 changes: 2 additions & 3 deletions src/components/MainParticipantInfo/MainParticipantInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,8 @@ interface MainParticipantInfoProps {

export default function MainParticipantInfo({ participant, children }: MainParticipantInfoProps) {
const classes = useStyles();
const {
room: { localParticipant },
} = useVideoContext();
const { room } = useVideoContext();
const localParticipant = room!.localParticipant;
const isLocal = localParticipant === participant;

const screenShareParticipant = useScreenShareParticipant();
Expand Down
2 changes: 1 addition & 1 deletion src/components/MenuBar/MenuBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export default function MenuBar() {
<Grid container justify="space-around" alignItems="center">
<Hidden smDown>
<Grid style={{ flex: 1 }}>
<Typography variant="body1">{room.name}</Typography>
<Typography variant="body1">{room!.name}</Typography>
</Grid>
</Hidden>
<Grid item>
Expand Down
2 changes: 1 addition & 1 deletion src/components/MobileTopMenuBar/MobileTopMenuBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export default function MobileTopMenuBar() {

return (
<Grid container alignItems="center" justify="space-between" className={classes.container}>
<Typography variant="subtitle1">{room.name}</Typography>
<Typography variant="subtitle1">{room!.name}</Typography>
<div>
<EndCallButton className={classes.endCallButton} />
<Menu buttonClassName={classes.settingsButton} />
Expand Down
5 changes: 2 additions & 3 deletions src/components/ParticipantList/ParticipantList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ const useStyles = makeStyles((theme: Theme) =>

export default function ParticipantList() {
const classes = useStyles();
const {
room: { localParticipant },
} = useVideoContext();
const { room } = useVideoContext();
const localParticipant = room!.localParticipant;
const participants = useParticipants();
const [selectedParticipant, setSelectedParticipant] = useSelectedParticipant();
const screenShareParticipant = useScreenShareParticipant();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,18 @@ import AttachVisibilityHandler from './AttachVisibilityHandler';
import useLocalVideoToggle from '../../../hooks/useLocalVideoToggle/useLocalVideoToggle';
import { render } from '@testing-library/react';
import * as utils from '../../../utils';
import useVideoContext from '../../../hooks/useVideoContext/useVideoContext';

jest.mock('../../../hooks/useVideoContext/useVideoContext', () => () => ({ room: {} }));
jest.mock('../../../hooks/useVideoContext/useVideoContext');
jest.mock('../../../hooks/useLocalVideoToggle/useLocalVideoToggle');

const mockUseVideoContext = useVideoContext as jest.Mock<any>;
const mockUseLocalVideoToggle = useLocalVideoToggle as jest.Mock<any>;
const mockToggleVideoEnabled = jest.fn();

Object.defineProperty(document, 'visibilityState', { value: '', writable: true });
mockUseLocalVideoToggle.mockImplementation(() => [true, mockToggleVideoEnabled]);
mockUseVideoContext.mockImplementation(() => ({ room: {} }));

describe('the AttachVisibilityHandler component', () => {
describe('when isMobile is false', () => {
Expand Down Expand Up @@ -70,5 +73,12 @@ describe('the AttachVisibilityHandler component', () => {
document.dispatchEvent(new Event('visibilitychange'));
expect(mockToggleVideoEnabled).not.toHaveBeenCalled();
});

it('should not add a visibilitychange event handler to the document when a room does not exist', () => {
mockUseVideoContext.mockImplementationOnce(() => ({}));
jest.spyOn(document, 'addEventListener');
render(<AttachVisibilityHandler />);
expect(document.addEventListener).not.toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export default function AttachVisibilityHandler() {
const shouldRepublishVideoOnForeground = useRef(false);

useEffect(() => {
if (isMobile) {
if (room && isMobile) {
const handleVisibilityChange = () => {
// We don't need to unpublish the local video track if it has already been unpublished
if (document.visibilityState === 'hidden' && isVideoEnabled) {
Expand Down
10 changes: 2 additions & 8 deletions src/components/VideoProvider/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ import useLocalTracks from './useLocalTracks/useLocalTracks';
import useRoom from './useRoom/useRoom';
import useHandleRoomDisconnectionErrors from './useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors';
import useHandleTrackPublicationFailed from './useHandleTrackPublicationFailed/useHandleTrackPublicationFailed';
import useHandleOnDisconnect from './useHandleOnDisconnect/useHandleOnDisconnect';
import useVideoContext from '../../hooks/useVideoContext/useVideoContext';

const mockRoom = new EventEmitter() as Room;
const mockOnDisconnect = jest.fn();
jest.mock('./useRoom/useRoom', () => jest.fn(() => ({ room: mockRoom, isConnecting: false, connect: () => {} })));
jest.mock('./useLocalTracks/useLocalTracks', () =>
jest.fn(() => ({
Expand All @@ -24,13 +22,11 @@ jest.mock('./useLocalTracks/useLocalTracks', () =>
);
jest.mock('./useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors');
jest.mock('./useHandleTrackPublicationFailed/useHandleTrackPublicationFailed');
jest.mock('./useHandleTrackPublicationFailed/useHandleTrackPublicationFailed');
jest.mock('./useHandleOnDisconnect/useHandleOnDisconnect');

describe('the VideoProvider component', () => {
it('should correctly return the Video Context object', () => {
const wrapper: React.FC = ({ children }) => (
<VideoProvider onError={() => {}} onDisconnect={mockOnDisconnect} options={{ dominantSpeaker: true }}>
<VideoProvider onError={() => {}} options={{ dominantSpeaker: true }}>
{children}
</VideoProvider>
);
Expand All @@ -41,7 +37,6 @@ describe('the VideoProvider component', () => {
room: mockRoom,
onError: expect.any(Function),
connect: expect.any(Function),
onDisconnect: mockOnDisconnect,
getLocalVideoTrack: expect.any(Function),
getLocalAudioTrack: expect.any(Function),
removeLocalVideoTrack: expect.any(Function),
Expand All @@ -54,13 +49,12 @@ describe('the VideoProvider component', () => {
expect(useLocalTracks).toHaveBeenCalled();
expect(useHandleRoomDisconnectionErrors).toHaveBeenCalledWith(mockRoom, expect.any(Function));
expect(useHandleTrackPublicationFailed).toHaveBeenCalledWith(mockRoom, expect.any(Function));
expect(useHandleOnDisconnect).toHaveBeenCalledWith(mockRoom, mockOnDisconnect);
});

it('should call the onError function when there is an error', () => {
const mockOnError = jest.fn();
const wrapper: React.FC = ({ children }) => (
<VideoProvider onError={mockOnError} onDisconnect={mockOnDisconnect} options={{ dominantSpeaker: true }}>
<VideoProvider onError={mockOnError} options={{ dominantSpeaker: true }}>
{children}
</VideoProvider>
);
Expand Down
13 changes: 4 additions & 9 deletions src/components/VideoProvider/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ import {
Room,
TwilioError,
} from 'twilio-video';
import { Callback, ErrorCallback } from '../../types';
import { ErrorCallback } from '../../types';
import { SelectedParticipantProvider } from './useSelectedParticipant/useSelectedParticipant';

import AttachVisibilityHandler from './AttachVisibilityHandler/AttachVisibilityHandler';
import useHandleRoomDisconnectionErrors from './useHandleRoomDisconnectionErrors/useHandleRoomDisconnectionErrors';
import useHandleOnDisconnect from './useHandleOnDisconnect/useHandleOnDisconnect';
import useHandleTrackPublicationFailed from './useHandleTrackPublicationFailed/useHandleTrackPublicationFailed';
import useLocalTracks from './useLocalTracks/useLocalTracks';
import useRoom from './useRoom/useRoom';
Expand All @@ -26,12 +25,11 @@ import useScreenShareToggle from './useScreenShareToggle/useScreenShareToggle';
*/

export interface IVideoContext {
room: Room;
room: Room | null;
localTracks: (LocalAudioTrack | LocalVideoTrack)[];
isConnecting: boolean;
connect: (token: string) => Promise<void>;
onError: ErrorCallback;
onDisconnect: Callback;
getLocalVideoTrack: (newOptions?: CreateLocalTrackOptions) => Promise<LocalVideoTrack>;
getLocalAudioTrack: (deviceId?: string) => Promise<LocalAudioTrack>;
isAcquiringLocalTracks: boolean;
Expand All @@ -47,11 +45,10 @@ export const VideoContext = createContext<IVideoContext>(null!);
interface VideoProviderProps {
options?: ConnectOptions;
onError: ErrorCallback;
onDisconnect?: Callback;
children: ReactNode;
}

export function VideoProvider({ options, children, onError = () => {}, onDisconnect = () => {} }: VideoProviderProps) {
export function VideoProvider({ options, children, onError = () => {} }: VideoProviderProps) {
const onErrorCallback = (error: TwilioError) => {
console.log(`ERROR: ${error.message}`, error);
onError(error);
Expand All @@ -68,10 +65,9 @@ export function VideoProvider({ options, children, onError = () => {}, onDisconn
} = useLocalTracks();
const { room, isConnecting, connect } = useRoom(localTracks, onErrorCallback, options);

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

return (
Expand All @@ -81,7 +77,6 @@ export function VideoProvider({ options, children, onError = () => {}, onDisconn
localTracks,
isConnecting,
onError: onErrorCallback,
onDisconnect,
getLocalVideoTrack,
getLocalAudioTrack,
connect,
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,19 @@ import { useEffect } from 'react';

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

export default function useHandleRoomDisconnectionErrors(room: Room, onError: Callback) {
export default function useHandleRoomDisconnectionErrors(room: Room | null, onError: Callback) {
useEffect(() => {
const onDisconnected = (_: Room, error: TwilioError) => {
if (error) {
onError(error);
}
};
if (room) {
const onDisconnected = (_: Room, error: TwilioError) => {
if (error) {
onError(error);
}
};

room.on('disconnected', onDisconnected);
return () => {
room.off('disconnected', onDisconnected);
};
room.on('disconnected', onDisconnected);
return () => {
room.off('disconnected', onDisconnected);
};
}
}, [room, onError]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ import { useEffect } from 'react';

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

export default function useHandleTrackPublicationFailed(room: Room, onError: Callback) {
const { localParticipant } = room;
export default function useHandleTrackPublicationFailed(room: Room | null, onError: Callback) {
useEffect(() => {
if (localParticipant) {
localParticipant.on('trackPublicationFailed', onError);
if (room) {
room.localParticipant.on('trackPublicationFailed', onError);
return () => {
localParticipant.off('trackPublicationFailed', onError);
room.localParticipant.off('trackPublicationFailed', onError);
};
}
}, [localParticipant, onError]);
}, [room, onError]);
}
20 changes: 7 additions & 13 deletions src/components/VideoProvider/useRoom/useRoom.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { act, renderHook } from '@testing-library/react-hooks';
import EventEmitter from 'events';
import { mockRoom } from '../../../__mocks__/twilio-video';
import useRoom from './useRoom';
import Video, { LocalTrack } from 'twilio-video';
Expand All @@ -11,11 +10,6 @@ describe('the useRoom hook', () => {
beforeEach(jest.clearAllMocks);
afterEach(() => mockRoom.removeAllListeners());

it('should return an empty room when no token is provided', () => {
const { result } = renderHook(() => useRoom([], () => {}, {}));
expect(result.current.room).toEqual(new EventEmitter());
});

it('should set isConnecting to true while connecting to the room ', async () => {
const { result, waitForNextUpdate } = renderHook(() => useRoom([], () => {}, {}));
expect(result.current.isConnecting).toBe(false);
Expand All @@ -25,7 +19,7 @@ describe('the useRoom hook', () => {
expect(result.current.isConnecting).toBe(true);
await waitForNextUpdate();
expect(Video.connect).toHaveBeenCalledTimes(1);
expect(result.current.room.disconnect).not.toHaveBeenCalled();
expect(result.current.room!.disconnect).not.toHaveBeenCalled();
expect(result.current.isConnecting).toBe(false);
});

Expand All @@ -44,7 +38,7 @@ describe('the useRoom hook', () => {
result.current.connect('token');
});
await waitForNextUpdate();
expect(result.current.room.state).toEqual('connected');
expect(result.current.room!.state).toEqual('connected');
});

it('should add a listener for the "beforeUnload" event when connected to a room', async () => {
Expand All @@ -64,7 +58,7 @@ describe('the useRoom hook', () => {
result.current.connect('token');
});
await waitForNextUpdate();
result.current.room.emit('disconnected');
result.current.room!.emit('disconnected');
await waitForNextUpdate();
expect(window.removeEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function));
});
Expand All @@ -84,10 +78,10 @@ describe('the useRoom hook', () => {
result.current.connect('token');
});
await waitForNextUpdate();
expect(result.current.room.state).toBe('connected');
result.current.room.emit('disconnected');
expect(result.current.room!.state).toBe('connected');
result.current.room!.emit('disconnected');
await waitForNextUpdate();
expect(result.current.room.state).toBe(undefined);
expect(result.current.room).toBe(null);
});

describe('when isMobile is true', () => {
Expand All @@ -111,7 +105,7 @@ describe('the useRoom hook', () => {
result.current.connect('token');
});
await waitForNextUpdate();
result.current.room.emit('disconnected');
result.current.room!.emit('disconnected');
await waitForNextUpdate();
expect(window.removeEventListener).toHaveBeenCalledWith('pagehide', expect.any(Function));
});
Expand Down
Loading

0 comments on commit c118c98

Please sign in to comment.