-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Allows for a custom display format #52
Conversation
ded16c8
to
9410154
Compare
Well dang! Haha, that's awesome, you rock @majapw. 👍 👍 |
getDateString(date) { | ||
const { displayFormat } = this.props; | ||
if (displayFormat) { | ||
return date && date.format(displayFormat); |
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.
when date
is falsy, can you remind me what toLocalizedDateString
will return?
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.
toLocalizedString
returns null
for falsey dates. I'm going to go back and add some tests for that I think.
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.
Ha! I already wrote a test - https://github.com/airbnb/react-dates/blob/master/test/utils/toLocalizedDateString_spec.js#L7:L9
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.
would it be better to do if (date && displayFormat)
so that falsy dates always go through toLocalizedDateString
? With the current approach, passing NaN
with a format will return NaN
, for example.
Hmm, I just realized that this still expects |
da34fe2
to
92f8f50
Compare
92f8f50
to
3e4f6c0
Compare
Hey @airbnb/webinfra can someone gimme a stamp on this? |
LGTM |
Fix for #26
@jcreamer898