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

[team-13 FE] 1주차 PR #29

Merged
merged 54 commits into from
Apr 26, 2021
Merged

Conversation

cchoongh
Copy link

@cchoongh cchoongh commented Apr 23, 2021

image

1주차에는 빨간색으로 표시한 컴포넌트 구현 위주로 진행되었습니다.

  • Slider 컴포넌트

    • 라이브러리로 배포할 목적으로 설계하고 구현해봤습니다. 아직은 초기구현 상태입니다.
    • Slider를 사용하는 부모의 사이즈와 내부에 보여질 아이템 크기에 맞춰 itemCntOnView매개변수 만큼의 아이템 개수를 한 번에 띄워줍니다.
    • 부모와 아이템의 사이즈를 런타임에서 파악하기 위해 useRef()를 사용했는데 맞는 방법인지 모르겠습니다.
    • 윈쪽, 오른쪽 버튼을 부모에서 커스터마이즈하여 사용할 수 있도록, useImperativeHandle(), forwardRef()를 적용해 봤습니다.
  • 서버 API관련 질문

    • 서버랑 통신하는 함수들은 보통 어떻게 분리하여 관리하는지 궁금합니다.
    • 서버 URL은 어디에 저장해놓고 사용하나요?
  • 기타 질문

    • 컴포넌트내에서 props로 넘어오는 값들이 유효한지 확인하고 에러를 throw하는 코드는 어디에 작성하면 좋을까요??
    • 변하지 않는 props들을 이펙트함수 내에서 사용할 때, deps array에 넣어주지 않으면 warning이 뜨는데, 굳이 넣어야하는 이유가 무엇인가요?
    • 아래와 같이 preview 기능에서 <img> 태그src 속성을 넣어서 사용한다고 하면, 작은 이미지들을 클릭할때마다, 확대이미지로 띄우기 위해 네트워크요청을 클릭할 때마다 하게되는데 이런식으로 구현하는게 일반적인건가요?
      image

zel0rd and others added 30 commits April 19, 2021 16:19
* 런타임 때 필요할 것 같지 않아, 일단 dependenciesDev에 추가돼 있는데, 추후에 변경 될 수도 있을 듯
* common/Modal 컴포넌트 추가. content 컴포넌트를 자식으로 둔 다음. content가 가운데 배치되도록 구현
* MenuDetailModal에서 common/Modal을 사용하고, content로 MenuDetail 컴포넌트를 사용하도록 구조를 짜봤음.
* 아직 거의 비어있음.
* 전역적으로 사용될 config들이 저장될 Global.js
* custom hook들이 있는 hooks 디렉터리
* 스타일 적용
* 이미지가 5개 미만일 시, 회색으로 채워지도록
* 클릭 로직 구현
* common/badge.js
* 추가하고 싶은 뱃지 있으면 위 파일안에다가 구현후 export에 추가하기
Fe/feature/menu detail modal, common한 벳지컴포넌트 추가
* 'OrderInfo'쪽 구현 도중에 조정한거라, 해당 부분이 살짝 포함 되어있음..
* 추후에 'common/'으로 뺄 가능성 농후
* 하위 구성요소 'OrderInfo', 'SmallInfoContainer' 컴포넌트 추가
* 수량 변화에 따른 총 주문가격 로직 구현
* 아직 '주문하기'버튼 클릭 로직은 미구현(백엔드에서 선처리 필요)
cchoongh and others added 12 commits April 24, 2021 21:31
* 전체의 아이템 개수가 화면에 보여질 아이템 개수로 딱 나눠 떨어지지 않을 때의 경우를 처리하는 로직 구현
[ADD]: Slider 끝부분 처리
* Slider쪽 코드도 같이 고침..
  * onSlide콜백을 받도록 수정
[ADD]: Slider 컨트롤 버튼 disabled기능 구현
* useFetch() => useFetchData() 이름 변경
* Global객체에 server url관련 함수 추가
* position속성 absolute => relative
* 추천 슬라이더 스타일 구현
* 추천 슬라이더 페이지 표시 구현
* 추천 슬라이더 버튼들 구현
* 전체적인 margin, padding 조정
[ADD] BestMenu Tab function added
@crongro
Copy link

crongro commented Apr 26, 2021

  • 모와 아이템의 사이즈를 런타임에서 파악하기 위해 useRef()를 사용했는데 맞는 방법인지 모르겠습니다.

제한적으로 사용할 수 있다고 생각합니다.

@crongro
Copy link

crongro commented Apr 26, 2021

  • 서버 URL은 어디에 저장해놓고 사용하나요?

배포되지 않는 설정파일에 두고 빌드타이밍에 소스코드를 replace하는 방식을 많이 하겠죠.

웹팩설정을 해야하는데, cra에서는 세련방 방법은 저도 찾아봐야겠네요.
https://stackoverflow.com/questions/42686149/cant-build-create-react-app-project-with-custom-public-url

@crongro
Copy link

crongro commented Apr 26, 2021

  • 컴포넌트내에서 props로 넘어오는 값들이 유효한지 확인하고 에러를 throw하는 코드는 어디에 작성하면 좋을까요??

컴포넌트 상단에서 제일 먼저 체크해야겠죠?

@crongro
Copy link

crongro commented Apr 26, 2021

  • 변하지 않는 props들을 이펙트함수 내에서 사용할 때, deps array에 넣어주지 않으면 warning이 뜨는데, 굳이 넣어야하는 이유가 무엇인가요?

변할까봐요.

@crongro
Copy link

crongro commented Apr 26, 2021

  • 아래와 같이 preview 기능에서 <img> 태그src 속성을 넣어서 사용한다고 하면, 작은 이미지들을 클릭할때마다, 확대이미지로 띄우기 위해 네트워크요청을 클릭할 때마다 하게되는데 이런식으로 구현하는게 일반적인건가요?

이미지는 기본적으로 브라우저에 캐시가 되니, 매번요청하게 되지는 않을거에요. 클릭할때 이미지를 가져오도록 하는것은 lazy loading차원에서 좋네요. 그리고 이미지 preloading이라는 기법으로 미리 이미지를 가져오는 방법도 있습니다.

Copy link

@crongro crongro left a comment

Choose a reason for hiding this comment

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

엄청 열심히 개발하셨군요.
잘 구현하셨네요. 컴포넌트 크기가 대체로 큰편이 아니라서 그부분이 좋았습니다.

복잡한 계산은 동일한 결과일 수 있으니, 메모이제이션을 적용할 곳이 있는지도 생각해보시고요.

const renderTabTitles = () => {
return response.body.map((v, idx) => {
if (index === idx) {
console.log("SAME")
Copy link

Choose a reason for hiding this comment

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

디버깅 코드는 지우고 커밋하기~

return (
<div style = {{ marginLeft : "5%" }}>
<style.BestMenuTitle>{BestMenuStatic.Title}</style.BestMenuTitle>
<FlexRowContainer >
Copy link

Choose a reason for hiding this comment

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

FlexRowContainer 도 styled.Flex 로 해도 될 듯 하고요. 일관성을 생각해서요.
공통요소라 예외로 한거 같군요.

}

return (
<div style = {{ marginLeft : "5%" }}>
Copy link

Choose a reason for hiding this comment

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

이건 또 별도 styled component로 만드신건 아니군요.

@@ -0,0 +1,14 @@
const BestMenuTab = ( props ) => {
const Items = Object.values(props)[0]
const renderItems = () => Items.map((v) => {
Copy link

Choose a reason for hiding this comment

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

destructuring 해서 Items.map(({cateId}) => {}

</style.HeaderLeftStyle>

<style.HeaderRightStyle>
<style.SearchBarFormStyle>
Copy link

Choose a reason for hiding this comment

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

네이밍 보니 앞에도 style을 붙이고, 뒤에도 Style이라고 하시느 중복이름인거 같아요.
정작중요한 것은 SearchBar, SearchButton 인데 가운데 끼여서 잘 드러나지 못하는 것 같아요.


function OrderInfo({ data }) {
const priceRef = useRef(data.prices[1].match(/[0-9]*/g).join(''));
const [totalPrice, setTotalPrice] = useState(data.prices[1]);
Copy link

Choose a reason for hiding this comment

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

(data.prices[1]) 1이 가진 의미가 좀더 구체적이면 좋겠죠.
prices 배열대신 객체구조를 사용한다던가 등


const renderSmallInfos = () => {
return data.map((info, idx) => {
const [entry, content] = info;
Copy link

Choose a reason for hiding this comment

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

이것도 위에서 같이 선언가능하겠네요.
return data.map(([entry, content], idx) => {

return (itemWidth + betweenMargin) * itemLength - betweenMargin;
}

const _calcPositionLeft = ({ itemWidth, currIdx, betweenMargin }) => {
Copy link

Choose a reason for hiding this comment

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

이런메서드들을 Slider밖에 선언한 것도 괜찮네요.

Comment on lines +51 to +58
itemWidth: itemRef.current.offsetWidth,
sliderWidth: styledRef.current.offsetWidth,
itemCntOnView
});
const newTotalWidth = _calcTotalWidth({
itemWidth: itemRef.current.offsetWidth,
itemLength: items.length,
betweenMargin: newBetweenMargin
Copy link

Choose a reason for hiding this comment

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

여기서 계산되는 값들이 매번 달라지는 값이면 어쩔수 없지만,
같은값인 경우에는 캐시를 잘 활용하셔야겠네요.
offsetWidth 이런값을 접근할때 reflow가 발생한다고 합니다.


// TODO: default slide buttons, disable slide button

function Slider({ itemCntOnView, items, onSlide, defaultBtn = true, pageable = false }, ref) {
Copy link

Choose a reason for hiding this comment

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

오늘 언급한 것처럼, Slider 컴포넌트가 범용적으로 동작하도록 계속 발전시켜보세요~

@crongro crongro merged commit 4fb0074 into codesquad-members-2021:team-13 Apr 26, 2021
sallyjellyy pushed a commit that referenced this pull request Apr 29, 2021
wheejuni pushed a commit that referenced this pull request Apr 29, 2021
[BE] mock-api 샘플 데이터 추가
wheejuni pushed a commit that referenced this pull request Apr 29, 2021
hayoung123 pushed a commit that referenced this pull request May 2, 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

4 participants