-
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
[Team23] Tab section 구현, 컴포넌트 트리 구조 추가 #17
Conversation
PR Review 받은것 수정완료
- 탭 같이 만들어봄 - 지금 TabSection에서 fetch가 안불러와지는 것 같음. 각자 문제 찾아 볼 예정
- style 작업 수행 - active 상태값에 따라 다른 스타일이 먹도록 구현 - 트리플 프로그래밍!
- 라벨 붙임 - 라벨 스타일 조정 - 베스트 라벨 추가
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.
안녕하세요. 리뷰를 맡은 윤입니다.
무의미한 div들을 사용하는 것은 문제가 있을 수 있으나, 배치를 위해서라면 styled-components를 사용해서 배치하는 것은 괜찮을 것 같아요. 😀 만약 html5의 다양한 시멘틱 태그들을 활용하면 더 좋을 것 같구요. (네비게이션 메뉴에는 <nav>
, 섹션에는 <section>
활용 등)
고생하셨습니다!
@@ -17,10 +21,10 @@ export const GlobalStyle = createGlobalStyle` | |||
|
|||
function App() { | |||
return ( | |||
<> | |||
<ThemeProvider theme={theme}> |
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.
[p5] 테마 활용 좋네요. 👍
import HeaderNavbar from "./HeaderNavbar.jsx"; | ||
import { CenterContainer } from "../../utils/styles/common.jsx"; |
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.
[p4] path를 편리하게 관리하기 위해 alias를 활용하면 좋을 것 같아요. eject 후 빌드 설정을 수정하는 방법이 있고, craco 와 같은 라이브러리를 활용하는 방법도 있습니다! 😀
} | ||
color: ${(props) => props.theme.colors.gray}; | ||
font-weight: "400"; | ||
`; | ||
|
||
const handleMouseEnter = () => { | ||
setIsMouseOver((isMouseOver) => (isMouseOver = true)); |
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.
[p5] setIsMouseOver(true);
로 바로 설정해도 되지 않나요? 혹시 이렇게 작성하신 이유가 있을까요? 🧐
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.
아 그러네요? 당시엔 잘 안보였던 것 같습니다.. 감사합니다 :)
</ThemeProvider> | ||
<NavLI onMouseEnter={handleMouseEnter} onMouseLeave={handleMouseLeave}> | ||
<NavLITitle>{list.title}</NavLITitle> | ||
{isMouseOver && <NavLIBox>{listSubElements}</NavLIBox>} |
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.
[p5] 개인적으로는 현재 마우스 오버 이벤트가 컴포넌트를 보여주고, 숨기기 위한 처리라면 css hover로 처리하는 것도 좋지 않을까 싶습니다! (혹 다른 이벤트나 동작을 추가할 경우가 아니라면요!) 😀. 성능적인 면에서도 css로 조절이 가능한 부분은 최대한 css를 활용하시는 것을 추천합니다.
const SearchBox = styled(CenterContainer)` | ||
justify-content: space-between; | ||
border-radius: ${(props) => props.theme.borders.radius}; | ||
background: ${(props) => props.theme.colors.lightGrayBG}; |
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.
[p5] css를 styled components의 장점을 활용해서 사용하시는 부분이 좋네요. 👍
) | ||
.then((response) => response.json()) | ||
.then((result) => setProducts(result.body)) | ||
.then((error) => console.log('error', error)); | ||
.then((error) => console.log("error", error)); |
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.
[p4] fetch api를 편리하게 사용하기 위한 custom hook을 만들어보시는 것도 추천합니다.
const response = await fetch(tempUrl); | ||
const json = await response.json(); | ||
setBestSidedishes(json.body); | ||
}; |
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.
[p4] api를 핸들링할 때 에러 처리는 꼭 추가하는 것이 좋습니다. 위에도 언급한 것처럼 fetch를 위한 custom hook을 만들어서 적용하면 좋을 것 같아요.
}, []); | ||
|
||
const handleTab = (id) => { | ||
setActiveTab(id); |
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.
[p4] setActiveTab 함수를 실행시키는 역할만 하는 경우 밑에 onClick에서 바로 setActiveTab 함수를 호출해도 좋을 것 같아요.
src={type === "베스트" ? mockImage : product.image} | ||
alt="card-image" | ||
/> | ||
<StyledTitle>{product.title}</StyledTitle> |
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.
[p5] 스타일링만을 위한 컴포넌트라도 네이밍을 <Title></Title>
, <Description></Description>
과 같이 가져가도 괜찮을 것 같아요.
const CardList = styled.ul` | ||
display: flex; | ||
justify-content: center; | ||
`; |
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.
자주 사용하는 스타일을 따로 분리해서 관리하는 부분 좋습니다 👍
리뷰 종료 후 머지까지 하는 가이드를 받아서, 머지하겠습니다 😀 |
[0422] Complete Header component
- DishDTO, BestResponseDTO 생성
- delivery 테이블 필드 delivery_type, delivery_fee, delivery_condition의 디폴트 값 설정 - data.sql에 수정된 스키마에 맞추어 쿼리문 수정
- Mockup API 네이밍을 참고해서, DishDTO에서 ItemDTO로 클래스명 수정
- 사용하지 않는 클래스 SectionResponseDTO.java 삭제
- DishRepository::findById, DishRepository::findAll 메소드를 오버라이드 - 커스텀 쿼리를 사용하여, 클라이언트의 요청에 맞는 데이터(ItemDTO)를 추출
- 특정 section에 속한 dish 데이터를 가져오는 기능 구현 - DishController::showDishsBySection 메소드 구현 - DishService::showDishsBySection 메소드 구현 - DishRepository::findAllBySection 구현 및 커스텀 쿼리 추가
- Mockup API의 네이밍에 맞게 Dish라는 명칭을 Item으로 수정
- 불필요한 메소드 ItemController::showDishByHash 삭제
* [#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>
어제, 오늘 한 일
궁금한 점
배치를 위해 div 요소가 필요한데, 통일감을 위해서 아무 내용이 없는 Styled component로 만들었습니다. 이렇게 하는 것이 더 나은지 아니면
<div></div>
로 쓰는 게 나은지 궁금합니다.컴포넌트 트리
👉 바로가기