Ahoyapps 980 switch camera crash fix (#412)

* Fix settings menu alignment

* Update VideoInputList so that it doesnt crash when the deivce is changed while the camera is off.

* Add missing dependency to hook

src/__mocks__/twilio-video.ts

@@ -30,6 +30,7 @@ const mockPreflightTest = new MockPreflightTest();
 const twilioVideo = {
   connect: jest.fn(() => Promise.resolve(mockRoom)),
   createLocalTracks: jest.fn(() => Promise.resolve([new MockTrack('video'), new MockTrack('audio')])),
+  createLocalVideoTrack: jest.fn(() => Promise.resolve(new MockTrack('video'))),
   testPreflight: jest.fn(() => mockPreflightTest),
 };
 

src/components/DeviceSelectionDialog/VideoInputList/VideoInputList.test.tsx

@@ -98,4 +98,16 @@ describe('the VideoInputList component', () => {
       deviceId: { exact: 'mockDeviceID' },
     });
   });
+
+  it('should not call track.restart when no video track is present', () => {
+    mockUseDevices.mockImplementation(() => ({ videoInputDevices: [mockDevice, mockDevice] }));
+    mockUseVideoContext.mockImplementationOnce(() => ({
+      room: {},
+      getLocalVideoTrack: mockGetLocalVideotrack,
+      localTracks: [],
+    }));
+    const wrapper = shallow(<VideoInputList />);
+    wrapper.find(Select).simulate('change', { target: { value: 'mockDeviceID' } });
+    expect(mockLocalTrack.restart).not.toHaveBeenCalled();
+  });
 });

src/components/DeviceSelectionDialog/VideoInputList/VideoInputList.tsx

@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useState } from 'react';
 import { DEFAULT_VIDEO_CONSTRAINTS, SELECTED_VIDEO_INPUT_KEY } from '../../../constants';
 import { FormControl, MenuItem, Typography, Select } from '@material-ui/core';
 import { LocalVideoTrack } from 'twilio-video';
@@ -24,13 +24,19 @@ export default function VideoInputList() {
   const { videoInputDevices } = useDevices();
   const { localTracks } = useVideoContext();
 
-  const localVideoTrack = localTracks.find(track => track.kind === 'video') as LocalVideoTrack;
+  const localVideoTrack = localTracks.find(track => track.kind === 'video') as LocalVideoTrack | undefined;
   const mediaStreamTrack = useMediaStreamTrack(localVideoTrack);
-  const localVideoInputDeviceId = mediaStreamTrack?.getSettings().deviceId;
+  const [storedLocalVideoDeviceId, setStoredLocalVideoDeviceId] = useState(
+    window.localStorage.getItem(SELECTED_VIDEO_INPUT_KEY)
+  );
+  const localVideoInputDeviceId = mediaStreamTrack?.getSettings().deviceId || storedLocalVideoDeviceId;
 
   function replaceTrack(newDeviceId: string) {
+    // Here we store the device ID in the component state. This is so we can re-render this component display
+    // to display the name of the selected device when it is changed while the users camera is off.
+    setStoredLocalVideoDeviceId(newDeviceId);
     window.localStorage.setItem(SELECTED_VIDEO_INPUT_KEY, newDeviceId);
-    localVideoTrack.restart({
+    localVideoTrack?.restart({
       ...(DEFAULT_VIDEO_CONSTRAINTS as {}),
       deviceId: { exact: newDeviceId },
     });

src/components/PreJoinScreens/DeviceSelectionScreen/SettingsMenu/SettingsMenu.tsx

@@ -12,11 +12,11 @@ import DeviceSelectionDialog from '../../../DeviceSelectionDialog/DeviceSelectio
 import SettingsIcon from '../../../../icons/SettingsIcon';
 import { useAppState } from '../../../../state';
 
-const useStyles = makeStyles((theme: Theme) => ({
+const useStyles = makeStyles({
   settingsButton: {
     margin: '1.8em 0 0',
   },
-}));
+});
 
 export default function SettingsMenu({ mobileButtonClass }: { mobileButtonClass?: string }) {
   const classes = useStyles();
@@ -56,7 +56,7 @@ export default function SettingsMenu({ mobileButtonClass }: { mobileButtonClass?
         anchorEl={anchorRef.current}
         getContentAnchorEl={null}
         anchorOrigin={{
-          vertical: 'bottom',
+          vertical: 'top',
           horizontal: isMobile ? 'left' : 'right',
         }}
         transformOrigin={{

src/components/VideoProvider/useLocalTracks/useLocalTracks.test.tsx

@@ -1,5 +1,5 @@
 import { act, renderHook } from '@testing-library/react-hooks';
-import { SELECTED_AUDIO_INPUT_KEY, SELECTED_VIDEO_INPUT_KEY } from '../../../constants';
+import { SELECTED_AUDIO_INPUT_KEY, SELECTED_VIDEO_INPUT_KEY, DEFAULT_VIDEO_CONSTRAINTS } from '../../../constants';
 import useLocalTracks from './useLocalTracks';
 import Video from 'twilio-video';
 import useDevices from '../../../hooks/useDevices/useDevices';
@@ -239,4 +239,53 @@ describe('the useLocalTracks hook', () => {
       expect(initialAudioTrack!.stop).toHaveBeenCalled();
     });
   });
+
+  describe('the getLocalVideoTrack function', () => {
+    it('should create a local video track', async () => {
+      const { result, waitForNextUpdate } = renderHook(useLocalTracks);
+
+      await act(async () => {
+        result.current.getLocalVideoTrack();
+        await waitForNextUpdate();
+      });
+
+      expect(Video.createLocalVideoTrack).toHaveBeenCalledWith({
+        ...(DEFAULT_VIDEO_CONSTRAINTS as {}),
+        name: 'camera-123456',
+      });
+    });
+
+    it('should not specify a device ID when the device ID stored in local storage does not exist', async () => {
+      const { result, waitForNextUpdate } = renderHook(useLocalTracks);
+
+      window.localStorage.setItem(SELECTED_VIDEO_INPUT_KEY, 'device-id-does-not-exist');
+
+      await act(async () => {
+        result.current.getLocalVideoTrack();
+        await waitForNextUpdate();
+      });
+
+      expect(Video.createLocalVideoTrack).toHaveBeenCalledWith({
+        ...(DEFAULT_VIDEO_CONSTRAINTS as {}),
+        name: 'camera-123456',
+      });
+    });
+
+    it('should specify a device ID when one is stored in local storage and the device exists', async () => {
+      const { result, waitForNextUpdate } = renderHook(useLocalTracks);
+
+      window.localStorage.setItem(SELECTED_VIDEO_INPUT_KEY, 'mockVideoDeviceId');
+
+      await act(async () => {
+        result.current.getLocalVideoTrack();
+        await waitForNextUpdate();
+      });
+
+      expect(Video.createLocalVideoTrack).toHaveBeenCalledWith({
+        ...(DEFAULT_VIDEO_CONSTRAINTS as {}),
+        name: 'camera-123456',
+        deviceId: { exact: 'mockVideoDeviceId' },
+      });
+    });
+  });
 });

src/components/VideoProvider/useLocalTracks/useLocalTracks.ts

@@ -22,18 +22,24 @@ export default function useLocalTracks() {
     });
   }, []);
 
-  const getLocalVideoTrack = useCallback((newOptions?: CreateLocalTrackOptions) => {
+  const getLocalVideoTrack = useCallback(() => {
+    const selectedVideoDeviceId = window.localStorage.getItem(SELECTED_VIDEO_INPUT_KEY);
+
+    const hasSelectedVideoDevice = videoInputDevices.some(
+      device => selectedVideoDeviceId && device.deviceId === selectedVideoDeviceId
+    );
+
     const options: CreateLocalTrackOptions = {
       ...(DEFAULT_VIDEO_CONSTRAINTS as {}),
       name: `camera-${Date.now()}`,
-      ...newOptions,
+      ...(hasSelectedVideoDevice && { deviceId: { exact: selectedVideoDeviceId! } }),
     };
 
     return Video.createLocalVideoTrack(options).then(newTrack => {
       setVideoTrack(newTrack);
       return newTrack;
     });
-  }, []);
+  }, [videoInputDevices]);
 
   const removeLocalAudioTrack = useCallback(() => {
     if (audioTrack) {

src/hooks/useLocalVideoToggle/useLocalVideoToggle.test.tsx

@@ -155,37 +155,5 @@ describe('the useLocalVideoToggle hook', () => {
       await waitForNextUpdate();
       expect(mockOnError).toHaveBeenCalledWith('mockError');
     });
-
-    it('should call getLocalVideoTrack with the deviceId of the previously active track', async () => {
-      const mockGetLocalVideoTrack = jest.fn(() => Promise.resolve('mockTrack'));
-
-      mockUseVideoContext.mockImplementation(() => ({
-        localTracks: [getMockTrack('camera', 'testDeviceID')],
-        room: { localParticipant: null },
-        removeLocalVideoTrack: jest.fn(),
-        getLocalVideoTrack: mockGetLocalVideoTrack,
-      }));
-
-      const { result, rerender, waitForNextUpdate } = renderHook(useLocalVideoToggle);
-
-      // Remove existing track
-      result.current[1]();
-
-      mockUseVideoContext.mockImplementation(() => ({
-        localTracks: [],
-        room: { localParticipant: null },
-        removeLocalVideoTrack: jest.fn(),
-        getLocalVideoTrack: mockGetLocalVideoTrack,
-      }));
-      rerender();
-
-      await act(async () => {
-        // Get new video track
-        result.current[1]();
-        await waitForNextUpdate();
-      });
-
-      expect(mockGetLocalVideoTrack).toHaveBeenCalledWith({ deviceId: { exact: 'testDeviceID' } });
-    });
   });
 });

src/hooks/useLocalVideoToggle/useLocalVideoToggle.tsx

@@ -1,5 +1,5 @@
 import { LocalVideoTrack } from 'twilio-video';
-import { useCallback, useRef, useState } from 'react';
+import { useCallback, useState } from 'react';
 import useVideoContext from '../useVideoContext/useVideoContext';
 
 export default function useLocalVideoToggle() {
@@ -12,19 +12,17 @@ export default function useLocalVideoToggle() {
   } = useVideoContext();
   const videoTrack = localTracks.find(track => track.name.includes('camera')) as LocalVideoTrack;
   const [isPublishing, setIspublishing] = useState(false);
-  const previousDeviceIdRef = useRef<string>();
 
   const toggleVideoEnabled = useCallback(() => {
     if (!isPublishing) {
       if (videoTrack) {
-        previousDeviceIdRef.current = videoTrack.mediaStreamTrack.getSettings().deviceId;
         const localTrackPublication = localParticipant?.unpublishTrack(videoTrack);
         // TODO: remove when SDK implements this event. See: https://issues.corp.twilio.com/browse/JSDK-2592
         localParticipant?.emit('trackUnpublished', localTrackPublication);
         removeLocalVideoTrack();
       } else {
         setIspublishing(true);
-        getLocalVideoTrack({ deviceId: { exact: previousDeviceIdRef.current } })
+        getLocalVideoTrack()
           .then((track: LocalVideoTrack) => localParticipant?.publishTrack(track, { priority: 'low' }))
           .catch(onError)
           .finally(() => setIspublishing(false));