-
Notifications
You must be signed in to change notification settings - Fork 50
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
[FE][Team19] Sidedish 1주차 PR #31
Conversation
[#1] 개발 환경 구축 완료
[#3] Header 구현 완료
- BestTab UI - 상수 파일 - const.js - 재사용 컴포넌트 - Label.jsx - ItemCard.jsx
[#5] feat: ✨ BestTab UI 구현
- App.jsx - SlideContainer import - BestItem.jsx - 삭제 (미사용) - BestItems.jsx - ItemCard prop 추가 - BestTab.jsx - 스타일 수정 - SlideContainer.jsx - UI 구현 - SlideItems.jsx - UI 구현 - SlideArrowBtn.jsx - UI 구현 - ItemCard.jsx - 스타일 수정 - prop 추가 - Label.jsx - prop 추가 - 기본 값 추가
- App.js - ShowMoreBtn import - ShowMoreBtn.jsx - UI 구현
[#7] feat: ✨ Slide UI 구현
- App.js - Header 경로 수정 - Header.jsx - 경로 변경 - HeaderLeft & Right 분리 - HeaderLeft.jsx - 컴포넌트화 - Navigations 컴포넌트화 - HeaderRight.jsx - 컴포넌트화 - Navigations.jsx - 컴포넌트화 - Dropdown 구현
[#10] feat: ✨ Header Dropdown 구현
- BestItems.jsx - API 데이터 동기화 - BestTab.jsx - useState, useEffect, API 요청 - BestTabContainer.jsx - API 데이터 동기화 - BestTabNavigator.jsx - API 데이터 동기화 - ItemCard.jsx - prop 변경 - Label.jsx - COLOR변수 추가
API 데이터 fetch 요청 + 베스트 반찬 tab 랜덤 출력
- App.js - PopUpContainer import - PopUpContainer.jsx - UI 구현 - PopUpImages.jsx - UI 구현 - PopUpInformations.jsx - UI 구현
[#15] feat: ✨ 상세 modal 페이지 UI 구현
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.
안녕하세요 리뷰를 맡은 Snow입니다 ☃️
깔끔하게 작성한 코드와 컴포넌트 구조 덕분에 읽는데 어려움이 없었습니다.
리뷰 코멘트 남긴 부분에 대한 의견 대부분 강력한 수정 필요! 보다는 의견 위주로 남겼습니다. 😃
Slider의 경우, 패키지로 배포하기에는 다소 아쉬운 점이 보입니다. 다른 라이브러리처럼 파라메터로 값을 넘겨주어 어느정도 커스텀이 가능하게 끔 만들어보는 것도 많은 도움이 될 것 같습니다. Material UI나 다른 Carousel 혹은 Slider과 같은 라이브러리를 참고하면 좋을 것 같습니다!
커밋 메세지가 간결하고 깔끔하여 아주 보기 좋군요 👍
<Header /> | ||
<BestTab /> | ||
<SlideContainer /> | ||
<ShowMoreBtn /> |
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.
Header이하의 컴포넌트들을 묶는 건 어떨까요 ? 혹은 MainPage와 같은 컴포넌트를 만들어 하위에 두는 것도 좋을 것 같습니다!
다른 페이지가 추가되는 경우 유용할 것 같습니다. 또한, 각 섹션별(BestTab
, SildeContainer
, ...) 간격을 조정하는데 좀 더 일괄적으로 다룰 수 있는 장점이 있을 것 같습니다
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.
Header이하의 컴포넌트들을 묶는 건 어떨까요 ? 혹은 MainPage와 같은 컴포넌트를 만들어 하위에 두는 것도 좋을 것 같습니다!
다른 페이지가 추가되는 경우 유용할 것 같습니다. 또한, 각 섹션별(
BestTab
,SildeContainer
, ...) 간격을 조정하는데 좀 더 일괄적으로 다룰 수 있는 장점이 있을 것 같습니다
Main 이라는 컴포넌트를 하나 만들어서 묶는 방식으로 수정하겠습니다!
🙏
outline:none; | ||
} | ||
* { | ||
box-sizing: border-box; |
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.
Reset css나 Normalize에 대해 한 번 학습해보면 좋을 것 같습니다!
Global Style ~ 👍
@@ -0,0 +1,15 @@ | |||
@import url("https://fonts.googleapis.com/css2?family=Noto+Sans+KR:wght@300;400;700;900&display=swap"); |
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.
display=swap을 통해 폰트가 로드되는 동안 글자가 표시되지 않는 이슈를 해결하셨군요 👀 👍
background-color: ${props => props.bgColor}; | ||
`; | ||
// #86C6FF, #82D32D | ||
const COLOR = { |
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.
한글 변수명! 처음봐요! 👍
혹시 이렇게 쓰신 이유가 있나요? 궁금해서 ! 여쭤보는겁니다!
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.
한글 변수명! 처음봐요! 👍
혹시 이렇게 쓰신 이유가 있나요? 궁금해서 ! 여쭤보는겁니다!
DB에서 label의 값을 한글 배열로 전달해주고 있어서 그 명칭에 맞춰서 작성했습니다!
그런데 생각해보니 그냥 영문이나 넘버로 처리하는게 어땠을까 싶네요..!
export default function SlideItems() { | ||
return ( | ||
<SlideItemsStyle> | ||
<ItemCard src = "http://web.archive.org/web/20190129014225im_/https://cdn.bmf.kr/_data/product/IF235/20ea0478c029ad4fda3fa2885d53c5d4.jpg" title = "[집밥의완성]초여름보양세트" description = "무더위에 대비하는 6월 한상차림 초여름 보양세트" salePrice="5000" normalPrice="6000" text="이벤트특가" color="#fff" bgColor="#82D32D"/> |
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.
Array와 map을 이용하면 좀 더 보기 좋을 것 같습니다!
임시인거죠? 🤔
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.
Array와 map을 이용하면 좀 더 보기 좋을 것 같습니다!
임시인거죠? 🤔
넵! 슬라이드 모듈을 만들기 전에 UI적인 처리만 해뒀습니다!
return ( | ||
<HeaderStyle> | ||
<HeaderLeft/> | ||
<HeaderRight/> |
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.
Left와 Right라는 모호한 명칭보다는 Navigation과 같은 해당 컴포넌트를 아우를 수 있는 단어를 쓰는 것을 권장드립니다
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.
Left와 Right라는 모호한 명칭보다는 Navigation과 같은 해당 컴포넌트를 아우를 수 있는 단어를 쓰는 것을 권장드립니다
Left와 Right에 들어가는 주요 컴포넌트로 이름을 변경하는게 더 명확하겠네요!
좋은 의견 감사합니다 👍
line-height: 1rem; | ||
} | ||
div:first-child { | ||
padding-bottom: 1rem; |
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.
이런방식으로 사용하게 되면 자식 Element 뿐만 아니라 자손 Element까지 적용될 것 같습니다. Child Selector과 descendant Selector에 대해 학습해보면 좋을 것 같습니다.
만약 decendant에 해당하는 모든 요소에 적용하기를 의도했다면 이 의견은 패스하셔도 좋습니다 😃
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.
descendant Selector에 대해 학습해보면 좋을 것 같습니다.
자손 요소가 없는 요소라 Child Selector를 적용하지 않았습니다!
비슷한 부분에서 자손 요소가 있는데 처리를 안해서 다시 수정한 기억이 떠오르네요 :)
좋은 의견 감사합니다!! 🥇
|
||
const SliderArrowBtnsStyle = styled.div` | ||
position: absolute; | ||
width: calc(100% + 7rem); |
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.
calc
를 활용한 배치가 인상적이군요 👍
이러한 방식도 좋지만 Slider의 크기와 화면 크기가 동일한 경우에는 해당 버튼들이 안보일 수도 있다는 문제가 생길 것 같습니다. 물론 반응형에 대한 기획과 개발은 없지만
이런 경우, Slider에 좌우 마진을 추가하고 그 자리에 버튼을 넣는 방식으로 구현하면 어느정도 큰 수정을 하지 않고도 반응형을 대응할 수 있을 것 같습니다. 이러한 방법에 대해 한 번 고민해보시고,
그래도 해결이 안된다면 좀 더 자세한 힌트와 함께 도와드리겠습니다. 주저없이 질문해주세요!
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.
calc
를 활용한 배치가 인상적이군요 👍이러한 방식도 좋지만 Slider의 크기와 화면 크기가 동일한 경우에는 해당 버튼들이 안보일 수도 있다는 문제가 생길 것 같습니다. 물론 반응형에 대한 기획과 개발은 없지만
이런 경우, Slider에 좌우 마진을 추가하고 그 자리에 버튼을 넣는 방식으로 구현하면 어느정도 큰 수정을 하지 않고도 반응형을 대응할 수 있을 것 같습니다. 이러한 방법에 대해 한 번 고민해보시고,
그래도 해결이 안된다면 좀 더 자세한 힌트와 함께 도와드리겠습니다. 주저없이 질문해주세요!
좋은 답변 감사합니다!
지금은 1200px 이상에서의 환경만 고려하고 있지만 반응형도 추가 개발 사항으로 고려하고 있어요!
수정하면 참고할 수 있도록 저장해두겠습니다! 💯
<li>절임/장아찌</li> | ||
</ul> | ||
</NavigationStyle> | ||
</NavigationsStyle> |
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.
NavigationItem
NavigationList / NavigationGroup 과 같은 이름으로 확 눈에 띄게 만드는 것도 좋을 것 같습니다 👍
변수 이름이 길어지는 것에 대해 두려워할 필요가 없습니다!
const HeaderRightStyle = styled.div` | ||
display: flex; | ||
align-items: center; | ||
width: 40vw; |
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.
이 코드와 별개로, vw
와 %
의 차이에 대해 아시면 좋습니다!
스크롤바와 관련 있습니다 😃
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.
이 코드와 별개로,
vw
와%
의 차이에 대해 아시면 좋습니다!
스크롤바와 관련 있습니다 😃
넵! 감사합니다!!! ::)
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.
안녕하세요 리뷰를 맡은 Snow입니다 ☃️
깔끔하게 작성한 코드와 컴포넌트 구조 덕분에 읽는데 어려움이 없었습니다.
리뷰 코멘트 남긴 부분에 대한 의견 대부분 강력한 수정 필요! 보다는 의견 위주로 남겼습니다. 😃
Slider의 경우, 패키지로 배포하기에는 다소 아쉬운 점이 보입니다. 다른 라이브러리처럼 파라메터로 값을 넘겨주어 어느정도 커스텀이 가능하게 끔 만들어보는 것도 많은 도움이 될 것 같습니다. Material UI나 다른 Carousel 혹은 Slider과 같은 라이브러리를 참고하면 좋을 것 같습니다!
커밋 메세지가 간결하고 깔끔하여 아주 보기 좋군요 👍
안녕하세요,
상세하게 리뷰해주신 덕분에 코드가 보다 깔끔해진 것 같습니다!
Slider는 현재 구현 중에 있는데, 말씀해주신 것처럼 커스텀 기능을 추가한 다음 별도로 분리해서 모듈로 배포하는 것을 목표로 하고 있습니다 :)
양이 적지 않았을텐데 꼼꼼히 봐주셔서 감사합니다~!! 👏🌷
* [#1] init: 🎉 개발 환경 구축 * [#3] feat: ✨ Header 만들기 * [#5] feat: ✨ BestTab UI 구현 - BestTab UI - 상수 파일 - const.js - 재사용 컴포넌트 - Label.jsx - ItemCard.jsx * [#7] feat: ✨ Slide UI 구현 - App.jsx - SlideContainer import - BestItem.jsx - 삭제 (미사용) - BestItems.jsx - ItemCard prop 추가 - BestTab.jsx - 스타일 수정 - SlideContainer.jsx - UI 구현 - SlideItems.jsx - UI 구현 - SlideArrowBtn.jsx - UI 구현 - ItemCard.jsx - 스타일 수정 - prop 추가 - Label.jsx - prop 추가 - 기본 값 추가 * [#8] feat: ✨ ShowMoreBtn UI 구현 - App.js - ShowMoreBtn import - ShowMoreBtn.jsx - UI 구현 * [#10] feat: ✨ Header Dropdown 구현 - App.js - Header 경로 수정 - Header.jsx - 경로 변경 - HeaderLeft & Right 분리 - HeaderLeft.jsx - 컴포넌트화 - Navigations 컴포넌트화 - HeaderRight.jsx - 컴포넌트화 - Navigations.jsx - 컴포넌트화 - Dropdown 구현 * [#13] feat: ✨ API에 fetch 요청 로직 구현 * [#13] feat: ✨ API 요청, 베스트 기능구현 - BestItems.jsx - API 데이터 동기화 - BestTab.jsx - useState, useEffect, API 요청 - BestTabContainer.jsx - API 데이터 동기화 - BestTabNavigator.jsx - API 데이터 동기화 - ItemCard.jsx - prop 변경 - Label.jsx - COLOR변수 추가 * [#15] feat: ✨ 상세 modal 페이지 UI 구현 - App.js - PopUpContainer import - PopUpContainer.jsx - UI 구현 - PopUpImages.jsx - UI 구현 - PopUpInformations.jsx - UI 구현 * [#16] feat: ✨ 모달 페이지 이벤트 구현중 * [#16] feat: ✨ 수량정보 컴포넌트 분리 * [#19] refactor: 🔨 리팩토링, 부족한 부분 추가 구현 - 파일 및 폴더 구조 변경 - common 폴더 생성 - Context.jsx - useContext 사용하여 prop drilling 개선 - 팝업 이벤트 구현 - 수량 변경 - 주문하기 - 주문결과 안내 메시지 UI * [#19] refactor: 🔨 리팩토링 * [#17] feat: ✨ dj-slider 폴더구조 구축 * [#17] feat: ✨ 슬라이드 1/2 구현 중 * [#23] refactor: 🔨 코드 리뷰 코멘트 반영 및 개선 * [#17] feat: ✨ 슬라이드 구현중/일부사항 수정 - util.js - price에 comma 붙이는 기능 구현 - PopUpItemCountContainer.jsx - price에 comma 붙이는 기능 import - ItemCard.jsx - price에 comma 붙이는 기능 import - 이미지 background로 수정 - Label.jsx - 라벨 배경색상 적용 * [#17] feat: ✨ 슬라이드 구현중 - 모듈화 - 시연을 위한 기능 구현을 위해 보류 * [#25] feat: ✨ 슬라이드 2/2 구현, API 데이터 동기화 - 슬라이드 명칭을 캐로셀로 변경 - API 데이터 동기화 - 캐로셀의 ItemCard를 children으로 변경 - 모듈화를 위함! - 아이템카드 mini, large 프로퍼티 추가 - 상세모달 캐로셀 추가 - 상세모달 스크롤 추가 - 모든 카테고리 보기 기능 구현 * [#27] feat: ✨ BestTab Skeleton UI 만들기 - Main.jsx - 시연을 위한 loop 설정 추가 - BestTab.jsx - SkeletonTab import - BestTabNavigator.jsx - 주석 제거 - SkeletonTab.jsx - Skeleton UI 구현 - DicoJsonCarousel.jsx - Carousel 구현중 * [#29] feat: ✨ PopUp Skeleton UI 만들기 - Context.jsx - 주석 제거 - BestTab.jsx - 주석 제거 - PopUpContainer.jsx - Skeleton import - PopUpItemsSlide.jsx - 주석 제거 - SkeletonPopUpContainerBody.jsx - Skeleton UI 구현 * feat: ✨ carousel loop 기능 구현 * [#31] feat: ✨ README.md 작성완료 Co-authored-by: kowoohyuk <kowoohyuk91@gmail.com>
컴포넌트 구조
진행현황
[완료한 일]
[해야할 일]
[추가구현사항 - 가능하다면]
회고
Json
잘한 점
개선할 점
Dico
잘한 점
개선할 점