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

Refactor/#215 댓글 시간을 시간,일,달 등 자세하게 표시한다 #494

Merged
merged 9 commits into from
Oct 17, 2023

Conversation

cruelladevil
Copy link
Collaborator

@cruelladevil cruelladevil commented Oct 6, 2023

📝작업 내용

이번 PR에서 작업한 내용을 간략히 설명(이미지 첨부 가능)

Intl.RelativeTimeFormat을 활용하여 댓글 시간을 방금 전, , , 시간, , , 으로 자세하게 표시하였습니다.

💬리뷰 참고사항

원활한 리뷰를 위해 전달하고 싶은 맥락(특정 파일, 디렉터리 등등)
리뷰어가 특별히 봐주었으면 하는 부분

  • 1분 미만방금 전으로 표시하도록 하였습니다.
  • 9분 이상 10분 미만은 -> 9분 전으로 표시됩니다.
  • 즉, 상대 시간 기준이 n <= 상대 시간 < n + 1 입니다.
  • 1달을 30일로 계산하기 때문에 n개월 전에는 오차가 약간 있습니다.

테스트에서 확인해 볼 수 있습니다.

  // 3일 전의 최소값
  test('차이가 1주 미만인 경우 n일 전을 반환한다.', () => {
    const createdAt = '2023-10-03T10:00:00';
    const now = '2023-10-06T10:00:00';

    expect(convertRelativeTimeString(createdAt, now)).toBe('3일 전');
  });

  // 1달을 30일로 계산하기 때문에 오차가 약간 있습니다.
  test('차이가 1년 미만인 경우 n개월 전을 반환한다.', () => {
    const createdAt = '2023-06-08T10:00:00';
    const now = '2023-10-06T10:00:00';

    expect(convertRelativeTimeString(createdAt, now)).toBe('4개월 전');
  });

  // 2년 전의 최대값
  test('차이가 1년 이상인 경우 n년 전을 반환한다.', () => {
    const createdAt = '2020-10-06T10:00:00.001';
    const now = '2023-10-06T10:00:00';

    expect(convertRelativeTimeString(createdAt, now)).toBe('2년 전');
  });

#️⃣연관된 이슈

연관된 이슈 번호를 모두 작성

close #215

@cruelladevil cruelladevil added [ 🌞 FE ] 프론트엔드 크루들의 빛나는 개발 이야기 하나둘셋 호! 🔨 Refactor 꾸준한 개선이 더 나은 애플리케이션을 만든다 labels Oct 6, 2023
@cruelladevil cruelladevil self-assigned this Oct 6, 2023
@github-actions
Copy link

github-actions bot commented Oct 6, 2023

Unit Test Results

  85 files    85 suites   11s ⏱️
321 tests 321 ✔️ 0 💤 0
324 runs  324 ✔️ 0 💤 0

Results for commit b3ec4aa.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@Creative-Lee Creative-Lee 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 28 to 31
const cutoffs = [LESS_THAN_MINUTE, MINUTE, HOUR, DAY, WEEK, MONTH, YEAR, MORE_THAN_YEAR];

const unitIndex = cutoffs.findIndex((cutoff) => cutoff > Math.abs(deltaSeconds));
const divisor = cutoffs[unitIndex - 1];
Copy link
Collaborator

Choose a reason for hiding this comment

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

💬 로직을 이해하는게 조금 어려웠는데요! 제가 이해한게 맞는지 확인 부탁드려요.

  1. getTime() - getTime() 의 몫을 1000ms 로 나눠서 초단위로 변환합니다.
  2. cutoffs 단위들을 순회하면서, 위 1번의 초단위 변환 값이 단위보다 작을때 index를 반환합니다.
  3. 위 2번에서 반환된 index에 -1을 적용한 index에 해당하는 단위가 divisor가 됩니다.
  4. 60초 미만이면 '방금전', 아니면 포멧한 결과

Copy link
Collaborator Author

@cruelladevil cruelladevil Oct 15, 2023

Choose a reason for hiding this comment

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

넵 맞습니다! 기준이 n 이상 n + 1 미만이기 때문에
n + 1이 되는 index (cutoff > Math.abs(deltaSeconds))를 찾으면 index - 1의 값이 해당 단위를 만드는 divisor입니다.
이전에는 설명이 잘못 되어 있어 나누는 divisor들이 1초를 기준으로 곱해져 있었고 현재는 ms로 수정되어있습니다.

Comment on lines 1 to 10
// NOTE: ms단위입니다. (SECOND - 1000ms)
const LESS_THAN_MINUTE = 0;
const SECOND = 1000;
const MINUTE = 60;
const HOUR = 60 * MINUTE;
const DAY = 24 * HOUR;
const WEEK = 7 * DAY;
const MONTH = 30 * DAY;
const YEAR = 365 * DAY;
const MORE_THAN_YEAR = Infinity;
Copy link
Collaborator

Choose a reason for hiding this comment

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

💬 요거 개인적으로는 살짝 헷갈렸습니다 ㅋㅋㅋㅋ...

ms단위입니다 주석 보고 내려가는데,
LESS_THAN_MINUTE = 0; -- 이게 뭘까?
SECOND = 1000 -- 음 ms 단위군.
Minute = 60 -- 음? 1초 단위군?
이런 느낌이었습니당

SECOND는 아래 cutoffs 단위에 안 들어가고,
LESS_THAN_MINUTE은 cutoffs 단위이니깐,
순서를 살짝 바꾸면 이해가 더 잘될것 같기도 해요.

// NOTE: ms단위입니다. (SECOND - 1000ms)
const SECOND = 1000;
const LESS_THAN_MINUTE = 0;
...

Copy link
Collaborator Author

@cruelladevil cruelladevil Oct 15, 2023

Choose a reason for hiding this comment

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

2가지 방안이 있는데 의견주세용~!

1번 방안

deltaSeconds를 deltaTime으로 바꾼다.

// NOTE: 상수는 모두 ms 단위입니다.
const SECOND = 1000;

// NOTE: 아래는 cutoff를 위한 상수입니다.
const LESS_THAN_MINUTE = 0;
const MINUTE = 60 * SECOND;
const HOUR = 60 * MINUTE;
const DAY = 24 * HOUR;
const WEEK = 7 * DAY;
const MONTH = 30 * DAY;
const YEAR = 365 * DAY;
const MORE_THAN_YEAR = Infinity;

const deltaTime = createdTime - nowTime;

2번 방안

기존과 동일하며 주석만 바뀐다.

// NOTE: 1000ms입니다. 댓글 생성 시간과 현재 시간 차이를 초로 변환하기 위해 나누는 용도입니다.
const SECOND = 1000;

// NOTE: cutoff를 위한 초(s) 단위입니다.
const LESS_THAN_MINUTE = 0;
const MINUTE = 60;
const HOUR = 60 * MINUTE;
const DAY = 24 * HOUR;
const WEEK = 7 * DAY;
const MONTH = 30 * DAY;
const YEAR = 365 * DAY;
const MORE_THAN_YEAR = Infinity;

const deltaSeconds = (createdTime - nowTime) / SECOND;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

1번 방안으로 채택되었습니다 👍

Copy link
Collaborator

@ukkodeveloper ukkodeveloper left a comment

Choose a reason for hiding this comment

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

잘 만들어주신 것 같습니다!!! 옆에서 잘 확인했습니다. 바로 어쁘루브 할게요!

];
const cutoffs = [LESS_THAN_MINUTE, MINUTE, HOUR, DAY, WEEK, MONTH, YEAR, MORE_THAN_YEAR];

const unitIndex = cutoffs.findIndex((cutoff) => cutoff > Math.abs(deltaTime));
Copy link
Collaborator

Choose a reason for hiding this comment

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

[전달 완료 / 기록용] 머리 정말 잘 쓰신 것 같아요. 💬 주석으로 cutoffs가 오름차순으로 정렬되어있고, 순차적으로 확인한다는 내용이 있으면 조금 더 이해하는 데 쉬울 것 같다는 생각도 드네요

Copy link
Collaborator Author

@cruelladevil cruelladevil Oct 16, 2023

Choose a reason for hiding this comment

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

[전달 완료 / 기록용]이 뭔가 했네용 🤔ㅋㅋ
상대 시간 기준임을 빠르게 이해할 수 있도록 주석 추가하였습니다.

✍️ 추가 커밋 - 39ab3d5

];
const cutoffs = [LESS_THAN_MINUTE, MINUTE, HOUR, DAY, WEEK, MONTH, YEAR, MORE_THAN_YEAR];

const unitIndex = cutoffs.findIndex((cutoff) => cutoff > Math.abs(deltaTime));
Copy link
Collaborator

Choose a reason for hiding this comment

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

[전달 완료 / 기록용]
const unitIndex = cutoffs.findIndex((cutoff) => cutoff > Math.abs(deltaTime)) - 1; 로 표현하면 뒤 로직에서 index를 변경하여 접근하지 않아서 좋은 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍👍 변수명도 기준이 되는 단위 인덱스인 unitIndex이니 우코 말대로 unitIndex에서 -1 해주어야 하겠네요!

✍️ 수정 커밋 - 9605872

@cruelladevil cruelladevil merged commit 8c6d7d1 into main Oct 17, 2023
@cruelladevil cruelladevil deleted the refactor/#215 branch October 17, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[ 🌞 FE ] 프론트엔드 크루들의 빛나는 개발 이야기 하나둘셋 호! 🔨 Refactor 꾸준한 개선이 더 나은 애플리케이션을 만든다
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[REFACTOR] 댓글 시간을 시간,일,달 등 자세하게 표시한다
3 participants