-
Notifications
You must be signed in to change notification settings - Fork 0
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
[FE] feat: 방 입장 전 세팅 화면 구성 #48
Conversation
4287f99
to
7374767
Compare
stream을 받아서 해당 stream을 보여주는 Video 컴포넌트 구현 (#45)
Button.Full, Button.Default 구성 (#45)
Home에 Button컴포넌트 합성 적용
user의 Stream을 관리하는 useMedia 커스텀 훅 기능 구현 stream 생성 stream 영상 변경 stream 마이크 변경 stream 스피커 변경 (#45)
media목록을 보여주고 media를 결정하는 MediaSelector 구현 (#45)
Video와 MediaSelector를 보여주는 SettingVideo컴포넌트 구현 (#45)
방 입장 전 Setting을 할 수 있는 Setting컴포넌트 기능 구현 (#45)
useMedia muted, offVideo 함수 추가 (#45)
기본 stream track을 selected 된 상태로 렌더링하는 기능 구현 (#45)
ControlButton을 위한 svg 파일 추가
stream이 useEffect로 변경되어 micOff와 videoOff가 제대로 동작하지 않던 문제 함수 삭제 (#45)
audio, video Control Button추가 (#45)
props로 넘겨주기 위해서 useMedia 반환타입 명시
Video 음성 추가 조건부 muted처리 (#45)
mediaObject props로 내려주기 적용 (#45)
settings에 참여하기 전까지 socket연결을 하지 않는 기능 추가 준비창에서 설정한 video, audio를 P2P통신에 사용하는 기능 추가 (#45)
lint default Props관련 룰 해제
zustand (#45)
zustand를 사용해 전역상태인 useSpeaker생성 전역상태 speaker를 통해 모든 video에 대해서 speaker를 동일하게 설정 (#45)
useMedia에서 반환하는 setSpeaker 삭제 (#45)
전역 setSpeaker 적용 (#45)
변경된 speaker를 video에 설정하는 부분 구현 (#45)
e1fd8cb
to
3f96b15
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
const RTCConnections: Record<string, RTCPeerConnection> = {}; | ||
|
||
const useRoom = (roomId: string, localStream: MediaStream, isSetting: boolean) => { | ||
const [isConnect, setIsConnect] = useState(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제네릭 빠진 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const [isConnect, setIsConnect] = useState(false);
요부분은 타입추론이 되어서 제네릭을 안써주는게 더 좋다고 생각하는데 어떻게 생각하시나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다른 state는 달아두셨길래 통일성 있게 하는건 어떤가 싶어서 말씀드렸어요
setStreamList((prev) => { | ||
const newArray = [...prev].filter(({ id }) => id !== socketId); | ||
return [...newArray, { id: socketId, stream: e.streams[0] }]; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setStreamList((prev) => { | |
const newArray = [...prev].filter(({ id }) => id !== socketId); | |
return [...newArray, { id: socketId, stream: e.streams[0] }]; | |
}); | |
setStreamList((prev) => [...prev].filter(({ id }) => id !== socketId).push({ id: socketId, stream: e.streams[0] })); |
배열 메서드를 활용한다면 이렇게도 표현할 수 있을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow 한 수 배워갑니다
{[ | ||
[cameraList, setCamera], | ||
[micList, setMic], | ||
[speakerList, setSpeaker], | ||
].map( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어떤 것에 대한 map
인지 한 번에 알아볼 수 있도록 변수로 분리하는 것도 좋을 것 같아요.
현재로서는 어떤 역할을 하는지 한 번에 확인하기 어려운 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런식으로 개선해보았습니다
const selector = [
{ list: camera.list, setFunc: camera.setCamera },
{ list: mic.list, setFunc: mic.setMic },
{ list: speaker.list, setFunc: setSpeaker },
];
...
{selector.map(
({ list, setFunc }, i) =>
list && (
<MediaSelector
key={i}
stream={stream}
optionsData={list as MediaDeviceInfo[]}
setFunc={setFunc as React.Dispatch<React.SetStateAction<string>>}
/>
),
)}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다!
…xport처리 import.meta.env를 유지하되, test코드에서 모킹하여 사용할 수 있도록 상수형태로 export처리
videoRef.current에 setSinkId가 없는 경우 호출하지 않도록 처리 (#45)
Room 조건부 렌더링 테스트 추가 (#45)
useInput 커스텀훅 기능 테스트
useMedia 기능 테스트 navigator.mediaDevices를 기반으로 가져온 stream으로 userStream을 업데이트할 수 있다. enumerateDevice를 통해 cameraList, micList, speakerList를 업데이트한다. selectedCamera, selectedMic, speaker가 바뀌면 useEffect가 재실행되어 enumerateDevice를 업데이트한다. (#45)
map의 배열 이름이 모호하던 부분 개선
불필요한 파일 삭제
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
PR 설명
방 입장 전 세팅 화면 구성
✅ 완료한 기능 명세
📸 스크린샷
고민과 해결과정
PeerConnection객체
기존의 useRoom에서는 내부에 PeerConnection객체가 존재하고 있었다.
이 때문에 useRoom을 호출하는 Room컴포넌트가 리액트 생명주기에 의해 rerender되는경우, PeerConnection이 빈 객체로 초기화되는 경우가 발생했다.
useMedia에서는 stream을 useState와 setState를 통해 변경시키고, Video역시 해당 state의 영향을 받아 useEffect로 videoRef를 변경시키는 형태로 구현했기때문에 PeerConnection을 유지하면서 기능을 구현할 수 없었다.
이에 PeerConnection 객체를 useRoom의 상위스코프로 분리시켰다.
이로써 useRoom이 몇번 호출되어도 PeerConnections객체가 빈 객체가 될 일은 없어졌다.
영상 stream, 오디오 stream이 변하는경우
useRoom에서 localStream에 의존성을 걸어서 peerConnection에 보내는 track을 변경해주었다.
세팅이 끝나고, 참여버튼을 눌러야 socket연결 처리
마찬가지로 useRoom에서 isSetting을 props로 받고, 해당 props가 변경되면 socket연결을 시작했다.
import.meta.env와 jest
jest가 import.meta.env만 만나면 에러를 throw해서 해당 부분을 constants/env.ts에서 객체로 export시키고 모킹으로 처리해주었다.
이후테스트에서는
이렇게 사용할 수 있었다.
전역상태관리
speaker는 video마다 다르게 설정할 수 있었는데, video컴포넌트가 여러개이므로, speaker정보를 전역으로 관리하기로 했다.
recoil과 zustand중에 고민을 했다.
recoil은 atom을 상태 그대로 set하기위해서 간단하게 쓰는 경우에 사용하고, zustand는 state와 action을 분리하는 형태로 사용한다.
그런데 나중에 상태가 많아지게 되면 recoil도 결국 custom hook형태로 관리하게된다고 하고, 번들 사이즈가 zustand가 작기때문에 zustand를 택했다.
참고로 recoil은 2.17mb, zustand는 1.16kb 이었다.
여기를 참고했다.
react custom Hook테스트
여기를 참고했다.