Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Zend\I18n\View\Helper\NumberFormat param to set the number of decimals #3646

Closed
wants to merge 1 commit into from

Conversation

tommyseus
Copy link
Contributor

Added an param to the viewhelper to set the number of decimals.

@micheh
Copy link
Contributor

micheh commented Feb 1, 2013

How about adding setAttribute() and setSymbol() methods, to allow setting all attributes and symbols? Ping @cgmartin

@cgmartin
Copy link
Contributor

cgmartin commented Feb 1, 2013

Looking back at this filter I'm questioning it's overall value vs. just using NumberFormatter directly in the view with the custom params/attributes. Are we over-engineering this? (Probably too late to ask at this point)

Considering that CurrencyFormat helper's use case is for simplifying currency formatting and NumberFormat is for "other" numbers, is the decimal attribute all that we'd need to make NumberFormat flexible for ~90% of use-cases?

If it isn't just decimals, I'd agree with @micheh to make it relatively maintenance free by supporting all attributes and symbols (and/or text attributes).

/cc @DASPRiD

@Freeaqingme
Copy link
Member

I agree with @cgmarin's considerations. @weierophinney what do you think?

@ghost ghost assigned weierophinney Feb 15, 2013
@weierophinney
Copy link
Member

It isn't just decimals -- we also have to consider other separators, such as thousands, millions, etc. as those vary from locale to locale. It looks like those are already covered though. This may be enough.

@ghost ghost assigned weierophinney Feb 19, 2013
weierophinney added a commit that referenced this pull request Mar 11, 2013
@weierophinney
Copy link
Member

Merged to the develop branch for 2.2.0.

weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants