-
-
Notifications
You must be signed in to change notification settings - Fork 11k
Fix: Timezone dropdown label now includes DST-aware GMT offsets #24557
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
base: main
Are you sure you want to change the base?
Fix: Timezone dropdown label now includes DST-aware GMT offsets #24557
Conversation
…T aware This uses the refactored timezone data function in the SDK package, that is DST aware.
WalkthroughThis change updates the handling and display of timezone data across the application. The code now imports and utilizes a function, Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Ensures that tests pass.
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/admin-x-settings/src/components/settings/general/TimeZone.tsx
(2 hunks)apps/admin-x-settings/test/acceptance/general/timeZone.test.ts
(1 hunks)ghost/admin/app/instance-initializers/config.js
(1 hunks)ghost/admin/mirage/fixtures/timezones.js
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
apps/admin-x-settings/src/components/settings/general/TimeZone.tsx
[error] 7-7: TypeScript error TS2614: Module '@tryghost/timezone-data' has no exported member 'timezoneDataWithGMTOffset'. Did you mean to use 'import timezoneDataWithGMTOffset from "@tryghost/timezone-data"' instead?
apps/admin-x-settings/test/acceptance/general/timeZone.test.ts
[error] 18-18: Timed out waiting for 'timezone-select' to be visible in timezone section.
🔇 Additional comments (6)
ghost/admin/mirage/fixtures/timezones.js (1)
1-270
: LGTM! Fixture data correctly updated for dynamic GMT offsets.The removal of GMT offset prefixes from all timezone labels is consistent with the PR objective to make GMT offsets dynamic. The fixture data now contains clean location names that will be enhanced with real-time GMT offsets by the
timezoneDataWithGMTOffset()
function.ghost/admin/app/instance-initializers/config.js (1)
9-9
: Function call syntax is correct.The updated getter correctly calls
timezoneDataWithGMTOffset()
as a function, which aligns with the expected dynamic behavior for GMT offset calculation.apps/admin-x-settings/test/acceptance/general/timeZone.test.ts (3)
20-20
: Test logic updated correctly for new label format.The test correctly selects "Alaska" instead of the previous GMT-prefixed label, aligning with the updated timezone data structure.
25-25
: Assertion updated to match new label format.The assertion correctly expects "Alaska" without the GMT prefix, consistent with the fixture data changes.
18-18
: Investigate test timeout - component may not be rendering.The pipeline failure shows a timeout waiting for the timezone select component to be visible. This suggests the component isn't rendering properly, likely due to JavaScript errors from the missing
timezoneDataWithGMTOffset
export.The test failure may be related to the missing package export. Once the TypeScript compilation issues are resolved, this test should pass.
apps/admin-x-settings/src/components/settings/general/TimeZone.tsx (1)
50-55
: Component logic correctly implemented for dynamic timezone data.The code properly calls
timezoneDataWithGMTOffset()
as a function and maps the results to the expected dropdown option format. The implementation will work correctly once the package dependency is resolved.
Description
This PR addresses #24559, where the timezone dropdown on the
/settings/timezone
page displayed static GMT offsets that did not account for Daylight Saving Time (DST).What’s Changed
timezoneDataWithGMTOffset()
function, which dynamically prepends each label with the current, DST-adjusted GMT offset.timezoneDataWithGMTOffset()
implementation is introduced in TryGhost/SDK#584.BEFORE
The dropdown displayed incorrect static offsets. For example, when GMT time was 03:55, Amsterdam (which was in GMT+2 due to DST) was shown as GMT+1:
Screen.Recording.2025-07-30.at.9.25.48.AM.mov
AFTER
The offsets now correctly reflect DST adjustments (e.g., Amsterdam is shown as GMT+2 when appropriate). Also, entries are sorted based on current offsets, placing GMT+0 entries like Azores alongside others in the same zone.
Screen.Recording.2025-07-30.at.9.42.47.AM.mov
🚨Note on Failing Tests
Unit tests are currently failing because this PR depends on changes in the
@tryghost/timezone-data
package introduced in TryGhost/SDK#584, which has not yet been published to npm.Once the SDK changes are released and the package is updated, the tests in this PR should pass without further modifications.
In the meantime, you can verify that the tests pass locally by running in the Ghost repo (after linking the modified SDK package as described in the testing instructions below).
Testing Instructions
This PR depends on TryGhost/SDK#584, which must be built and linked locally:
/settings/timezone
.