Fix: Timezone dropdown label does not account for DST #584
Conversation
This implements a GMT offset string that is DST aware, and is prepended to the label value.
…s supported by most modern browsers, so should be safe.
niranjan-uma-shankar
changed the title
Uses longOffset to simplify the regex match and code.
There was a problem hiding this comment.
@niranjan-uma-shankar before I dig too deep into this - could you add some tests for the new functions? Thanks!
packages/timezone-data/src/index.ts
Outdated
| }; | ||
|
|
||
| const labelWithGMTOffset = (label: string, offsetString: string): string => { | ||
| return '(' + offsetString + ') ' + label; |
There was a problem hiding this comment.
nit: could use a template literal here to stay consistent with L308
There was a problem hiding this comment.
Makes sense, I've updated with this change.
…inutes (2) added a try/catch block for graceful failure
@troyciesco I've added tests now. |
|
@niranjan-uma-shankar just noting i haven't forgotten about this - finishing up a couple other projects currently, then i'll be able to give more careful attention to the group of timezone PRs here and in the main repo so everything gets updated smoothly 👍 |
|
@troyciesco Just circling back on this one. If you get a moment to share feedback, happy to make any further adjustments, if needed. |
| .sort((a, b) => a.offsetMinutes - b.offsetMinutes); | ||
| }; | ||
|
|
||
| export default timezoneData; |
There was a problem hiding this comment.
@niranjan-uma-shankar thanks for your patience, I finally have a bit of bandwidth to look at this again!
One thought: is there any reason we couldn't export the result of timezoneDataWithGMTOffset as the default?
I haven't manually tested it, but think that might mean we wouldn't need TryGhost/Ghost#24557 anymore, and if @tryghost/timezone-data is imported in any other repos anywhere the labels will still look as expected without needing to import timezoneDataWithGMTOffset instead of timezoneData.
2 participants
Fixes TryGhost/Ghost#24559. The Admin facing changes are implemented in TryGhost/Ghost#24557, and needs to be deployed along with this PR.
Changes:
timezoneDataWithGMTOffset()function dynamically prepends the GMT offset to the label, thereby ensuring that it respects DST.formatter.formatToParts().