-
Notifications
You must be signed in to change notification settings - Fork 10
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
Added helpers for formatting timezones #96
Conversation
lib/dateTime.js
Outdated
@@ -1128,10 +1128,24 @@ export function formatDateTime(date, options) { | |||
|
|||
} | |||
|
|||
export function formatDateTimeFromTimestamp(timestamp, options) { | |||
function parseLocalDateTimeFromTimestamp(timestamp, options) { |
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.
Methods which return JS Date
objects seem to be named parse<dateFormat>
so I tried to keep that consistent here.
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.
Looks good! Do you have merging permissions? If so, when you merge if you append feat:
to the merge commit message then it will update the version (https://github.com/BrightspaceUI/intl#versioning--releasing)
🎉 This PR is included in version 3.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
I would like to propose we add the following methods to the date time helpers:
formatDateFromTimestamp
formatTimeFromTimestamp
This seems consistent with the other formatting helpers which provide options for formatting the date or time individually and allows more flexibility for consumers using timestamps.
Another thought I had was to expose the
parseLocalDateTimeFromTimestamp
method and allow consumers to format the resultingDate
object however they want. I'm not sure if that would be a preferable option in this case.