-
Notifications
You must be signed in to change notification settings - Fork 113
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
Return timestamp in Profile events list #1360
Conversation
**WHY**: Users should be able to see the time AND date for events in their account history.
config/locales/event_types/en.yml
Outdated
@@ -9,3 +9,4 @@ en: | |||
email_changed: Email address changed | |||
phone_changed: Phone number changed | |||
phone_confirmed: Phone confirmed | |||
timestamp: '%{date} at %{time} (Eastern)' |
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.
This key is very specific to eastern time, can we put that in the key name somewhere? timestamp_eastern
?
def to_s | ||
I18n.t( | ||
'event_types.timestamp', | ||
date: eastern_timestamp.strftime('%B %e, %Y'), |
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.
Maybe a good use for I18n.l
?
# some translation.yml
en:
time:
formats:
date: '%B %e, %Y'
time: '%-l:%M %p'
I18n.t(
'event_types.timestamp',
date: I18n.l(eastern_timestamp, format: :date),
time: I18n.l(eastern_timestamp, format: :time)
)
And I know on our ticket we specified full "Eastern" but I think we could get close with "%Z"
:
en:
time:
formats:
event_timestamp: "%B %e, %Y at %-l:%M %p (%Z)"
I18n.l(eastern_timestamp, format: :event_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.
COOL! love that use of I18n.l
done here: f8f0212
note that I went without the %z
since that output an integer and @andrewhughey was pretty clear in the GH issue that he wanted "Eastern" for clarity.
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.
Sorry I meant capital '%Z'
which is EST
but yeah totally understood about full "Eastern"
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.
LGTM
WHY: Users should be able to see the time AND date for events in their account history.