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

[ DDeong ] 1차 완성 / Carousel npm 배포 #19

Merged
merged 19 commits into from
Apr 26, 2021

Conversation

jjunyjjuny
Copy link
Collaborator

@jjunyjjuny jjunyjjuny commented Apr 22, 2021

진행 상황

  • Modal 구현 및 Button 유틸 컴포넌트 작성
  • 1차 완성 후 코드 리팩토링 중
  • carousel 1차 완성 후 npm 배포 완료.

carousel npm

jjunyjjuny and others added 19 commits April 21, 2021 15:50
좌, 우 클릭시 각종 상황을 판단해서 단순 이동(미구현) / 새로운 요소 추가 / 반대편 요소 가져오기 동작 구현
implement Modal feature without rendered by clicking Card
카드 이미지 클릭시 API를 호출해 상세정보 Modal에 띄워주는 이벤트 추가
[0421] Complete Modal component
좌우 이동 관련된 기능 완료.
스타일드 컴포넌트 테마 적용, Card컴포넌트 인자 수정, CSS 색상 단축표기법 적용
[0421] Refactor BestList, Badge, Card, Modal
[0422] Complete Button component
[0422] Complete Header component
@jjunyjjuny jjunyjjuny added the review-FE FE 리뷰 label Apr 22, 2021
@sphilee sphilee self-requested a review April 25, 2021 12:06
@sphilee sphilee self-assigned this Apr 25, 2021
Copy link

@sphilee sphilee left a comment

Choose a reason for hiding this comment

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

안녕하세요, 리뷰를 맡게된 팀입니다 😃

아직 1차 완성하시고 리팩토링 중이시군요,,

전체적으로 컴포넌트를 각각 만드셔서 최상위 파일에서 불러서 사용하시는 것 같은데 혹시 각각 파일로 나눠서 처리하면 1파일에서 처리하는 컴포넌트가 적어지고 컴포넌트 depth도 줄어들 것 같습니다ㅎㅎ

아래 의견드렸는데 궁금하시거나 이상하신부분 있으시면 말씀주세요:)

그럼 이번주도 화이팅하세용!

Comment on lines +114 to +134
<Menu onMouseEnter={() => setM1On(true)} onMouseLeave={() => setM1On(false)}>
<MenuTitle isOn={isM1On}>든든한 메인요리</MenuTitle>
<MenuBodyWrapper isOn={isM1On}>
<MenuBody>육류 요리</MenuBody>
<MenuBody>해산물 요리</MenuBody>
</MenuBodyWrapper>
</Menu>
<Menu onMouseEnter={() => setM2On(true)} onMouseLeave={() => setM2On(false)}>
<MenuTitle isOn={isM2On}>뜨끈한 국물요리</MenuTitle>
<MenuBodyWrapper isOn={isM2On}>
<MenuBody>국/밥/찌개</MenuBody>
</MenuBodyWrapper>
</Menu>
<Menu onMouseEnter={() => setM3On(true)} onMouseLeave={() => setM3On(false)}>
<MenuTitle isOn={isM3On}>정갈한 밑반찬</MenuTitle>
<MenuBodyWrapper isOn={isM3On}>
<MenuBody>나물/무침</MenuBody>
<MenuBody>조림/볶음</MenuBody>
<MenuBody>절임/장아찌</MenuBody>
</MenuBodyWrapper>
</Menu>
Copy link

Choose a reason for hiding this comment

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

text내용과 MenuBody갯수만 다른것 같은데 혹시 자식 컴포넌트로 분리해서 재사용할 수 있지 않을까요?

@@ -3,11 +3,13 @@
"version": "0.1.0",
"private": true,
"dependencies": {
"@jjunyjjuny/react-carousel": "0.0.2",
Copy link

Choose a reason for hiding this comment

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

직접 만드신 모듈이신걸까요? 👍

import BestList from "./js/components/bestList/BestList";
import Main from "./js/components/Main";
Copy link

Choose a reason for hiding this comment

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

p5) 개인적으로 jsx파일 형식자 인듯 한데 혹시 js폴더 밑에 두는게 이상해보이긴하네요.

Comment on lines +138 to +141
<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
<path d="M11 19C15.4183 19 19 15.4183 19 11C19 6.58172 15.4183 3 11 3C6.58172 3 3 6.58172 3 11C3 15.4183 6.58172 19 11 19Z" stroke="#BDBDBD" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round" />
<path d="M20.9999 21L16.6499 16.65" stroke="#BDBDBD" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round" />
</svg>
Copy link

Choose a reason for hiding this comment

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

svg도 파일이름으로 만들어서 import하면 해석하기 편할 것 같네용

https://github.com/ryanmcdermott/clean-code-javascript#use-searchable-names

Comment on lines +32 to +33
{body.replace(free, "")}
<b>{body.includes(free) ? free : ""}</b>
Copy link

Choose a reason for hiding this comment

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

p5) replace와 regex으로 한번에 처리할 수 있을 것 같기도 하네요 ㅎㅎ

</CardWrapper>
<ThemeProvider theme={theme}>
<CardWrapper>
<div onMouseEnter={() => setHover(true)} onMouseLeave={() => setHover(false)} onClick={clickHandler}>
Copy link

Choose a reason for hiding this comment

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

나중에 inline function과 useCallback 함수와 성능차이도 공부해보시면 좋을 것 같습니다 ㅎㅎ

Comment on lines +125 to +128
<CardPrice>
<CardPrice1>{n_price ? n_price + "원" : s_price}</CardPrice1>
<CardPrice2>{n_price ? s_price : ""}</CardPrice2>
</CardPrice>
Copy link

Choose a reason for hiding this comment

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

CardPrices라는 컴포넌트를 만들고 그 파일안에서 CardPrice 컴포넌트로 처리하면 어떨까요?

Comment on lines 115 to 127
<CarouselWrapper>
<Button prev onClick={() => move("prev")} />
<CarouselContainer ref={container} direction={direction.current}>
{renderList()}
</CarouselContainer>
<Button next onClick={() => move("next")} />
<Button prev onClick={() => move("prev")}>
<HiOutlineChevronLeft />
</Button>
<CarouselContent>
<CarouselContainer gap={gap} ref={container} dir={direction.current}>
{renderList()}
</CarouselContainer>
</CarouselContent>
<Button next onClick={() => move("next")}>
<HiOutlineChevronRight />
</Button>
</CarouselWrapper>
Copy link

Choose a reason for hiding this comment

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

jsx문법을 사용하셨는데 혹시 파일형식자를 js로 하신 이유가 있으실까요?

Comment on lines +41 to +42
const foo = [...thumb_images];
while (foo.length < 5) foo.push("");
Copy link

Choose a reason for hiding this comment

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

p5) 큰 차이는 없는데 아래처럼 하는 방법도 있을 것 같네요

Suggested change
const foo = [...thumb_images];
while (foo.length < 5) foo.push("");
const foo = [...thumb_images, '','','','',''].slice(0,5)

useEffect(() => {
setTopImage(() => top_image);
setThumbImages(() => {
const foo = [...thumb_images];
Copy link

Choose a reason for hiding this comment

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

foo라는 이름보다 의미있는 이름을 지어주시면 좋을 것 같습니다 ㅎㅎ

https://github.com/ryanmcdermott/clean-code-javascript#use-meaningful-and-pronounceable-variable-names

@jjunyjjuny jjunyjjuny merged commit ccb932d into codesquad-members-2021:DDeong Apr 26, 2021
pay-napster-x pushed a commit that referenced this pull request Apr 26, 2021
ehgud0670 pushed a commit that referenced this pull request Apr 27, 2021
ksundong pushed a commit that referenced this pull request Apr 28, 2021
[FIX] : 배포 시 JSON파일 읽을 수 없는 문제 수정
ksundong pushed a commit that referenced this pull request Apr 28, 2021
[BE] Service 구현 

issue: closes #5
MJbae pushed a commit that referenced this pull request Apr 29, 2021
샘플카드 및 theme 작업 중
sallyjellyy added a commit that referenced this pull request Apr 29, 2021
[BE] 카테고리 DTO 생성 및 유틸 패키지 분리
ksundong pushed a commit that referenced this pull request May 1, 2021
- DishService 생성 및 테스트 코드 추가
ksundong pushed a commit that referenced this pull request May 1, 2021
crongro pushed a commit that referenced this pull request May 4, 2021
* [#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-FE FE 리뷰
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants