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-22/ 도비 & JS] 1주차 수요일 PR #49

Merged
merged 32 commits into from
May 26, 2022

Conversation

JiminKim-dev
Copy link
Collaborator

@JiminKim-dev JiminKim-dev commented May 25, 2022

안녕하세요 구디! 도비와 JS입니다.
3주 동안 잘 부탁드립니다!

진행 사항

2022-05-25.6.37.20.mov

앞으로의 계획

  • 요구사항 분석하기
  • 에어비앤비의 서치바를 참고하여 클릭 기능을 추가할 예정입니다.
  • 캘린더 구현 방법을 생각하고, 캔버스 사용 방법을 학습할 예정입니다.

궁금한점

  • 아직 MUI가 익숙치않아서 styeld-component와 혼용하는 방식으로 진행하고 있습니다. 이 방법을 계속해서 사용하는게 좋을까요 아니면 MUI만 쓰도록 리팩토링을 하는게 좋을까요?
  • 타입스크립트를 사용한 프로젝트이지만 props를 받지않고 뷰만 존재하는 컴포넌트 파일의 확장자는 jsx로 하는것이 좋을까요?

bugpigg and others added 28 commits May 23, 2022 17:19
Build : 초기 환경 세팅, 필요한 라이브러리 설치
material 페키지 설치
globalStyle 설정
아이콘, 배경 다운로드
Co-authored-by: herrakam <herrakam@users.noreply.github.com>
Co-authored-by: herrakam <herrakam@users.noreply.github.com>
Co-authored-by: herrakam <herrakam@users.noreply.github.com>
- plugin:react/jsx-runtime 추가

Co-authored-by: herrakam <herrakam@users.noreply.github.com>
Co-authored-by: herrakam <herrakam@users.noreply.github.com>
Co-authored-by: herrakam <herrakam@users.noreply.github.com>
- result 페이지의 미니 서치바를 클릭하면 큰 서치바로 변함

Co-authored-by: herrakam <herrakam@users.noreply.github.com>
- 기능은 작동하지 않음
- 요금 범위 슬라이더는 아직 구현하지 않음

Co-authored-by: herrakam <herrakam@users.noreply.github.com>
- 서치바의 배경색 추가
- PriceModal을 display: none으로 설정
- 로고를 클릭하면 메인페이지로 이동
- 마이페이지 아이콘들을 버튼 태그안에 넣음
@BB-choi BB-choi added the review-FE New feature or request label May 25, 2022
@BB-choi BB-choi requested a review from junzero741 May 25, 2022 14:40
Copy link

@junzero741 junzero741 left a comment

Choose a reason for hiding this comment

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

도비 & JS, 안녕하세요!
리뷰를 맡은 Goody 라고 합니다.
이번 프로젝트는 기간이 3주나 된다고 들었는데요!
앞으로 같이 재밌는 프로젝트 만들어가셨으면 좋겠습니다. ㅎㅎㅎ
저도 리뷰로 도울 수 있는 만큼은 최대한 도와드릴 수 있도록 해보겠습니다!

제가 듣기로는 필수 선택 미션이 아래 셋 중 하나라고 하던데, 혹시 어떤 미션을 선택해서 개발할 예정이신지 여쭤봐도 괜찮을까요?

  • 라우터를 직접 개발한다.
  • CSS-in-Js 를 직접 개발한다.
  • 글로벌 상태관리 라이브러리를 직접 개발한다.
  • 캘린더 컴포넌트를 재사용 가능한 방식으로 개발해서 NPM 에 올린다.

타입스크립트를 사용한 프로젝트이지만 props를 받지않고 뷰만 존재하는 컴포넌트 파일의 확장자는 jsx로 하는것이 좋을까요?
타입스크립트를 주로 사용하는 프로젝트라면 컴포넌트 파일은 tsx로 작성하는 것이 일관성 유지를 위해 좋을 것 같습니다! 뷰만 존재하는 컴포넌트 파일의 확장자를 jsx로 하고 싶으신 이유가 궁금합니다!

"react-scripts": "5.0.1",
"styled-components": "^5.3.5",
"styled-reset": "^4.4.1",
"typescript": "^4.6.4",

Choose a reason for hiding this comment

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

typescript 를 dependencies에 넣으신 의도가 궁금합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

CRA로 프로젝트를 생성 했을때 자동적으로 dependencies에 설치되었습니다!

@@ -0,0 +1,7 @@
import React from 'react';

Choose a reason for hiding this comment

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

React 17 버전 이상부터는 굳이 React 를 임포트 해오지 않아도 되긴 합니다..ㅎㅎ

Choose a reason for hiding this comment

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

네 삭제하겠습니다!

Comment on lines +10 to +15
<ThemeProvider theme={globalTheme}>
<CssBaseline />
<StyledThemeProvider theme={StyledTheme}>
<Router />
</StyledThemeProvider>
</ThemeProvider>

Choose a reason for hiding this comment

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

크롱이 MUI를 쓰라고 하신 건 스타일링에서 너무 시간 뺏기지 말라고 의도하신 듯 한데요.
사실 MUI와 styeld-component와 혼용을 하는 것 자체는 문제될 게 없어 보이는데
프로젝트 전체로 놓고 봤을 땐 추후에 이게 MUI에서 가져온 스타일인지 style-components 로 관리하는 스타일인지 헷갈릴 소지가 있을 것 같아요!

Choose a reason for hiding this comment

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

그래도 MUI에서 가져온 컴포넌트 중 커스텀이 필요한 아이들은 같이 쓰는 것도 괜찮아 보입니다.

Choose a reason for hiding this comment

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

MUI로 가져온 컴포넌트는 styled-components와는 네이밍을 다르게 해서 구분하겠습니다!

Comment on lines +28 to +33
text-decoration: none;

&:hover {
background: none;
${({ theme }) => theme.fontStyles.bold16px};
text-decoration: none;

Choose a reason for hiding this comment

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

text-decoration: none 스타일이 두 곳에 다 들어가는 이유가 궁금합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MUI의 Link 컴포넌트가 기본적으로 text-decoration을 가지고 있어서 스타일을 추가했습니다.
hover의 text-decoration은 제가 디자인을 착각해서 실수로 넣었습니다 수정하도록 하겠습니다!

Comment on lines +14 to +28
const CustomBreadcrumbs = styled(Breadcrumbs)`
&& {
margin: 0 auto;
margin-bottom: 32px;
border: none;
}
`;

const CustomLink = styled(Link)`
&& {
border: none;
padding: 0 6px;
color: ${({ theme }) => theme.colors.black};
${({ theme }) => theme.fontStyles.normal14px};
text-decoration: none;

Choose a reason for hiding this comment

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

화면 중앙 상단에 링크를 일정 간격으로 띄워놓고 싶은 거라면, display: flex 속성을 써봐도 괜찮을 것 같습니다!

Comment on lines +1 to +13
import styled from 'styled-components';
import { ModalWrap } from './styled';

function PeriodModal() {
return <PeriodModalWrap>기간모달</PeriodModalWrap>;
}

const PeriodModalWrap = styled(ModalWrap)`
width: 916px;
height: 512px;
display: none;
`;
export default PeriodModal;

Choose a reason for hiding this comment

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

검색창 하단에 모달이 떠서 폴더 구조를 SearchBar/Modals/ 로 잡으신 것 같은데요.
당장의 요구사항에는 모달이 검색창 하단에만 뜬다고 되어있겠지만,
진짜로 에어비앤비 서비스를 개발한다고 생각해봤을 때
기간을 나타내는 모달이 검색창 하단에만 뜨리라고 속단하기는 어려울 것 같습니다.
그래서 저라면 모달과 관련한 컴포넌트들을 모아놓는 폴더를 페이지와 독립적으로 만들 것 같아요!

Comment on lines +14 to +23
const PersonnelInfo = PERSONNEL_INFO.map((info, index) => {
return (
<>
<PersonnelModalWrap key={info.id} info={info} />
{index !== PERSONNEL_INFO.length - 1 && (
<Divider style={{ width: '100%', margin: '24px 0' }} />
)}
</>
);
});

Choose a reason for hiding this comment

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

map 으로 반복되는 마크업을 피하신 것 너무 좋은 것 같습니다. 수정이 용이할 것 같아요

Choose a reason for hiding this comment

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

다만 이 파일 내에서만 Divider에 커스텀 스타일을 적용하고 싶으신 거라면 styled(Divider)`` 를 쓰는게 좀 더 return 문이 깔끔해지지 않을까 싶습니다 ㅎㅎ

Choose a reason for hiding this comment

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

styled를 사용해서 바꿨습니다!

const StyledDevider = styled(Divider)`
  width: 100%;
  margin: 24px 0;
`;

Comment on lines +14 to +18
<CheckIn>체크인</CheckIn>
<Btn aria-label="체크인 날짜 입력 버튼">날짜 입력</Btn>
</CommonWrapper>
<CommonWrapper>
<CheckOut>체크아웃</CheckOut>

Choose a reason for hiding this comment

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

화면상으론 <CheckIn><Checkout> 별다른 점이 없어 보이는데요. 컴포넌트를 굳이 나누신 이유가 있을까요?

Choose a reason for hiding this comment

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

하나로 묶을 네이밍이 생각나지 않아서 컴포넌트를 분리하고 스타일은 하나를 상속받아서 만들었습니다. 적합한 이름을 사용해서 하나의 컴포넌트로 바꿔보겠습니다!

}

return (
<SearchWrap onMouseLeave={() => mouseLeave()}>

Choose a reason for hiding this comment

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

여기는 mouseLeave 함수에 무언가 따로 인자를 전달해줄 게 아니라면 굳이 콜백으로 작성하신 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

JSX props should not use functions react/jsx-no-bind 라는 오류 때문에 콜백으로 작성했습니다!

const [miniFocus, setMiniFocus] = useState(true);

const changeSearchBar = () => {
return miniFocus ? setMiniFocus(false) : setMiniFocus(true);

Choose a reason for hiding this comment

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

miniFocus 값이 true 면 false로, false면 true로 바꿔주고 싶은 거라면, 아래와 같은 방법도 있습니다!

setMiniFocus(s => !s);

Choose a reason for hiding this comment

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

와 엄청 간단하네요 저 방법으로 바꿔보겠습니다!

@JiminKim-dev
Copy link
Collaborator Author

제가 듣기로는 필수 선택 미션이 아래 셋 중 하나라고 하던데, 혹시 어떤 미션을 선택해서 개발할 예정이신지 여쭤봐도 괜찮을까요?

  • 재사용 가능한 캘린더 컴포넌트 만들기를 도전해 볼 예정입니다!

타입스크립트를 주로 사용하는 프로젝트라면 컴포넌트 파일은 tsx로 작성하는 것이 일관성 유지를 위해 좋을 것 같습니다! 뷰만 존재하는 컴포넌트 파일의 확장자를 jsx로 하고 싶으신 이유가 궁금합니다!

  • 타입스크립트의 문법을 사용해야지만 tsx 확장자를 사용하는 줄 알았습니다.. 🥲

리뷰를 바탕으로 수정해보도록 하겠습니다 리뷰 감사합니다!!

@street62 street62 merged commit 0b8f894 into codesquad-members-2022:team-22 May 26, 2022
sabgilhun pushed a commit that referenced this pull request Jun 10, 2022
[feat] 예약 목록화면 구현
naneun pushed a commit that referenced this pull request Jun 10, 2022
[iOS] 하단 툴바의 '건너뛰기' '지우기' '다음' 버튼을 사용할 수 있음
choisohyun pushed a commit that referenced this pull request Jun 12, 2022
* Feature/3 (#26)

* rename: gnb 폴더 생성하여 관련 컴포넌트 이동
   - styled -> customStyled 로 변경 적용
* refactor: gnb 컴포넌트 css 수정
* feat: main banner UI 구현
* refactor: gnb 컴포넌트 hover 및 cursor pointer 적용

* Feature/2 (#27)

* chore: mui 버전 통일

* feat: 글로벌 스타일 추가

* feat: SearchBar 컴포넌트 style 작성 및 이벤트 추가

* Feature/alias (#28)

* chore: alias 경로 설정을 위한 react-app-rewired 설치 및 설정파일 생성
* refactor: 절대경로 반영 (index, app, gnb, main-banner)
* fix: search input width 고정

* Feature/4 (#29)

* feat: calender page component 구현
* feat: 달력 월~일요일 부분 컴포넌트 구현
* feat: 해당 연-월에 맞는 날짜 영역 컴포넌트 구현
* feat: 이전, 다음 버튼 컴포넌트 및 클릭 기능 구현
* feat: calender 컴포넌트 구현
   - 보여줄 달력 개수를 정하는 page 옵션
   - curData 기반으로 page 만큼의 달력 렌더링
* chore: 브라우저 확인용 App 에 calender 반영
* feat: calender 관련 상태 context 사용하여 분리 및 provider 컴포넌트 생성
   - 불필요한 주석 제거
* refactor: 컴포넌트 이름 변경, DatesOfMonth 에서 DateBox 컴포넌트 분리
* feat: DateBox 컴포넌트 구현
   - 클릭 시 checkIn, checkOut 상태 변경
   - checkIn, checkOut 날짜에 따라 동적 css 적용
* chore: 오타 수정, app.js 에 provider 적용

* feat: header 컴포넌트 생성
- gnb, search-bar, calender를 header 하위 컴포넌트로 수정

* Feature/30 (#32)

* refactor: 캘린더 체크인/체크아웃 모드와 date box 체크 위치 상태를 나타내는 문자열을 상수화
* refactor: 컴포넌트 별 데이터 가공 유틸 함수를 컴포넌트 밖으로 분리
* refactor: Date box 내부에 있던 날짜 비교용 checkInTime, checkOutTime 을 콘텍스트 provider 로 이동
   - checkInDate, checkOutDate 변수명 변경 -> checkInInfo, checkOutInfo
   - checkIn, checkOut, current 변수명 변경 -> checkInTime, checkOutTime, currentTime

* refactor: 캘린더 month 빈배열 생성 시 0 -> Null 로 변경

* Feature/31 (#33)

* refactor: 코드 리뷰 반영
- visibility 속성 display로 변경
- bool 타입 결과값 함수 중복 검사 제거
- SearchMenu 컴포넌트 div 태그로 변경

* feat & refactor: SearchBar 컴포넌트 컨텍스트 추가
- props로 내려주던 상태 context로 관리 하도록 함

* feat : 검색바 반응형 컴포넌트로 수정

* refactor: isFocus를 상태 관리에서 제외
- currentInput의 상태를 통해 얻을 수 있도록 함

* feat&refactor: Header 컴포넌트 반응형으로 수정
- GNB, MainBanner, SearchBar 모두 적용

* feat: 체크인, 체크아웃 인풋 영역 클릭 시 캘린더 모달 팝업 기능 추가

* feat: Header 컴포넌트 fixed 속성 추가

* feat: 모달창 클릭시 검색바 blur가 적용되지 않게 함

* Feature/34 (#36)

* feat: 체크인-체크아웃 사이 날짜 1일과 마지막일 전후 음영 반영
   - DatesOfMonth 컴포넌트 last date 월이 안맞는 오류 수정
     : getDate() 는 현재달 인덱스에 date 0을 주면 이전달 마지막날 반환
* feat: 현재 날짜 이전 날짜 선택 불가 및 회색 표시

* feat: 검색바 체크인, 체크아웃 날짜 출력 기능 추가 (#38)

* feat: 검색바 체크인, 체크아웃 날짜 출력 기능 추가
- 캘린더에서 선택한 날짜를 출력하도록 함

* Feature/20 (#40)

* refactor: ResetButton 동적으로 추가하도록 수정
- input요소의 value 유무에 따라 추가, 삭제

* feat: ResetButton 클릭 이벤트 추가
- 버튼 클릭시 input value 초기화
- 검색바가 포커스되어 있을 때만 버튼이 노출되도록 수정

* feat: 검색바 캘린더 체크인 체크아웃 상태 연동 (#41)

* fix: 포커스 이벤트 에러 수정
- ResetButton 포커스 시에만 노출되도록 수정
- 검색바 포커스 시 세로로 늘어나는 오류 수정

* fix: 캘린더 날짜 선택 오류 수정
- 체크아웃을 먼저 선택 후 체크인 선택 시 체크아웃보다 뒷날이어도 선택되는 오류 수정
- DateBox 컴포넌트 내 handle click 로직 수정

* feat: 캘린더 모달 선택 시 보여지는 달 업데이트
- 체크인 날짜 있는 경우 체크인 달로 시작
- 없는 경우 현재 달로 시작

* fix: 캘린더 토요일과 날짜 정렬

* Feature/15 (#42)

* refactor: 모달 배경 관련 스타일 별도 컴포넌트로 분리

* rename: custom-styled-component 폴더 utils 하위로 이동
- Header.js => Header.jsx 로 변경

* rename : calender 폴더 modal 하위 폴더로 이동
- 절대 경로 설정 및 적용

* refactor: ModalContainer 스타일 theme 사용 하도록 수정

* feat: Personnel 모달 컴포넌트 추가

* Feature/7 (#43)

* rename: context 폴더 분리
- provider 모두 context 폴더 하위로 이동

* feat: 인원 설정 모달 컨트롤러 버튼 컴포넌트 분리

* feat: 검색바 인원 상태 관리를 위한 관련 상수 추가

* feat: 검색바 인원 설정 모달 인원 추가, 인원 삭제 기능 추가

* feat: 검색바에 선택된 인원 출력 기능 추가 (#44)

* feat: 검색바에 선택된 인원 출력 기능 추가

* Feature/11 (#45)

* feat: 성인 없이 어린이, 유아 인원 추가 방지 기능 추가

* fix: 성인 인원을 줄일 수 없는 상황에 버튼 비활성화 색상 스타일 적용

* refactor: ResetButton 컴포넌트 onClick 핸들러 props로 받도록 수정

* feat: 인원 설정 초기화 기능 추가

* refactor&style: 중복코드 삭제, 줄띄움

* fix: 검색바 초기화 버튼 유무따라 크기가 변경되는 부분 수정
- 검색 버튼 아이콘 컬러 변경

* fix: 검색바 사이즈 줄면 초기화 버튼이 검색 버튼에 가리는 오류 수정

* Feature/46 (#47)

* refactor: 중복 코드 함수화, StyledExpandBackground 분기문
   - info 를 받아 time 으로 바꿔주는 함수
   - 체크인-체크아웃 사이에 있고, 1일이거나 마지막날일 경우에만 StyledExpandBackround 컴포넌트 생성

* feat: calender provider 에 prev date, next date 추가
   - 불필요한 useEffect 코드 제거

* feat: calender 컴포넌트에서 page 생성 부분을 calender carousel 컴포넌트로 이동
   - calender position 상태 생성

* chore: calender page width 수정(336 -> 350px)

* feat: 이전, 다음달 이동 버튼 로직 수정
   - curDate 를 직접 바꾸는 방식에서 calender position 만 바꿔주는 방식
   - calender position 에 따라 carousel에서 translateX 후 curDate 변경

* feat: Calender Carousel 컴포넌트 구현
   - prevDate, curDate, nextDate 기준으로 각각 Calender page wrapper 로 묶음
   - calender display 영역 내에서 moveArea 를 이동

* fix: 검색바 모달 관련 인풋창 아래에 나타나도록 위치 수정

* Feature/15 1 (#49)

* feat: Price 컴포넌트 생성
- Title 컴포넌트 재사용을 위해 분리

* refactor: 검색바 블러 이벤트 대신 dim레이어 클릭 시 모달이 사라지도록 수정
- e.preventDefault 때문에 range input의 thumb가 움직이지 않아 수정

* feat: 요금 설정 모달 요금별 데이터 그래프 추가
- 임시 데이터 추가

* feat: 요금 범위 설정 슬라이더 추가

* chore: chart.js 설정 추가

* fix: MainBanner 이미지 연결 오류 수정

* feat: 요금 모달 세부 텍스트 출력

* feat: 검색바에 요금 상태 출력
- 초기화 버튼 이벤트 핸들러 추가

* fix: merge 후 모달창이 관련 인풋창 아래에 위치하지 않는 오류 수정

* refactor: 인원 모달 컨트롤 버튼 하나의 컴포넌트로 합침
- type으로 add, remove 구분하도록 props 추가

* refactor: calender Prev Button, Next Button 을 통합한 Calender Button 구현 (#50)

Co-authored-by: moon-yerim <75062526+moonyerim2@users.noreply.github.com>

* refactor: 모달창 클릭 시 닫히지 않도록 하는 기능 수정
- dim레이어 삭제
- Node.contains를 사용한 방법으로 수정

Co-authored-by: bangdler <90082464+bangdler@users.noreply.github.com>
Co-authored-by: bangdler <zbthz90@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-FE New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants