Skip to content

fix: adjust start & end of event when system timezone is using DST #1222

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

Merged
merged 2 commits into from
Jul 22, 2022

Conversation

adhrinae
Copy link
Contributor

Please check if the PR fulfills these requirements

  • It's the right issue type on the title
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)
  • It does not introduce a breaking change or has a description of the breaking change

Description

While rendering events, we need to calculate the offset difference if the system timezone is applicable to DST.

So we need to calculate it under the circumstance below:

  1. When the system timezone is using DST
  2. When the system timezone is not using DST(but applicable)

Thank you for your contribution to TOAST UI product. 🎉 😘 ✨

@adhrinae adhrinae requested review from heenakwag and lja1018 July 22, 2022 06:41
@@ -32,7 +32,7 @@ export function calculateTimezoneOffset(timezoneName: string, targetDate: TZDate
'Intl.DateTimeFormat is not fully supported. So It will return the local timezone offset only.\nYou can use a polyfill to fix this issue.'
);

return -targetDate.getTimezoneOffset();
return -targetDate.toDate().getTimezoneOffset();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TZDategetTimezoneOffsetDategetTimezoneOffset 과 약간 다른데, 이 때문에 문제가 제대로 해결되지 않고 있었습니다. 그래서 시스템의 타임존을 정확하게 꺼낼 필요가 있기 때문에 toDate 로 전환을 해 주었습니다.

@@ -170,6 +174,86 @@ DaylightSavingTimeTransitionSouthern.args = {
},
};

// NOTE: For manual testing purposes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

시스템의 타임존을 모킹하는 동작이 자동화된 테스트 환경에서는 어려웠기 때문에 직접 테스트용 페이지를 작성하여 확인했습니다.

@@ -17,6 +19,7 @@ export function useEventsWithTimezone(events: Collection<EventModel>) {
return events;
}

const isSystemUsingDST = isUsingDST(new TZDate());
Copy link
Contributor Author

@adhrinae adhrinae Jul 22, 2022

Choose a reason for hiding this comment

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

isUsingDST 는 두 번째 인자로 타임존 이름을 넣지 않을 경우 현재 시스템을 기준으로 해당 시간의 DST 적용 여부를 결정합니다.

이를 통해 브라우저가 DST 적용중인지 아닌지 확인할 수도 있고, 어떤 이벤트가 시스템 타임존 기준으로는 DST 적용을 받고 있는지 아닌지 확인할 수 있습니다.

Copy link
Member

@heenakwag heenakwag left a comment

Choose a reason for hiding this comment

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

리뷰 완료합니다.

@adhrinae adhrinae merged commit 95b6e12 into main Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants