-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Feat : custom timezone spec (#723) #721
Conversation
ok to test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
진행한 데까지는 남겨놓고 이어서할게요.
src/js/common/timezone.js
Outdated
|
||
/** | ||
* Set primary timezone code | ||
* @param {string} timezoneCode - timezone code (such as 'Asia/Seoul', 'America/New_York') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"IANA time zone names" 이라고 명시해 주면 좋을 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
timezone name (time zone names of the IANA time zone database, such as 'Asia/Seoul', 'America/New_York')
이렇게 설명하는건어떻할까요?
src/js/common/timezone.js
Outdated
* @param {Timezone} timezoneObj - {@link Timezone} | ||
*/ | ||
function setPrimaryTimezoneByOption(timezoneObj) { | ||
var timezoneCode = timezoneObj.timezone; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
timezone, timeZone의 네이밍이 통일되어 있지 않습니다. 적어도 인터페이스에서는 맞추어야 되겠습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인터페이스 확인 부탁드립니다.
timezone: {
zones: [
{
timezoneName: 'Asia/Seoul',
displayLabel: 'GMT+09:00',
tooltip: 'Seoul'
},
{
timezoneName: 'America/New_York',
displayLabel: 'GMT-05:00',
tooltip: 'New York',
}
],
offsetCalculator: function(timezone, timestamp){
// e.g. +09:00 => -540, -04:00 => 240
return moment.tz.zone(timezone).utcOffset(timestamp);
},
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰 완료합니다. 고생하십니다!
@@ -49,7 +46,7 @@ var differentOffset = { | |||
* @private | |||
*/ | |||
function getTimezoneOffset(timestamp) { | |||
timestamp = timestamp || Date.now(); | |||
timestamp = !util.isUndefined(timestamp) ? timestamp : Date.now(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 이거 빼먹을 뻔했네요! 멋져부러!
기존의 |
네, 하위버전 호환가능하도록 기존에 사용된 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생 많으셨습니다!
Please check if the PR fulfills these requirements
fix #xxx[,#xxx]
, where "xxx" is the issue number)Description
If the system local setting uses a time zone that includes DST, the schedule should always be rendered the same, regardless of whether the time is summer or standard time at the time the calendar instance is created.
The custom timezone spec has been changed. (feat Custom time zone designation option specification definition #723 )
Thank you for your contribution to TOAST UI product. 🎉 😘 ✨