-
Notifications
You must be signed in to change notification settings - Fork 26
[권수형] Sprint5 #85
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 #85
The head ref may contain hidden characters: "Basic-\uAD8C\uC218\uD615-sprint5"
Conversation
…tion을 class 기반에서 함수기반으로 변경
…nationBar, DropdownButton 기능 구현
쓰로틀링을 추가한 resize hook을 이용하여 화면 사이즈마다 불러오는 item의 갯수를 다르게 설정
baeggmin
left a comment
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.
첫 리액트 미션이었음에도 불구하고 정말 깔끔하게 구현해주셨네요! 👍👍👍
크게 리뷰드릴 내용이 없어 간단하게 리뷰 남겼으니 확인 부탁드립니다. 고생많으셨어요. 😊
질문 주신 내용은 아래에 답변 남깁니다.
- css 토큰 자동완성 관련
말씀주신 JS 객체로 선언하고 css 함수로 변환하는 방법 좋습니다. 제 생각엔 그렇게 코드가 복잡해지진 않는 것 같아요. 굳이 단점을 찾자면 css 변수보단 덜 선언적이라 브라우저 devtool에서 식별하기 좀 어렵다는 점...? 정도일 것 같네요.
위 방법이 취향이 아니시라면 styled-component 의 ThemProvider 를 이용해보셔도 좋겠습니다. ThemeProvider 를 사용하면 주입하는 theme 을 기반으로 타입 생성이 되어서 자동완성이 됩니다.
- useIsMobile 의 적합성 관련
인라인 코멘트로 답변 남겼습니다.
https://awkward-panda-market.netlify.app/items
요구사항
기본
심화
주요 변경사항
customFetch.js를 통해 일괄적으로 fetch의 설정을 관리함useAsynchook을 통해 loading, error 처리 로직을 일괄적으로 관리함useResizeEffecthook을 통해서 화면 사이즈를 추적하고 사이즈에 따라 새로운 getProducts 요청을 300ms 간격으로 보냄useIsMobilehook을 통해 isMobile 상태를 받고, 모바일뷰를 관리함스크린샷
멘토에게
현재
/src/styles/global.js에서 전역 스타일 토큰을 정의하고,styled-component에서 var()를 통해 불러오는 방식으로 스타일을 관리하고 있습니다. 지금 방식은 var() 작성할때 토큰의 자동완성이 되지 않아 불편한 점이 있고, styled-components의 css 함수를 이용하거나, 토큰을 Js 변수로 선언하는 방식은 나름대로 코드가 복잡해보이기도 한 것 같습니다! 혹시 추천하시는 방법이 있을까요?현재 페이지가 모바일 사이즈인지 체크할때
useIsMobile과 같이 관리하는 것이 바람직한걸까요? css media로는 해결이 안되는 UI들이 있어서 작성했지만, 지금처럼 하게 되면 모든 페이지에서 resize 이벤트를 호출하게 될 것 같습니다.또 다른 개선점이 있다면 말씀 부탁드리겠습니다 🙇♂️