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-33 포키 & bangtae] 2주차 첫번째 pr보냅니다. #145

Merged
merged 17 commits into from
May 31, 2022

Conversation

moonyerim2
Copy link
Collaborator

안녕하세요 33팀 피알보내드립니다!

배포사이트

구현 목록

  • 캘린더 모달 구현
  • 검색바 구현
  • 컴포넌트 반응형으로 수정

에어비앤비가 UX적으로 완성도 높은 사이트라고 해서 그 부분을 많이 참고하면서 구현할 때 반영 해보려고 시도했습니다.
캘린더를 구현할 때 그런 부분들을 반영을 하다보니 CSS를 동적으로 변경해야 하는 경우가 많았는데 그래서 분기를 계속 해야해서 코드가 길어지는 것 같았습니다.
그리고 캘린더 구현할 때 날짜 하나하나를 컴포넌트로 구현했는데 상태가 바뀌면 모든 날짜 컴포넌트가 리렌더링 되어야하는데 이 리렌더링은 괜찮은지 궁금합니다.

bangdler and others added 16 commits May 26, 2022 12:41
* rename: gnb 폴더 생성하여 관련 컴포넌트 이동
   - styled -> customStyled 로 변경 적용
* refactor: gnb 컴포넌트 css 수정
* feat: main banner UI 구현
* refactor: gnb 컴포넌트 hover 및 cursor pointer 적용
* chore: mui 버전 통일

* feat: 글로벌 스타일 추가

* feat: SearchBar 컴포넌트 style 작성 및 이벤트 추가
* chore: alias 경로 설정을 위한 react-app-rewired 설치 및 설정파일 생성
* refactor: 절대경로 반영 (index, app, gnb, main-banner)
* fix: search input width 고정
* 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 적용
- gnb, search-bar, calender를 header 하위 컴포넌트로 수정
* refactor: 캘린더 체크인/체크아웃 모드와 date box 체크 위치 상태를 나타내는 문자열을 상수화
* refactor: 컴포넌트 별 데이터 가공 유틸 함수를 컴포넌트 밖으로 분리
* refactor: Date box 내부에 있던 날짜 비교용 checkInTime, checkOutTime 을 콘텍스트 provider 로 이동
   - checkInDate, checkOutDate 변수명 변경 -> checkInInfo, checkOutInfo
   - checkIn, checkOut, current 변수명 변경 -> checkInTime, checkOutTime, currentTime
* 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가 적용되지 않게 함
* feat: 체크인-체크아웃 사이 날짜 1일과 마지막일 전후 음영 반영
   - DatesOfMonth 컴포넌트 last date 월이 안맞는 오류 수정
     : getDate() 는 현재달 인덱스에 date 0을 주면 이전달 마지막날 반환
* feat: 현재 날짜 이전 날짜 선택 불가 및 회색 표시
* feat: 검색바 체크인, 체크아웃 날짜 출력 기능 추가
- 캘린더에서 선택한 날짜를 출력하도록 함

* Feature/20 (#40)

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

* feat: ResetButton 클릭 이벤트 추가
- 버튼 클릭시 input value 초기화
- 검색바가 포커스되어 있을 때만 버튼이 노출되도록 수정
- ResetButton 포커스 시에만 노출되도록 수정
- 검색바 포커스 시 세로로 늘어나는 오류 수정
- 체크아웃을 먼저 선택 후 체크인 선택 시 체크아웃보다 뒷날이어도 선택되는 오류 수정
- DateBox 컴포넌트 내 handle click 로직 수정
- 체크인 날짜 있는 경우 체크인 달로 시작
- 없는 경우 현재 달로 시작
@moonyerim2 moonyerim2 self-assigned this May 31, 2022
@moonyerim2 moonyerim2 added the review-FE New feature or request label May 31, 2022
Copy link

@choisohyun choisohyun left a comment

Choose a reason for hiding this comment

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

안녕하세요~ 리뷰어 후입니다!
캘린더 넘 수고많으셨습니다 동작 잘되는것도 확인했습니다 👍 👏

css in js를 사용할 때 스타일 코드 내부에서 분기처리, prop들이 많아지는 문제가 있다고 느낍니다 코드가 장황해지는 느낌이죠..
styled component 장단점을 소개해 주는 아티클 몇개 첨부하니 참고해 주세요~

https://itnext.io/css-in-js-vs-pre-post-processors-in-2019-8b1e20c066ed
https://likejirak.tistory.com/71

리랜더링 같은 경우에는 캘린더 모달이 열고 닫힐 때 스타일 영역에 변화가 생겨서 리페인트 되는것으로 보입니다~

Comment on lines +33 to +37
for (let i = 1; i < page; i++) {
const prevYear = displayPageArray[i - 1].year;
const prevMonth = displayPageArray[i - 1].month;
const newDate = prevMonth === 12 ? { year: prevYear + 1, month: 1 } : { year: prevYear, month: prevMonth + 1 };
displayPageArray.push(newDate);

Choose a reason for hiding this comment

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

조금 더 선언형인 map을 사용해 볼수도 있겠네요~

<StyledContainer
page={page}
onMouseDown={e => {
e.preventDefault();

Choose a reason for hiding this comment

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

여기에 preventDefault만 넣으신 이유가 있을까요?? 아무 동작을 하지 않을것 같네요~

Copy link

@bangdler bangdler Jun 1, 2022

Choose a reason for hiding this comment

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

검색바 컴포넌트에 blur 이벤트가 걸려있는데, 캘린더 모달 클릭 시에 검색바 blur 이벤트가 발생하지 않도록 넣었습니다 ㅎㅎ

Comment on lines +58 to +60
`grid-template-columns: ${page > 1 ? `repeat(2, 1fr)` : ``};
column-gap: ${page > 1 ? 20 : 0}px;
row-gap: ${page > 2 ? 40 : 0}px;

Choose a reason for hiding this comment

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

개인적인 의견입니다만 이렇게 분기처리가 많이 들어가는 경우에는 class 명으로 제어하는 편이 좀더 깔끔하지 않나 싶은 생각은 있습니다..!
StyledCalenderPageWrapper 에 class를 추가하고 그 안에서 스타일을 작성하는 식으로요
best practice는 아니고 의견 공유이니 참고만 해주세요!

Copy link

Choose a reason for hiding this comment

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

클래스명을 다르게 주려면 분기가 필요할 것 같은데, 아래와 같이 컴포넌트 내에서 분기처리를 해주는 방식을 말씀하시는 걸까요?

{page >1? <Calender className="page1" /> : <Calender className="page2" />}

Choose a reason for hiding this comment

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

네넵 이런식으로 하거나 styled component만으로 쓰고싶다면 default style을 A로 만들고 B = styled(A), C = styled(A) 이런식으로 해도 될거같아요~

return displayPageArray;
}

const StyledContainer = styled.div`

Choose a reason for hiding this comment

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

시맨틱한 태그를 사용해볼 수 있겠네요 ex) main, section 등

Copy link

Choose a reason for hiding this comment

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

모달창을 만들면서 무의식적으로 대부분 div 로 했네요,, 모달창에 어울리는 시맨틱 태그는 dialog 나 section 인 것 같은데, StyledContainer 를 dialog 나 section 으로 변경해보겠습니다!


useEffect(() => {
const newCheckOutTime =
checkOutInfo === null ? 0 : new Date(`${checkOutInfo.year}-${checkOutInfo.month}-${checkOutInfo.date}`).getTime();

Choose a reason for hiding this comment

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

20라인과 로직 중복입니다! 날짜 정보 객체를 인자로 받아서 Time을 리턴하는 간단한 함수를 만들어봐도 좋겠네요~

Choose a reason for hiding this comment

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

추가적으로 checkInInfo와 checkOutInfo 상태로 하는일이 거의 비슷한 형태이기 때문에 hook으로도 따로 분리시킬 수도 있어 보입니다

return `
position: absolute;
top: 0;
right: -50px;

Choose a reason for hiding this comment

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

위쪽과 스타일이 left, right 빼고는 동일해서 나머지 스타일은 밖으로 빼도 될거같네요

Copy link

Choose a reason for hiding this comment

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

<StyledExpandBackground checkState={checkState} date={date} lastDate={lastDate} />

위 styled component 자체가 조건문 (체크인~체크아웃 사이에 있으면서, 1일이거나 마지막 날일 경우) 에 부합할 때만 css 를 주는 방식으로 구현하다보니 공통 스타일을 밖으로 못뺐습니다.

생각해보니 컴포넌트가 있는데 css가 하나도 없어 렌더링이 안되는 경우가 있다면 헷갈릴 수 있을 것 같아, 분기를 주고 공통 스타일을 분리했습니다.

{isRequiredExpandBackground ? <StyledExpandBackground date={date} lastDate={lastDate} /> : null}

const StyledExpandBackground = styled.div`
  position: absolute;
  top: 0;
  width: 50px;
  height: 50px;
  ${({ date, lastDate }) => {
    if (date === 1) {
      return `
      left: -50px;
      background: linear-gradient(270deg, #F5F5F7 30%, #fff);
      `;
    } else if (date === lastDate) {
      return `
      right: -50px;
      background: linear-gradient(90deg, #F5F5F7 30%, #fff);
      `;
    }
  }}
`;

const year = date.year;
const month = date.month;
const firstDay = new Date(year, month - 1, 1).getDay();
const lastDate = new Date(year, month, 0).getDate();

Choose a reason for hiding this comment

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

이건 여담이긴 한데, 사파리에서는 new Date('2020-03-22')와 같은 형태는 지원이 안되서 작성하신 new Date(2020,3,22) 형태여야 동작합니다 ㅎㅎ

Comment on lines +23 to +31
function getDateArray({ firstDay, lastDate }) {
// 이전달 빈칸
const blanks = Array(firstDay).fill(null);
const dates = Array.from({ length: lastDate }, (_, idx) => idx + 1);

return [...blanks, ...dates];
}

function isBeforeToday({ year, month, date }) {

Choose a reason for hiding this comment

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

이런 함수들 좋네요! 👍

@choisohyun choisohyun merged commit 21c4ad7 into codesquad-members-2022:team-33 May 31, 2022
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

3 participants