Skip to content
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

[정성현] sprint10 #129

Conversation

jsh1147
Copy link
Collaborator

@jsh1147 jsh1147 commented Nov 2, 2024

요구사항

  • sprint9에서 멈춘 상태입니다ㅠ

주요 변경사항

  • media 전역상태화
  • media 결정 이후 데이터 패칭 수행

스크린샷

image

멘토에게

  • 진도가 많이 느립니다😅
  • 미디어 & 데이터 관련된 수정밖에 없긴.. 합니다😅

- 훅이 갖고 있던 패칭 로직을 컴포넌트에게 넘김
- useMediaQuery 훅을 useMedia & MediaContext로 전역화
- 미디어 결정 이후 데이터 패칭이 발생하도록 수정
- 자유게시판 베스트 게시글 CSS 구현
@jsh1147 jsh1147 requested a review from GANGYIKIM November 2, 2024 14:59
@jsh1147 jsh1147 self-assigned this Nov 2, 2024
@jsh1147 jsh1147 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Nov 2, 2024
Copy link
Collaborator

@GANGYIKIM GANGYIKIM left a comment

Choose a reason for hiding this comment

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

성현님 이번 스프린트 미션 고생하셨습니다~
스프린트 미션은 진행하신 만큼 제출하시면 되니 부담가지지 말고 편하게 코딩해주세요~

  • 저번 리뷰 반영해서 리팩하신것도 큰 변경사항이라고 생각합니다 😁

Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
이건 분리하기보다 mediaContext 파일 하단에 같이 있는게 더 좋을 것 같아요.
이렇게 작업하기 위해서는 결국 mediaContext를 export 하게되서 구지 useMedia 라는 커스텀훅을 이용해서 접근하지 않을 수 있으니까요~

Copy link
Collaborator

Choose a reason for hiding this comment

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

context 관련해서 아래 블로그 글 한번 읽어보세요~
https://velog.io/@velopert/react-context-tutorial

Comment on lines +15 to +23
const checkMedia = (width: number): MediaType => {
if (width >= 1200) return "PC";
if (width >= 768) return "TABLET";
return "MOBILE";
};

const handleWindowResize = () => {
setMedia(checkMedia(window.innerWidth));
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
가능하면 useEffect 내부의 함수 정의를 외부로 빼주세요~
아래처럼 코드를 입력하면 가독성에 더 좋습니다.

  const checkMedia = (width: number): MediaType => {
    if (width >= 1200) return "PC";
    if (width >= 768) return "TABLET";
    return "MOBILE";
  };

  const handleWindowResize = useCallback(() => {
    setMedia(checkMedia(window.innerWidth));
  }, []);

  useEffect(() => {
    handleWindowResize();

    window.addEventListener("resize", handleWindowResize);
    return () => {
      window.removeEventListener("resize", handleWindowResize);
    };
  }, [handleWindowResize]);

@GANGYIKIM GANGYIKIM merged commit fd7230b into codeit-bootcamp-frontend:Next-정성현 Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants