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

Add renderer for type :decimal and style :USD for form fields. #5

Merged
merged 2 commits into from Jun 24, 2020

Conversation

murtaza52
Copy link
Contributor

@murtaza52 murtaza52 commented Jun 23, 2020

Hi,

There are two commits, one is for implementing the currency rendering when the type is :decimal and :style :USD in form fields.

The second commit is a bug fix when the type is :decimal and :style :default, to allow default values to be displayed in the form.

Thanks,
Murtaza

The default values do not pass the test of math/numeric?, and thus are
not displayed. Assumption is that this check is not needed, as the values are
coming from DB, where the type constraint is applied.
@awkay
Copy link
Member

awkay commented Jun 24, 2020

Remind me: where was the USD thing coming up? The demo? Technically, currency formatting is a "locale" thing, so calling numeric->currency-string, which is probably going to be ported to use Locales at some point, is a good thing, but USD is then not the right name. Perhaps :currency as the style. If the demo uses USD, then it should probably just add a custom style for that, OR we could consider that numeric->currency-string might take an additional argument to specify which currency.

@murtaza52
Copy link
Contributor Author

murtaza52 commented Jun 24, 2020 via email

@awkay
Copy link
Member

awkay commented Jun 24, 2020

OK, well, this is probably ok then...it is an implementation detail I can deal with later.

@awkay awkay merged commit e6c4de3 into fulcrologic:develop Jun 24, 2020
@awkay
Copy link
Member

awkay commented Jun 24, 2020

released in 0.0.11-alpha

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.

None yet

2 participants