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

Allow no decimals #165

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Allow no decimals #165

wants to merge 11 commits into from

Conversation

darylteo
Copy link

In reference to #82.

This feature turned up in one of my client work today, so I figured I'd try to implement it myself (spare time, unsponsored :( )

This patch introduces the allowNoDecimal property (false by default).

By turning it on the follow semantics apply:

  • values are initially treated as integer
  • any numbers added are automatically thousand'd.
  • the first time a decimal is placed, the number is now treated as a float
  • further attempts to insert a decimal is ignored
  • you may add up to precision number of digits after the decimal
  • if adding a decimal indicator to, or masking an, existing value causes an over-precision, the rest are discarded.
  • if adding a digit to the left of the decimal indicator, it is treated as part of the integral portion of the value, and is added properly (UNTESTED, see below).

Both the precision and the decimal properties influences this behaviour (so if decimal is changed to ",", the "," key must be used to start decimals).

I hope these semantics make sense, as I will be using these in my client project.

I have also updated the Readme, the demo html, as well as introduced 4 new tests for testing this behaviour. Unfortunately I could not get "." and cursor keypresses to work in the test :( if you let me know how to get them to work, I'll add more tests.

Kind Regards,
Daryl

@darylteo
Copy link
Author

btw amazed that this feature is 2 years late =) thanks for the plugin.

@darylteo
Copy link
Author

Ah... unmasked fails, because it depends on specific number of segments in the string. ( I think you mentioned this in another pull request I spotted). Troublesome. Without knowing which decimal indicator is being used in the unmasked method, this becomes difficult. Will mull.

Edit: ahh... unmasked doesn't even require the field to be initialised, so even if I did depend on data attributes, it may not even be there... can we make this a, you know, requirement? What use case would we have to call maskMoney on a field that isn't initialised? (and I ask this because all the tests in unmasked_test.js do not initialise maskMoney)

Fix unmasked

Fix doco

Fix jshint

Oops, return number not string.

Tweak the test by initialising accordingly.
@gbass84
Copy link

gbass84 commented Mar 11, 2015

There's a couple options here, but I think the easiest to merge and
maintain is to explicitly write the config properties to the masked
element when masking. This can't be done with .data(), because that data
is stored internally in jQuery, and the element loses it's reference to
it. I think the best thing to do is use .attribute() or the vanilla
equivalent.

On 3/11/15 5:09 AM, Daryl Teo wrote:

Ah... unmasked fails, because it depends on specific number of
segments in the string. ( I think you mentioned this in another pull
request I spotted). Troublesome. Without knowing which decimal
indicator is being used in the unmasked method, this becomes
difficult. Will mull.


Reply to this email directly or view it on GitHub
#165 (comment).

@darylteo
Copy link
Author

I did not know that! You leant something new everyday.

Yes I agree if jQuery data doesn't put the attribute on the element the
setup is a little strenuous... I'll modify it when I have the time today.

This doesn't solve the question of whether unmasked method can be called on
non-initialized input fields, for the time being. But maybe this discussion
can lead us there.

On Thursday, March 12, 2015, Graham Bass [email protected] wrote:

There's a couple options here, but I think the easiest to merge and
maintain is to explicitly write the config properties to the masked
element when masking. This can't be done with .data(), because that data
is stored internally in jQuery, and the element loses it's reference to
it. I think the best thing to do is use .attribute() or the vanilla
equivalent.

On 3/11/15 5:09 AM, Daryl Teo wrote:

Ah... unmasked fails, because it depends on specific number of
segments in the string. ( I think you mentioned this in another pull
request I spotted). Troublesome. Without knowing which decimal
indicator is being used in the unmasked method, this becomes
difficult. Will mull.


Reply to this email directly or view it on GitHub
<
#165 (comment)
.


Reply to this email directly or view it on GitHub
#165 (comment)
.

@plentz plentz added the feature label Mar 15, 2015
@pojda
Copy link

pojda commented Jul 5, 2016

Anything new on this?

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

Successfully merging this pull request may close these issues.

4 participants