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 #76

Conversation

hyuk-dev
Copy link
Collaborator

@hyuk-dev hyuk-dev commented Jan 9, 2025

요구사항

  • 반응형
  • 상품 조회
  • 페이지네이션

https://helpful-trifle-e965b6.netlify.app/

@hyuk-dev hyuk-dev requested a review from junguksim January 10, 2025 02:03
@hyuk-dev hyuk-dev changed the title React 이동혁 sprint5 [이동혁] sprint5 Jan 10, 2025
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.

전체적으로 잘 짜주셨습니다! 동작 잘 하네요. ㅎㅎ
모듈화 관련 팁 남겨두었습니다!

if(windowWidth < 865) return 'mobileBestItemList';
}

useEffect(()=> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 반복되는 것은 customHook 으로 모듈화할 수 있습니다!
한 번 찾아보고 적용해보시면 좋을 것 같아요 :)

if(windowWidth < 865) setBestPageSize(1);
},[windowWidth])

async function getBestProducts() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

에러 처리가 되면 좋겠습니다!

const [bestPage,setBestPage] = useState(1);

function currentEnv() {
if(windowWidth >= 1200) return 'bestItemList';
Copy link
Collaborator

Choose a reason for hiding this comment

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

1200, 865,등의 값이 반복되므로, export + 변수 하나 설정해서 한번에 다 관리되도록 하면 더욱 좋겠네요.
PC_WIDTH, TABLET_WIDTH 이런 식으로요!

@junguksim junguksim merged commit c009f48 into codeit-sprint-fullstack:react-이동혁 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.

3 participants