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

[Fix] 연도가 다른 타임 블록 배치 #210

Merged
merged 21 commits into from
Aug 7, 2024

Conversation

namdaeun
Copy link
Member

@namdaeun namdaeun commented Aug 3, 2024

해당 이슈 번호

closed #205


체크리스트

  • 🔀 PR 제목의 형식을 잘 작성했나요? e.g. [feat] PR을 등록한다.
  • 💯 테스트는 잘 통과했나요?
  • 🏗️ 빌드는 성공했나요?
  • 🧹 불필요한 코드는 제거했나요?
  • ✅ 컨벤션을 지켰나요?
  • 💭 이슈는 등록했나요?
  • 🏷️ 라벨은 등록했나요?
  • 💻 git rebase를 사용했나요?
  • 🙇‍♂️ 리뷰어를 지정했나요?

📌 내가 알게 된 부분

다른 연도에 타임블록이 걸쳐있는 경우

  if (blockStartDate.getFullYear() === currentYear && blockEndDate.getFullYear() === currentYear) {
... 생략
  } else {  // 추가
    if (startMonth <= selectedMonth || selectedMonth <= endMonth) {
      blockStartDate =
        startMonth === selectedMonth && blockStartDate.getFullYear() === currentYear ? blockStartDate : firstDay;
      blockEndDate = endMonth === selectedMonth && blockEndDate.getFullYear() === currentYear ? blockEndDate : lastDay;
    }
  }
  • 기존 코드에서는 같은 연도일 때 (상단의 if문)만 고려를 해놔서 else문을 추가해주었습니다.
  • 블록의 시작 달과 선택한 달이 같은 경우, 즉 selectedMonth === startMonth의 경우 블록이 시작하는 연도도 고려해서 연도까지 동일한 경우가 블록이 시작하는 날짜이기 때문에 blockStartDate로 설정해주었습니다.
  • blockEndDate를 할당하는 방식도 동일합니다.

daySectionRef.current?.scrollTo(0, 0);
  • 연도와 달이 바뀔 때 왼쪽부터 볼 수 있도록 스크롤 위치를 설정해줬습니다 !

  • 기존에 선언되어 있던 block_area 식별자를 가져와서 달과 연도가 바뀔 때 핸들러에 다음 코드를 추가해줬습니다.
    -> 변경된 코드처럼 ref를 가져와서 바꿔주는 방향으로 수정했습니다!

  • 그리고 연도가 바뀔 때 달이 1월로 초기화되지 않길래 해당 코드도 setState 있던 거 넘겨줘서 1월로 설정해줬습니다!


📌 질문할 부분

  • 영상 찍다보니 무진장 깜박거리네요 나중에 LazyComponent 설정해서 타임라인도 스켈레톤 설정해주면 UX에 좋을 듯 함니다

📌스크린샷 (선택)

2024-08-04.2.32.50.mov

Copy link
Contributor

@wuzoo wuzoo left a comment

Choose a reason for hiding this comment

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

굳굳이네요 ! 고생하셧습니다 !

Comment on lines 17 to 24
setSelectedMonth('1월');
document.getElementById('block_area')?.scrollTo(0, 0);
};

const handleNextYear = () => {
setCurrentYear((prevYear) => prevYear + 1);
setSelectedMonth('1월');
document.getElementById('block_area')?.scrollTo(0, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

이렇게 쓸거라면 block_areadiv요소에 ref를 하나 처리해주는 것은 어떨까요 ? 렌더링에 관련 없이 해당 요소의 참조를 기억하고 싶은 거니까요 !

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 좋습니다 ! id 중복 방지하는 측면에서도 id를 가져와서 사용하는 것보다 ref를 전달해주는 게 좋겠네요👍

Copy link
Contributor

@rtttr1 rtttr1 left a comment

Choose a reason for hiding this comment

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

LGTM! 고생하셨어유!!

Copy link
Contributor

@Bowoon1216 Bowoon1216 left a comment

Choose a reason for hiding this comment

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

점점 알고리즘의 신이 되어가고 있는 다은님 수고하셨습니다 👍

@@ -4,7 +4,7 @@ import { endOfMonth } from 'date-fns';

import { useState } from 'react';

export const useDate = () => {
export const useDate = (ref: React.RefObject<HTMLDivElement>) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

ref를 인자로 받게 한다면, useDate 커스텀 훅의 재사용성이 조금 낮아질 것 같다는 생각인데, 다른 대안은 없을까요 ?

Copy link
Member Author

@namdaeun namdaeun Aug 7, 2024

Choose a reason for hiding this comment

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

재사용성이 낮아진다는 의견에 동의해요!
옵셔널로 받아서 아무런 값을 인자로 받지 않아도 가능하게끔 수정했습니다!

@@ -73,6 +76,11 @@ const ArchivingPage = () => {
setCurrentContent(<UploadModal onClose={closeModal} teamId={+teamId} type={type} blockData={blockData} />);
};

const handleMonthClick = (month: MonthType) => {
setSelectedMonth(month);
daySectionRef.current?.scrollTo(0, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

요고는 의도가 무엇이죠 ???

Copy link
Member Author

@namdaeun namdaeun Aug 5, 2024

Choose a reason for hiding this comment

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

달이 바뀔 때 스크롤 됐던 부분을 좌측으로 스크롤해주고자 작성해주었습니다 !

Comment on lines 53 to 59
useEffect(() => {
if (!isMounted.current) {
isMounted.current = true;
return;
}
setSelectedMonth(`${currentDate.getMonth() + 1}월` as MonthType);
}, [teamId]);
Copy link
Contributor

Choose a reason for hiding this comment

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

return이 아닌, current 값을 true로 바꾸고 조건문 안에서 setter를 실행해야 올바른 동작인 것 같습니다 ! 마운트되었다면 실행이 안돼야 하니까요 !

마운트까지 고려하셔서 useEffect를 적용하신거 너무 보기 좋네요 :)

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 수정하겠습니다 !! 감사합니당

Copy link
Contributor

@cindy-chaewon cindy-chaewon left a comment

Choose a reason for hiding this comment

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

점점 타임블록이 완벽 해지는 거 같아서 좋습니당!! 스크롤 통해서 사용자 경험 도 개선하는 코드도 짜고..!! 최고입니다!!

@namdaeun namdaeun merged commit 759c740 into develop Aug 7, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

타임라인 1년 타임블록 이슈
5 participants