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

[Goody,Kyle] 캐러셀 구현 중... #25

Merged
merged 10 commits into from
Apr 26, 2021

Conversation

hayoung123
Copy link
Collaborator

구현

  • 캐러셀
  • 리뷰기반 리팩토링

추가로 진행해야 될 사항

  • 캐러셀 라이브러리화???
  • 상세페이지 모달
  • 헤더 탭
  • 백엔드 부분

금주 회고

kyle

라이브러리로 만들려고 하다 보니 어느 부분까지 제가 묶어서? 만들어야 될지 고민이 많이 되는 것 같습니다. 다른 라이브러리를 참고하려고 보는데 쉽지 않네요...ㅎㅎ 백엔드 부분도 공부해보고 싶어서 다음주에는 백엔드에 집중해볼 것 같습니다.

goody

리액트 컴포넌트를 개발하면서, 함수의 추상화를 염두에 두지 않고 개발했다는 점이 아쉽고 고민이 됩니다.

Copy link

@snowjang24 snowjang24 left a comment

Choose a reason for hiding this comment

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

안녕하세요, 리뷰를 맡은 스노우 입니다 ☃️

커스텀 훅의 사용, 유틸의 분리 등 로직 분리를 의도적으로 계속 노력하는 모습이 너무 보기 좋습니다!

로직 분리가 잘 되어 있어 코드를 전체적으로 읽는데 편했으며, 컴포넌트 나누기도 잘 되어 있어 앞으로의 프로젝트 진행이 기대가 되는군요!

리뷰된 부분에 대해 고려해보시고 학습이 더 필요한 부분은 찾아보며 공부하시면 앞으로 더 좋은 코드를 작성하실 것 같아 기대가 됩니다 😄

카로셀의 경우 라이브러리로 만든다면, 어느정도까지의 자유도를 줄지를 고민해보는 것도 좋을 것 같습니다. 또한, 파라메터 이름과 메서드 이름에 대해서도 고민을 해보면 좋을 것 같습니다. 마지막으로, Carousel 에서 리액트의 children을 활용하여 Container과 Item을 따로 분리하여 만드는 방법에 대해서도 한 번 고민해보면 좋은 라이브러리가 될 것 같습니다

현재 @hayoung123 의 커밋만 보이는 것 같은데, @junzero741 풀리퀘를날리지 않은걸까요 ?

저희가 페어프로그래밍으로 진행을해서 이번 PR에는 제 커밋만 남았습니다!
저희도 라이브러리로 만들 때 어느까지의 자유도를 줘야될지 어떻게 주어야될지가 많이 고민이 되네요...
알려주신 라이브러리를 보면서 공부해보겠습니다!

body {
@import url('https://fonts.googleapis.com/earlyaccess/notosanskr.css');
font-family: "Noto Sans KR", normal !important;
}

Choose a reason for hiding this comment

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

폰트 로드 방법 및 최적화에 대해 한번 공부해보면 좋을 것 같습니다.
preloadfont-display: swap; 와 같이 웹 폰트를 불러 오거나, 로컬 폰트를 로드하는 다양하고 적절한 방법에 대해 공부해보시기를 권장드립니다.

Choose a reason for hiding this comment

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

폰트를 로드해오는 방식이 조금 하드하다고 생각하고 있었는데, 좋은 방법들이 많네요!!
추천해주셔서 감사합니다. 꼭 공부해보겠습니다!

};

const { data: bestList, loading, error } = useFetch({ url: URL.best(), parse: parseBestList });

Choose a reason for hiding this comment

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

커스텀 훅 분리가 인상적이네요 👀

Choose a reason for hiding this comment

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

kyle 이 완전 리액트 고수입니다..짱짱..

const parsedData = parseBestList(data);
setBestList(parsedData);
setFocusedCategory(parsedData[0].id);
const parseBestList = (json) => {

Choose a reason for hiding this comment

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

파싱 함수를, sort 함수의 compareFunction처럼 파싱하는 함수를 넘겨주는 방식은 어떨까요 ? 한 눈에 어떤 기능을 하고, 커스텀훅에서 왜 이러한 인자를 받는지 쉽게 이해할 수 있을 것 같습니다.

지금도 잘 짜셨지만, 이러한 방향으로도 한 번 생각해보시면 좋을 것 같습니다!

let numbers = [4, 2, 5, 1, 3];
const compareFunction =(a, b) => {
  return a - b;
};
numbers.sort(compareFunction);

Choose a reason for hiding this comment

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

추천해주신 방법대로 하면 파싱 함수의 재사용성도 높아지고, 코드도 더 보기 깔끔해질 것 같습니다.
다음 프로젝트부터는 꼭 시도해보겠습니다! 감사합니다.

import styled from 'styled-components';
import { IoChevronBackSharp, IoChevronForwardSharp } from 'react-icons/io5';

const Carousel = ({ children, itemWidth, maxItem, skipItem, animationTime }) => {

Choose a reason for hiding this comment

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

이런경우 width를 지정해야하고 다양한 기기에서의 활용이 불편할 것 같습니다.
반응형을 고려하여 짜보는 건 어떨까요 ?

아래와 같은 유명한 라이브러리에서 영감을 얻는 것도 좋은 방법일 것 같습니다. 코드를 참고하지 않고도, 완성된 카로셀을 개발자도구로 관찰하며 공부하는 것도 도움이 될 것 같습니다!
react-slick

Copy link

@junzero741 junzero741 May 1, 2021

Choose a reason for hiding this comment

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

리뷰 주신 부분 반영하여 width 인자로 받지 않아도 되는 카로셀을 만들었습니다.
오픈소스 코드를 읽어보는 연습도 게을리하지 않아야겠네요.
감사합니다!!


const handleMouseLeave = () => setIsHover(false);

const getImgSize = (size) => (size === 'L' ? '384px' : '308px');

Choose a reason for hiding this comment

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

이렇게 케이스별 중간 처리 없이 값만 리턴하는 경우
함수보다는 객체를 사용하면 좀 더 로직이 깔끔하고 한눈에 보일 것 같습니다.

const ImageSize = {
  lg: '384px',
  md: '308px'
}

width={ImageSize.lg}

Choose a reason for hiding this comment

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

말씀해주신 부분대로 수정해서 반영했습니다!
훨씬 보기 좋은 코드가 된 것 같아요.
감사합니다.

@crongro crongro merged commit 74d0791 into codesquad-members-2021:goooyle Apr 26, 2021
MJbae pushed a commit that referenced this pull request Apr 29, 2021
스프링 프로젝트 생성, jdbc 설정
ghis22130 pushed a commit that referenced this pull request Apr 29, 2021
refactor: BanchanListViewController Refactoring
@junzero741
Copy link

@snowjang24
코드 리뷰를 엄청 꼼꼼하게 해주셨는데,
프로젝트 진행하느라 너무 정신이 없어서 이제야 리플을 답니다.. ㅜㅜ
리뷰 너무 감사드립니다!!

crongro pushed a commit that referenced this pull request May 3, 2021
* [#1] feat : create CRA

 - CRA를 설치했습니다

* [#1] feat : mediumCard index.jsx prop 추가

 - 상동

* [#1] feat : node-sass 추가

 - node sass를 추가했습니다

* [#1] feat : 폴더 구조 정리

 - atoms, molecules, images 등 파일 구조 수정했습니다

* [#5] feat : reset scss 추가

- reset scss 추가
- 기본 글꼴 추가

* [#5] feat : button, icon, tag components 추가

 - atoms의 button, icon, tag component를 재사용 가능한 형태로 생성했습니다.

* [#5] feat: set moculses structure

- LargeCard 구조설정
- MediumCard 구조 설정

* [#5] chore : 잘못 생성된 파일 수정

 - 파일 명을 sidedish -> frontend로 바꾸는 과정에서 폴더 구성이 꼬여서 수정

* [#5] feat: molcules- LargeCard,MediumCard 구조 생성

* [#13] feat : MainDish, More 컴포넌트 생성

 - 신규 컴포넌트 생성: maindish, more organisms

* [#11] feat: fetch를 위한 util dir 생성

- util/loadData 생성 : fetch로 data가져오기
- util/url 생성: 기본 url주소 변수 설정

* [#11] feat: HeaderLeft 생성

* [#13] feat : MediumCard TagType 추가

 - 메인요리에 medium카드를 사용하기 위해 medium카드 일부 속성을 수정했습니다.

* [#13] feat : useFetch 컴포넌트 추가

 - fetch기능을 분리하기 위해 util-useFetch컴포넌트를 생성

* [#11] feat: Header 생성

- HeaderLeft 구조 생성
- HeaderRight 구조 생성
- BestDish 구조 생성 중

* [#11] feat: useFatch fetch오류 해결

* [#12] feat: BestDish useTabs hook 사용

* [#13] feat : 카드 캐로셀 기능 완성

 - setTimeout으로 렌더링을 지연시켜 캐로셀 기능을 완성

* [#12] feat: BestDish UI 및 기능구성

- Tab 버튼 클릭시 rendering
- Header Style 일부 수정 (HeaderRight,HeaderLeft,Span,Icon)

* deploy : build 210423

* [#13] feat: CSS style 수정

- innerTitle 설정

* chore: add gitginore

* chore: Add basic gradlew files

* [#13] feat: MainDish,SideDish,BestDish CSS style디테일 설정

- mainDish의 <Icon>,<Image> margin 설정

* [#13] fix : tag 버튼 오류, carousel 렌더링 지연 방식 변경

 - tag 가 이벤트 특가만 나타나는 오류 수정
 - carousel 렌더링 지연 방식을 setTimeOut ->  onTransitionEnd 로 변경

* deploy : build 210424

* [#25] feat : Detail UI 컴포넌트 생성

 - Detail UI 를 위한 InfoGeneral, InfoImages, InfoNumber, InfoPrice, InfoProduct molecules 컴포넌트 생성

* [#24] feat: ADD tab click event(color change)

- Fix MediumCard Tag 부분

* [#25] fix : icon onClick 이벤트 실행 로직 수정

 - icon이 left, right 일 경우에만 moveSlide이벤트가 실행되도록 수정

* [#24] feat: ADD Img MouseOver event

- useState를 통한 isHover로 상태변경 가능

* [#24] feat: Fix CSS Detail in HoverEvent

* deploy : build 210427

* [#24] feat: ADD HeaderDrop event

- Header MouseOver 시 Drop event 추가
- Header DropMenu MouseOver 시 hover event 추가
- Private Component 이름 통일변경 (ex Div -> WrapDiv)
- Component들의 inex.style.jsx 추가 (Styled div 분리)

* [#25] fix : MainDish Carousel 스타일 적용 함수 분리

 - MainDish 스타일 및 매직넘버 삭제

* [#37] refactor : 코드리뷰 반영

 - useFatch 구조 변경 : loadData 컴포넌트 생성해서 데이터만 받아오고 useEffect는 각 컴포넌트에서 실행
 - tag 내용 및 컨텐츠 적용 방식 수정
 - span 스타일 컴포넌트 적용방식을 className 사용으로 변경

* [#25] feat : Detail UI 컴포넌트 수정

 - pages 컴포넌트 생성

* [#25] feat : Modal 컴포넌트 생성

- 모달 컴포넌트 생성 및 클릭 후 모달 오픈, close 버튼 클릭시 숨김 기능 구현

* [#25] feat: Modal data fetch 연결

* deploy : build 210428

* deploy : rebuild 210428

* deploy : rebuild 210428

* deploy : rebuild 210428

* build : rebuild 210428

* [#26] feat : 상세페이지 내 이미지 이벤트 추가

 - 이미지 및 썸네일 추가
 - 썸네일 클릭 시 메인 이미지 변경 이벤트 추가

* [#26] feat : large 카드에 모달 이벤트 추가

- large 카드에도 모달 이벤트를 추가했으나, 아직 베스트 반찬에는 데이터 api를 못받아오고 있으므로 실행은 되지 않게끔 주석처리 했습니다

* [#27] feat: Detail - Info UI 생성 및 fetch data 연결

* deploy :build 210429

* [#27] feat: Fix syntax error

* build : build 210429

* [#26] refactor : carousel 컴포넌트 생성

- 라이브러리화를 위해 carousel 컴포넌트 생성

* build : rebuild 210429

* build : rebuild 210429

* [#26] refactor : carousel 컴포넌트 완성

carousel, carousel style 컴포넌트를 mainDish 컴포넌트에서 분리 완료

* [#26] feat : OtherCard, DetailOther 컴포넌트 생성

 - 디테일 Carousel 작업을 위해 상기 컴포넌트 생성

* [#27] feat: Datail CSS UI 수정

- UI: BestMenu 오른쪽마진 수정
- UI: HoverCard 가운데 정렬  수정
- UI: Mouse cursor 디테일 설정
- feat: useTabs 삭제 및 component내부로 수정
- faet: getComma 파일생성 ( 원, (,) 넣는 함수)

* deploy : rebuild 210429

* deploy : build 210430

* [#27] FE API data fetch 연결 확인

* [#45] FE API data fetch 연결 확인

* [#26] feat : OtherCard 스타일 적용 완성

 - Other Card의 레이아웃 완성

* [#26] feat : OtherCard 케로셀 적용 중

* [#45] feat: 디테일한 CSS UI 구성

* [#26] feat : OtherCard 케로셀 완료

- portal을 적용해서 기존 캐러셀 컴포넌트를 재활용할 수 없는 관계로 캐로셀을 중복해서 사용함

* [#45] feat: fetch Data API and 합치기

* [#45]feat: Details 파일명 변경 및 데이터 전달

* [#45] feat:Details close error 해결

* [#45]feat: CSS style 수정

* [#45] feat: Image data 수정

* fix : Change directory name to camel case

Co-authored-by: ink-0 <71919983+ink-0@users.noreply.github.com>
Co-authored-by: woody <woojihye2339@gmail.com>
Co-authored-by: Tree <gmldbs1109@naver.com>
Co-authored-by: Ubuntu <ubuntu@ip-172-31-44-162.ap-northeast-2.compute.internal>
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.

4 participants