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

fixing custom formats by string #29

Merged
merged 1 commit into from
Apr 28, 2014
Merged

Conversation

caridy
Copy link
Collaborator

@caridy caridy commented Apr 28, 2014

This PR fixes the option to pass a second argument as string to select a custom formatter:

{{intlNumber NUM "foo"}}
{{intlDate 1390518044403 "bar"}}

It also fixes the message format for something like this:

the cost {price, number, foo} will be ready by {deadline, date, bar}

where foo is a number formatter, and bar is a data formatter.

@yahoocla
Copy link

CLA is valid!

@drewfish
Copy link
Contributor

+1

@@ -97,7 +97,7 @@ See the accompanying LICENSE file for terms.

if (formatOptions) {
if (typeof formatOptions === 'string') {
formatOptions = intlGet('formats.date.' + format, options);
formatOptions = intlGet('formats.date.' + formatOptions, options);
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this was just a copy/paste error then that was causing it to not work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

probably a renaming process, but essentially yes, jshint reported the issue when I was copy and pasting this into the react-intl. I wonder why it didn't do it in this repo, will investigate more.

@caridy caridy merged commit e388570 into formatjs:master Apr 28, 2014
caridy added a commit that referenced this pull request Apr 28, 2014
@caridy
Copy link
Collaborator Author

caridy commented Apr 29, 2014

+v0.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants