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

[정한샘] sprint5 #80

Merged

Conversation

koseha
Copy link
Collaborator

@koseha koseha commented Jan 10, 2025

https://seha-pandamarket-react.netlify.app/

요구사항

기본

  • [x]

심화

  • [x]

주요 변경사항

스크린샷

seha-pandamarket-basic netlify app_ (3)

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

Copy link
Collaborator

@junguksim junguksim left a comment

Choose a reason for hiding this comment

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

잘 짜주셨습니다!
명명이 잘못된 게 있어서 하나랑, 사소한 팁 남겨뒀습니다.


const baseUrl = process.env.REACT_APP_API_URL;

const GetProducts = async ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

슥 보고 컴포넌트인줄 알았는데 함수네요!
컴포넌트는 PascalCase, 함수는 camelCase 를 지켜주세요!

@@ -0,0 +1,2 @@
export * from "./useMediaQuery";
Copy link
Collaborator

Choose a reason for hiding this comment

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

별도의 파일을 만들지 말고, useMediaQuery.js 에서 export default 를 사용해보세요!

const ProductBest = () => {
const [products, setProducts] = useState([]);
const media = hooks.useMediaQuery("");
const initColumn = media === "pc" ? 4 : media === "tablet" ? 2 : 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

initColumn 도 어차피 media 에 의해 정해지는 값이라면, useMediaQuery 커스텀 훅에서 같이 처리해서 리턴해도 좋을 것 같아요.

const {media, initColumn} = useMediaQuery()

이런식으로요~

@junguksim junguksim merged commit fba7940 into codeit-sprint-fullstack:main Jan 14, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants