[team16-FE] 펭도리, 루크 1차 PR #56
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
폴더 구조 짜봤습니다 ㅎㅎ
오늘 관련 스키마 설계하여 mwb와 png파일 추가하였습니다.
This reverts commit 9546450.
Feature/best
…ture/PopUpModal
Feature/pop up modal
Feature/pop up modal
fix: ui별 각각 스타일 적용
feat: style, model tab ui 적용
refactor: Carousel
[FE]Feature/detail modal 캐로셀 refactor + model detail
feat: 주문하기
feat: POST order && PopUp Carousel
Feature/login
refactor: PopUpModal 클릭시 best Modal과 함께 뜨는 오류 수정
chore: add gitignore
Feature/login
useContext를 사용한다고 관리해야 할 상태가 줄어든 건 아니고요.. |
crongro
approved these changes
Apr 30, 2021
Comment on lines
+13
to
+14
| const soupRef = useRef(); | ||
| const sideRef = useRef(); |
There was a problem hiding this comment.
ref로 하위 컴포넌트를 접근하지 않는 방식을 찾아보세요.
어떤 상태에 의해서 하위 컴포넌트가 렌더링 되도록이요.
Comment on lines
+42
to
+43
| ? sideRef.current.Slider(1) | ||
| : sideRef.current.Slider(-1); |
There was a problem hiding this comment.
요런부분에서 어떤 상태를 만들어서 값을 변경시키고,
하위컴포넌트는 그 상태에 의존해서 렌더링 되도록 바꿔보세요.
| } | ||
| } | ||
| return ( | ||
| <PopUpModal |
Comment on lines
+17
to
+26
| const fetchData = async () => { | ||
| const soupData = await axios | ||
| .get("/products/soup") | ||
| .then((res) => res.data.data.items); | ||
| const sideData = await axios | ||
| .get("/products/side") | ||
| .then((res) => res.data.data.items); | ||
| setSoup(soupData); | ||
| setSide(sideData); | ||
| }; |
There was a problem hiding this comment.
async 함수를 사용할때 가급적 then을 사용하지 않고 await 로 표현해보세요.
| `; | ||
|
|
||
| const Best = ({ modal, setModal, ModalData, setModalData }) => { | ||
| const [toggleState, setToggleState] = useState(1); |
There was a problem hiding this comment.
toggle 이라면 0/1 또는 true/false 와 같이 어떤 두 가지 상태를 왔다갔다 하는 것일텐데
그런가요?
Comment on lines
+111
to
+127
| <MenuBtn | ||
| onMouseOver={() => setIsShownDrop1(true)} | ||
| onMouseLeave={() => setIsShownDrop1(false)} | ||
| > | ||
| 든든한 메인 요리 | ||
| {isShownDrop1 && ( | ||
| <DropMenu1> | ||
| <DropList> | ||
| <DropBtn>육류 요리</DropBtn> | ||
| </DropList> | ||
| <DropList> | ||
| <DropBtn>해산물 요리</DropBtn> | ||
| </DropList> | ||
| </DropMenu1> | ||
| )} | ||
| </MenuBtn> | ||
| </HeaderList> |
Comment on lines
+97
to
+99
| const [isShownDrop1, setIsShownDrop1] = useState(false); | ||
| const [isShownDrop2, setIsShownDrop2] = useState(false); | ||
| const [isShownDrop3, setIsShownDrop3] = useState(false); |
There was a problem hiding this comment.
저 함수들의 네이밍은 별로 인거 같습니다.
컴포넌트를 하위로 몇 개 만들고 그 안에서 1,2,3 이름은 제거하면 좋겠어요.
Comment on lines
+15
to
+17
| const data = await axios | ||
| .get("/products/main") | ||
| .then((res) => res.data.data.items); |
There was a problem hiding this comment.
다음에 axios 처럼 동작하는 fetch 라이브러리를 만들어보세요.
Comment on lines
+51
to
+61
| <CarouselButton Name={"Left"} Slide={leftSlider} /> | ||
| <Carousel | ||
| MainTitle={"모두가 좋아하는 든든한 메인요리"} | ||
| Food={Food} | ||
| setFood={setFood} | ||
| Ref={mainRef} | ||
| setModal={setModal} | ||
| ref={foodRef} | ||
| setModalData={setModalData} | ||
| /> | ||
| <CarouselButton Name={"Right"} Slide={rightSlider} /> |
There was a problem hiding this comment.
요렇게 캐로셀이 사용되는 구조를 보면,
사용하기 좋게 만들어진거 같아요.
| <CarouselButton Name={"Left"} Slide={leftSlider} /> | ||
| <Carousel | ||
| MainTitle={"모두가 좋아하는 든든한 메인요리"} | ||
| Food={Food} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Header & All
레이아웃 및 메뉴 노출 효과
메뉴에 마우스 올리면 레이어 노출
모든 카테고리 보기 누르면 추가 카테고리 노출
로그인 현재 구현하려고 공부중 ~Ing
Best
모든 메뉴 카드정보 및 Modal on&off
탭 카테고리에 따른 이미지 노출
-Main
-PopUpModal
코딩하면서 어려웠던점, 물어보고 싶은 내용
Luke
어려웠던 점: 분업으로 작업했던 팀원의 코드에 내코드를 적용 시키려고 할때, 코드에 대한 이해가 부족해서 작업을 원활이 이어가는 데에 조금은 어려운 점이 있었던것 같습니다. 그래도 모르는 부분은 물어보면서 작업을 하니 문제를 어느정도 해결할수 있었습니다.
펭도리
어려웠던점 : 분업을 하였지만 상대방에게 재사용 가능한 컴포넌트를 만들어주고 싶다는 생각에 상대방에게 컴포넌트 사용을 강요한 것 같습니다. 이 부분을 어떻게 협업해야 잘 협업한 것인지 아직까지 잘 모르겠습니다.
문제점
useContext를 사용하지 않아 상위 컴포넌트에서 상태를 받아서 관리해야하는 어려움에 직면하였고 그러다보니 관리해야할 상태들이 점점늘어나게 되었습니다.
Carousel Slide를 배열 갯수에 맞게만 동작하게 하고싶었으나 팝업창에서 캐러셀을 재활용 하고자 하니 어려움이 느껴져 계속해서 재활용 할 방법을 생각해 보고 있으나 잘 해결하지 못하고 있습니다.
PopUp Modal 에서 너무 하드코딩을 한것같은 느낌이 강하게 들었습니다. 무려 500줄... 다른것들을 분리한것처럼 분리할까도 생각했지만.. 뭔가 팝업모델은 한가지 파일에 있어야 할 것같아서 이렇게 만들었는데 맞는 방법인지 모르겠습니다.
또한 부모 컴포넌트에서 모달 컴포넌트를 사용할 때 특수한 경우에 임의로 props를 만들어 상황에 따라 모달창이 다르게 동작하게 했는데 이것도 점점 많아지다 보니 관리가 어려워졌는데 어떻게 해야 좀더 상황에 맞는 모달창을 만들 수 있는지?? 궁금합니다.