[Team-26][FE][이든&도리] 1주차 02 PR 요청 #68

kimyouknow · 2022-05-27T03:28:15Z

안녕하세요 어텀. 도리 이든입니다. 이번 pr도 잘부탁드려요😀

PR 피드백

1. `GET /search`에서 사용하는 location관련 params

네이버 지도, 구글지도, airbnb에서 지도 api가 어떻게 동작하는지 사례를 찾아봤는데 사이트마다 동작방식이 달라 어떤 방법이 좋을지 더 고민하여 추후 수정할 예정입니다.

2. 필수 선택 미션

Global 상태관리를 위한 라이브러리를 직접 개발한다.
캘린더 컴포넌트를 재사용 가능한 방식으로 개발해서 NPM에 올린다.

위 두가지가 관심이 있는데 우선적으로 캘린더 컴포넌트를 구현할 예정입니다.

진행상황

⬇️간단한 데모

캘랜더

차트

1. 설계

차트 설계

캘린더 설계

2. 차트 구현

차트를 구현하는 방법으로 svg와 canvas 등 여러가지 방법으로 시도중입니다!

3. 코딩 마라톤 방식 느낀 점

저번 pr에서 언급해주셨던 마라톤 방식으로 캘린더 컴포넌트와 차트 컴포넌트를 구현하고 있습니다.

코딩 마라톤중 다른 팀원과 속도 차이가 발생한다면?

서로 어느정도 균형 있는 분량을 해야 앞 사람의 코드를 이해하고 다른 사람이 자신의 코드를 이어서 짤 수 있는 분량 만큼 나와야한다고 느꼈습니다. 그래서 중간중간 진행상황을 공유하면서 속도 차이가 많이 난다면 먼저 구현한 사람은 다른 개념을 학습하고 있거나, 진행이 더딘 내용을 페어로 진행하는 방식을 고려했습니다.

느낀점

마라톤을 시작한지 얼마 안되었지만 코드를 짜면서 다음 사람이 잘 이해할 수 있도록 변수명이나 함수명, 컨벤션등을 이전보다 더 신경쓰게 되었습니다.

해결이 되지 않은 점

typescript로 react 컴포넌트 npm에 배포해보기

npm 라이브러리 배포하는 과정에서 js로는 배포 할 수 있지만 typescript 배포하는과정에서 tsc 컴파일 스크립트 사용의 미숙으로 실패한 상황입니다. 자세한 것은 링크에 작성하였습니다.

npm 라이브러리 배포하기

* docs: 템플릿 수정(리뷰반영) * refactor: config폴더내의 파일을 루트로 이동(리뷰반영) * build: react-refresh 추가 * feat: 모달 구현

* feat: 현재 달의 calendar를 모달에 출력 * fix: calendar날짜가 마지막 일까지 출력되도록 수정 * refactor: 캘린더 뷰에 사용하는 상수를 파일로 분리

* Feat: svg 그래프 * Feat: canvas 레이아웃 - 선 그리는 로직 미완 * Chore: svgChart에서 plots 좌표값 관련 주석 작성

deprecated-hongbiii

이든, 도리 안녕하세요!
설계 문서를 공유해주셔서 코드 이해가 더 잘 되는 것 같아요. 👍

캘린더 구현해주시고 있는 부분은 이번달(5월)만 먼저 그려보신 걸로 보여요.
캘린더 캐로셀 구현까지 고려하신다면 설계가 많이 바뀌어야 할 것 같습니다!

공유해주신 캘린더 설계 노션 문서에서 : 왼쪽의 숫자와 오른쪽의 숫자는 각각 어떤 의미인지 궁금합니다!

deprecated-hongbiii · 2022-05-28T09:12:47Z

FE/package.json

@@ -22,7 +22,9 @@
  "homepage": "https://github.com/kimyouknow/react-boiler-template#readme",
  "dependencies": {
    "react": "^18.1.0",
-    "react-dom": "^18.1.0"
+    "react-dom": "^18.1.0",
+    "react-refresh": "0.13.0",


webpack의 hot module replacement를 사용하지 않고 패키지를 설치한 이유가 궁금해요!

deprecated-hongbiii · 2022-05-28T09:16:43Z

FE/src/components/WindowModal/style.js

+  border: 1px solid black;
+  z-index: 1012;
+  position: relative;
+  --saf-0: rgba(var(--sk_foreground_low, 29, 28, 29), 0.13);


--saf-0 이건 무슨 속성인가요?? 처음 봐서요! 👀
--sk_foreground_low 이것도 궁금해요!
외부 라이브러리를 사용하는 건가요?

앗...제가 이전에 썼던 코드를 복사했는데 뭔지도 모르고 무지성으로 복사해버렸네요..
찾아봐도 자료가 별로 없어 해당 코드를 지웠습니다 ㅠㅜ

deprecated-hongbiii · 2022-05-28T09:30:07Z

FE/src/components/WindowModal/Portal/index.tsx

+    };
+  }, []);
+
+  const rootElement: Element | DocumentFragment | null = document.getElementById('modal');


modal id를 가진 dom이 있어야만 기대한대로 동작(=모달 띄우기)을 할텐데요,
물론 modal id를 가진 dom은 최초에 한 번만 세팅해주면 되겠지만
개인적으로는 모달을 띄우기 위해 index.html과 컴포넌트 양쪽으로 코드를 작성해줘야 하는 점에서 관심사가 분산된 듯한 느낌이 듭니당
컴포넌트 호출만으로 모달을 띄울 수 있는 방법은 없을까요??

deprecated-hongbiii · 2022-05-28T09:32:04Z

FE/src/components/WindowModal/Portal/index.tsx

+interface Props {
+  children: ReactNode;
+}
+
+function Potal({ children }: Props) {


Props 인터페이스 네이밍이 좀 더 자세하면 좋을 것 같아요!
지금 모든 컴포넌트에 Props 라고 이름이 지어져 있는데,
프로젝트 규모가 커지고 함께 일하는 개발자가 많아질수록 검색도 많이 하게 되거든요.
그 때 같은 이름이 많다면 원하는 특정 변수를 찾기가 힘들어진답니다.

deprecated-hongbiii · 2022-05-28T09:37:37Z

FE/src/hooks/useToggle.ts

+
+const useToggle = (initialMode: boolean): ReturnProps => {
+  const [isToggleOn, setIsToggleOn] = useState(initialMode);
+  const handleToggle = () => setIsToggleOn(prev => !prev);
+  return [isToggleOn, handleToggle, setIsToggleOn];


리액트의 useState가 반환하는 것이 [state, setState] 형태의 배열이라서
[isToggleOn, handleToggle, setIsToggleOn] 이 순서보다는 [isToggleOn, setIsToggleOn, handleToggle] 이 순서가 좋을 것 같습니다!
또는 객체로 리턴해줘도 괜찮을 것 같아요.

return { isToggleOn, handleToggle, setIsToggleOn }

deprecated-hongbiii · 2022-05-28T09:44:59Z

FE/src/components/Calendar/index.tsx

+import * as S from './style';
+
+export default function Calendar() {
+  const overDay = 32;


28, 29 (윤년), 30, 31일까지인 달들이 있는데 32로 고정해도 괜찮은 건가용?? 아직 5월 달력만 고려하셔서 32인 걸까요? 🤔

[Feat] 그래프 그리고 슬라이더와 연동

- 체크인, 체크아웃 파리미터가 Null 인 경우에 대응하기 위해, 숙소 검색 쿼리를 수정하였습니다.

…vorite feat: [46] 유저 위시리스트 취소 기능 구현

feat: Http Request 로깅

* [Muffin] 검색결과 페이지 - 왼쪽 레이아웃 작업 ( 더미데이터 사용하여 화면 출력) (#67) * feat: Search Component 분리 * refactor: 메인페이지 가까운 레이아웃 스켈레톤 UI 적용해보기 * feat: 검색결과 페이지 왼쪽 레이아웃 더미데이터 사용하여 렌더링 * refactor: location 타입 지정 * refactor: room 컴포넌트 내 함수 네이밍 수정 * [Park] create mock server (#68) * feat: create msw * feat: some data use dummy data * refactor: main page type & varriables name refactoring * feat: useFetch hooks * feat: useFetch type modify * data: roomDetailList -> roomDetail 로 수정 * [Muffin] 검색 결과페이지 숙소 모달 관련 기능 작업, moment 라이브러리 추가 * [Muffin] modal창을 열었을때 Header 밀리는 현상 * [Muffin] 차트 슬라이드 에러 수정 * refactor: slider 에러 관련 처리 및 날짜 미선택 시 차트 모달에 안내 텍스트 보여주기 * refactor: 빅서치바 인풋 체크인, 체크아웃 x 버튼클릭시 에도 차트 관련 데이터 리셋 * refactor: price 금액 초기화 하는 context SET_INIT_PRICE type으로 초기화 하도록 수정 * [Park] GitHub OAuth login * feat: github login authorization and get code * feat: post logic 백엔드에서 처리 -> CORS 문제로 이전 * feat: github login, avatar url * style: 안쓰는 변수 제거 * fix: .env github client id add * refactor: 중복 로직 제거 및 안쓰는 변수 제거 * fix: map scroll & rooms type Co-authored-by: Muffin <jinlog9@gmail.com>

HongJungKim-dev and others added 7 commits May 26, 2022 16:28

[Feat] 모달 구현 (#8)

7a5e831

* docs: 템플릿 수정(리뷰반영) * refactor: config폴더내의 파일을 루트로 이동(리뷰반영) * build: react-refresh 추가 * feat: 모달 구현

refactor: config폴더내의 파일을 루트로 이동(리뷰반영)

8848ce1

build: react-refresh 추가

32f12cd

build: 빌드 수정

6c02d87

build: webpack styled options 수정

57fe8a7

[feat] 캘린더 한 달 뷰 (#11)

30b7fce

* feat: 현재 달의 calendar를 모달에 출력 * fix: calendar날짜가 마지막 일까지 출력되도록 수정 * refactor: 캘린더 뷰에 사용하는 상수를 파일로 분리

[feat] 차트 레이아웃 시도 (svg, canvas) (#12)

4ce74f5

* Feat: svg 그래프 * Feat: canvas 레이아웃 - 선 그리는 로직 미완 * Chore: svgChart에서 plots 좌표값 관련 주석 작성

kimyouknow added the review-FE New feature or request label May 27, 2022

kimyouknow requested a review from deprecated-hongbiii May 27, 2022 03:28

kimyouknow assigned deprecated-hongbiii May 27, 2022

deprecated-hongbiii reviewed May 28, 2022

View reviewed changes

kimyouknow merged commit f294128 into codesquad-members-2022:eden-dori May 30, 2022

This was referenced May 30, 2022

[PR] 1주차 두번째 PR 리뷰 반영 kimyouknow/codesquad-airbnb#13

Closed

[PR] 1주차 두번째 PR 리뷰 반영 kimyouknow/codesquad-airbnb#14

Merged

SangHwi-Back added a commit that referenced this pull request Jun 3, 2022

Merge pull request #68 from ak2j38/feature-drawGraph

071eb3b

[Feat] 그래프 그리고 슬라이더와 연동

guswns1659 pushed a commit that referenced this pull request Jun 5, 2022

Fix: 숙소 검색 쿼리 수정 (#68)

c40d6f3

- 체크인, 체크아웃 파리미터가 Null 인 경우에 대응하기 위해, 숙소 검색 쿼리를 수정하였습니다.

wooody92 pushed a commit that referenced this pull request Jun 9, 2022

Merge pull request #68 from nathan29849/be/feature/#46-user_remove_fa…

740322e

…vorite feat: [46] 유저 위시리스트 취소 기능 구현

tmdgusya pushed a commit that referenced this pull request Jun 9, 2022

Merge pull request #68 from junu0516/requestLogging

eb09d0d

feat: Http Request 로깅

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Team-26][FE][이든&도리] 1주차 02 PR 요청 #68

[Team-26][FE][이든&도리] 1주차 02 PR 요청 #68

kimyouknow commented May 27, 2022 •

edited

deprecated-hongbiii left a comment

deprecated-hongbiii May 28, 2022

deprecated-hongbiii May 28, 2022

kimyouknow May 31, 2022

deprecated-hongbiii May 28, 2022

deprecated-hongbiii May 28, 2022

deprecated-hongbiii May 28, 2022

deprecated-hongbiii May 28, 2022

[Team-26][FE][이든&도리] 1주차 02 PR 요청 #68

[Team-26][FE][이든&도리] 1주차 02 PR 요청 #68

Conversation

kimyouknow commented May 27, 2022 • edited

PR 피드백

1. GET /search에서 사용하는 location관련 params

2. 필수 선택 미션

진행상황

1. 설계

2. 차트 구현

3. 코딩 마라톤 방식 느낀 점

해결이 되지 않은 점

typescript로 react 컴포넌트 npm에 배포해보기

deprecated-hongbiii left a comment

Choose a reason for hiding this comment

deprecated-hongbiii May 28, 2022

Choose a reason for hiding this comment

deprecated-hongbiii May 28, 2022

Choose a reason for hiding this comment

kimyouknow May 31, 2022

Choose a reason for hiding this comment

deprecated-hongbiii May 28, 2022

Choose a reason for hiding this comment

deprecated-hongbiii May 28, 2022

Choose a reason for hiding this comment

deprecated-hongbiii May 28, 2022

Choose a reason for hiding this comment

deprecated-hongbiii May 28, 2022

Choose a reason for hiding this comment

kimyouknow commented May 27, 2022 •

edited

1. `GET /search`에서 사용하는 location관련 params