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

Date: Ensure timezone offset is a number #25684

Closed
wants to merge 1 commit into from

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Sep 28, 2020

When using a UTC offset timezone (instead of a named timezone), the offset is saved as a numeric string. This is not understood by moment, which expects either a number or a formatted string (ex "+08:00"). This means dates are not formatted correctly for sites with UTC timezones. You can see this by changing the settings on a site between 2 options that represent the same time, ex "UTC-7" and "Vancouver".

wp.date.dateI18n('F j, Y g:i a', 1601532000000 )
  -> Vancouver: "September 30, 2020 11:00 pm"
  -> UTC-7: "October 1, 2020 2:00 am" (this is my local time, EDT)

Alternately you can see the problem more clearly by trying to pass in a string timezone offset to date:

wp.date.date('F j, Y g:i a', 1601532000000, '-7' )
  ⚠️ Moment Timezone has no data for -7.

Fixes #21977

How has this been tested?

Tested in browser as described above (with UTC-7 and UTC+1:30), and ran unit tests.

Types of changes

Bug fix (non-breaking change which fixes an issue)

When using a UTC offset timezone (instead of a named timezone), the offset is saved as a numeric string. This is not understood by moment, which expects either a number or a formatted string (ex "+08:00").
@github-actions
Copy link

Size Change: +4 B (0%)

Total Size: 1.17 MB

Filename Size Change
build/date/index.js 31.9 kB +4 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.52 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 8.61 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 129 kB 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.6 kB 0 B
build/block-library/editor.css 8.6 kB 0 B
build/block-library/index.js 135 kB 0 B
build/block-library/style-rtl.css 7.61 kB 0 B
build/block-library/style.css 7.6 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.78 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.5 kB 0 B
build/components/index.js 168 kB 0 B
build/components/style-rtl.css 15.4 kB 0 B
build/components/style.css 15.4 kB 0 B
build/compose/index.js 9.42 kB 0 B
build/core-data/index.js 12 kB 0 B
build/data-controls/index.js 1.27 kB 0 B
build/data/index.js 8.42 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.42 kB 0 B
build/edit-navigation/index.js 10.7 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.25 kB 0 B
build/edit-post/style.css 6.24 kB 0 B
build/edit-site/index.js 20.5 kB 0 B
build/edit-site/style-rtl.css 3.54 kB 0 B
build/edit-site/style.css 3.54 kB 0 B
build/edit-widgets/index.js 17.7 kB 0 B
build/edit-widgets/style-rtl.css 2.73 kB 0 B
build/edit-widgets/style.css 2.73 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/index.js 45.4 kB 0 B
build/editor/style-rtl.css 3.83 kB 0 B
build/editor/style.css 3.82 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.49 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.55 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.34 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.5 kB 0 B
build/server-side-render/index.js 2.6 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.74 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

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

I realize this is a bit stale, but LGTM!

I've got to note though that __experimentalGetOffset does not exist—#21977 (comment) is probably referencing __experimentalGetSettings. This change still affects formatting when using other functions within @wordpress/date.

Base automatically changed from master to trunk March 1, 2021 15:44
@jeremyfelt
Copy link
Member

I've got to note though that __experimentalGetOffset does not exist—#21977 (comment) is probably referencing __experimentalGetSettings. This change still affects formatting when using other functions within @wordpress/date

Thanks for this! I'm not sure where I got __experimentalGetOffset() from. I've updated the title and description in #21977 to better reflect the naming. :)

@ciampo ciampo requested a review from noisysocks August 26, 2022 08:27
@ciampo
Copy link
Contributor

ciampo commented Aug 26, 2022

Pinging @noisysocks in the context of #21977 (comment)

@t-hamano
Copy link
Contributor

I have checked the latest trunk status.
Perhaps a similar change is included in #39670.

return dateMoment.utcOffset( +settings.timezone.offset );

@noisysocks
Can we close this PR?

@noisysocks
Copy link
Member

Yes #39670 included essentially what's in this PR.

@noisysocks noisysocks closed this Dec 19, 2022
@noisysocks noisysocks deleted the fix/date-offset-timezone branch December 19, 2022 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Date /packages/date
Projects
None yet
Development

Successfully merging this pull request may close these issues.

__getExperimentalSettings timezone offset may be a string or integer
6 participants