Skip to content

Conversation

@ShipFriend0516
Copy link
Member

@ShipFriend0516 ShipFriend0516 commented Nov 10, 2024

Note

작성자: 서정우
작성 날짜: 2024.11.10

화상 통화 페이지의 구현을 우선하여 개발했기 때문에 리팩토링 작업을 진행했습니다. 컴포넌트로 분리하거나 커스텀 훅으로 분리하는 등의 작업을 진행했습니다. 리팩토링 위주이기 때문에 약간의 스타일 디테일과 기능 향상을 목적으로 작업했습니다.

해당 작업을 통해 SessionPage 코드의 길이를 25% 줄일 수 있었습니다. 아직 더 개선의 여지가 있습니다.

관련 이슈 번호

#11

✅ 체크리스트

  • 코드가 정상적으로 작동하는지 확인했습니다.
  • 주요 변경사항에 대한 설명을 작성했습니다.
  • 코드 스타일 가이드에 따라 코드를 작성했습니다.

🧩 작업 내용

  • SessionPage 컴포넌트 내부 로직 커스텀 훅으로 분리
  • 미디어 장치 관련 로직 개선 및 실시간 장치 변경 기능 추가
  • UI 컴포넌트 구조 개선 및 스타일링 수정

📝 작업 상세 내역

컴포넌트 분리 및 커스텀 훅 구현

  • 상세 설명:
    • SessionPage의 Sidebar와 FooterToolbar를 컴포넌트로 분리
    • SessionPage의 복잡한 로직을 관심사 별로 분리하여 커스텀 훅으로 구현
    • 미디어 장치 관리, 소켓 처리 등을 별도 훅으로 분리
    • 각 훅의 책임을 명확히 하여 코드 재사용성과 유지보수성 향상
  • 구현된 커스텀 훅:
    - useMediaDevices: 미디어 장치 목록 관리 및 스트림 제어
    - useSocket: 시그널링 서버와의 소켓 연결 제어
    
useSocket
const useSocket = (socketURL: string) => {
  const [socket, setSocket] = useState<Socket | null>(null);

  useEffect(() => {
    const newSocket = io(socketURL || "http://localhost:3000");

    newSocket.on("connect_error", socketErrorHandler);
    setSocket(newSocket);

    return () => {
      newSocket.disconnect();
      setSocket(null);
    };
  }, [socketURL]);

  return { socket };
};

const socketErrorHandler = (error: Error) => {
  console.error("시그널링 서버와의 연결에 실패했습니다.", error);
};
useMediaDevices
const useMediaDevices = () => {
  // 유저의 미디어 장치 리스트
  const [userAudioDevices, setUserAudioDevices] = useState<MediaDeviceInfo[]>(
    []
  );
  const [userVideoDevices, setUserVideoDevices] = useState<MediaDeviceInfo[]>(
    []
  );

  // 유저가 선택한 미디어 장치
  const [selectedVideoDeviceId, setSelectedVideoDeviceId] =
    useState<string>("");
  const [selectedAudioDeviceId, setSelectedAudioDeviceId] =
    useState<string>("");

  // 본인 미디어 스트림
  const [stream, setStream] = useState<MediaStream | null>(null);

  useEffect(() => {
    // 비디오 디바이스 목록 가져오기

    const getUserDevices = async () => {
      try {
        const devices = await navigator.mediaDevices.enumerateDevices();
        const audioDevices = devices.filter(
          (device) => device.kind === "audioinput"
        );
        const videoDevices = devices.filter(
          (device) => device.kind === "videoinput"
        );

        setUserAudioDevices(audioDevices);
        setUserVideoDevices(videoDevices);
      } catch (error) {
        console.error("미디어 기기를 찾는데 문제가 발생했습니다.", error);
      }
    };

    getUserDevices();
  }, []);

  // 미디어 스트림 가져오기: 자신의 스트림을 가져옴
  const getMedia = async () => {
    try {
      if (stream) {
        // 이미 스트림이 있으면 종료
        stream.getTracks().forEach((track) => {
          track.stop();
        });
      }
      const myStream = await navigator.mediaDevices.getUserMedia({
        video: selectedVideoDeviceId
          ? { deviceId: selectedVideoDeviceId }
          : true,
        audio: selectedAudioDeviceId
          ? { deviceId: selectedAudioDeviceId }
          : true,
      });

      setStream(myStream);
      return myStream;
    } catch (error) {
      console.error("미디어 스트림을 가져오는 도중 문제가 발생했습니다.", error);
    }
  };

  return {
    userAudioDevices,
    userVideoDevices,
    selectedAudioDeviceId,
    selectedVideoDeviceId,
    setSelectedAudioDeviceId,
    setSelectedVideoDeviceId,
    getMedia,
    stream,
  };
};

미디어 장치 변경 기능 개선

  • 상세 설명:
    • 기존: 화상회의를 시작하면 미디어 장치를 변경해도 다시 연결해야 변경된 미디어 장치로 화상회의가 가능
    • 개선: 화상회의 도중에 미디어 장치 변경해도 바로 적용되도록 개선
    • 화상 회의 중 실시간으로 미디어 장치를 변경할 수 있도록 기능 추가

UI/UX 개선

  • 상세 설명:
    • 미디어 장치 선택 UI 개선
      스크린샷 2024-11-10 오후 3 53 35
    • 화면 크기 큰 화면 지원
    • 참여자 리스트를 오른쪽 사이드바에서 볼 수 있도록 지원
      image

📌 테스트 및 검증 결과

  • 기능 테스트:
    • 미디어 장치 간 전환 테스트

💬 다음 작업 또는 논의 사항

  • 화상 회의 중 반응(�일단은 따봉) 기능 추가 예정

🐥 리뷰 받고 싶은 포인트

  • 컴포넌트 분리를 어디에서 더 해볼 수 있을지

@ShipFriend0516 ShipFriend0516 self-assigned this Nov 10, 2024
@ShipFriend0516 ShipFriend0516 added the 🎁 Refactoring 리팩토링 label Nov 10, 2024
@ShipFriend0516 ShipFriend0516 requested review from a team, blu3fishez, twalla26 and yiseungyun and removed request for a team November 10, 2024 08:15
Copy link
Collaborator

@blu3fishez blu3fishez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretendard 폰트를 직접 가져오시는데 혹시 CDN 활용해보시는건 어떨까요??? (살려줘요..)

리팩토링 너무 잘 된 것 같습니다!! 따봉 5개 드리겠씁니다 ^__^ bbbbb

Comment on lines +10 to +19
interface Props {
handleVideoToggle: () => void;
handleMicToggle: () => void;
userVideoDevices: MediaDeviceInfo[];
userAudioDevices: MediaDeviceInfo[];
setSelectedVideoDeviceId: (deviceId: string) => void;
setSelectedAudioDeviceId: (deviceId: string) => void;
isVideoOn: boolean;
isMicOn: boolean;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인터페이스 구성 좋습니다!

제가 예엣날에 프론트 선배님(?)께 배운 방식은 컴포넌트Props 로 별도로 명세해서 추가적으로 어떤 프로퍼티인지 알려주는 식으로 구성하긴 했어요

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 컴포넌트를 예시로 들 경우 SessionToolbarProps 와 같은 형식이에용

Comment on lines +25 to +38
try {
const devices = await navigator.mediaDevices.enumerateDevices();
const audioDevices = devices.filter(
(device) => device.kind === "audioinput"
);
const videoDevices = devices.filter(
(device) => device.kind === "videoinput"
);

setUserAudioDevices(audioDevices);
setUserVideoDevices(videoDevices);
} catch (error) {
console.error("미디어 기기를 찾는데 문제가 발생했습니다.", error);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try - catch 문 사용 너무 좋습니다!

문제가 발생했을 때 유저에게 어떤 변화를 줄 지도 추후에 생각해보는 식으로 확장해나가면 좋겠네요!

useEffect(() => {
const newSocket = io(socketURL || "http://localhost:3000");

newSocket.on("connect_error", socketErrorHandler);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함수로 따로 빼주신 것 좋습니다!

blu3fishez
blu3fishez previously approved these changes Nov 10, 2024
@blu3fishez blu3fishez linked an issue Nov 10, 2024 that may be closed by this pull request
7 tasks
@blu3fishez blu3fishez removed a link to an issue Nov 10, 2024
7 tasks
yiseungyun
yiseungyun previously approved these changes Nov 10, 2024
Copy link
Member

@yiseungyun yiseungyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 리팩토링 하고 싶다 생각했었는데, 너무 좋습니다. 👍☃️👍
pretendard는 제가 디자인 작업하면서 @import url('https://cdn.jsdelivr.net/gh/orioncactus/pretendard/dist/web/static/pretendard.css');으로 가져오고 있어서 폰트 파일은 없어도 될거 같습니다!

진짜 수고 많으셨어요. ☃️

@yiseungyun
Copy link
Member

프로젝트에 따라 컴포넌트를 어떻게 분리하고 폴더 구조를 어떻게 설정할지는 정답이 다 다르다 생각하는데, [React] 실무용 폴더구조 container / presenter 글처럼 데이터 처리와 UI를 분리하는 걸 시도해보는 건 어떨까요?
저는 아직 한 번도 시도 안해봤어서 이렇게하면 좀 더 깔끔해질거 같습니다. 같이 시도해보면 좋을거 같긴해요! 🤔 아니면 다른 방법도 좋습니다.

twalla26
twalla26 previously approved these changes Nov 11, 2024
Copy link
Collaborator

@twalla26 twalla26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적인 리팩토링 작업을 하셨군요! LGTM!

Copy link
Collaborator

@blu3fishez blu3fishez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

흠.. 이렇게 된거 main 브랜치로 가는 것만 전원이 리뷰하도록 만드는 것도 좋을 것 같네요

@ShipFriend0516 ShipFriend0516 merged commit c3ad1a4 into boostcampwm-2024:dev Nov 11, 2024
@ShipFriend0516 ShipFriend0516 deleted the refactor/session-page branch November 12, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🎁 Refactoring 리팩토링

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants