Conversation

pareeohnos

This resolves #53 and #57 by allowing null values to be passed in and not have it replaced with a 0 value. Currently, the emptyValue attribute is only able to handle actual values, so supplying null will not result in an empty field and instead defaults back to 0.

This changes allows null to be passed to the emptyValue property. It also allows the value property to be null, as this also prevents it defaulting to 0.

@kevinongko

Hi @pareeohnos,
Thanks for the PR 👍
Please make sure the CI checks is all green before I can merge it

@codecov

Codecov Report

Merging #71 into master will increase coverage by 0.25%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #71      +/-   ##
==========================================
+ Coverage   97.95%   98.21%   +0.25%     
==========================================
  Files           1        1              
  Lines          49       56       +7     
  Branches       20       25       +5     
==========================================
+ Hits           48       55       +7     
  Partials        1        1
Impacted FilesCoverage Δ
src/vue-numeric.vue98.21% <100%> (+0.25%)⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f639863...fda2992. Read the comment docs.

@pareeohnos

Apologies @kevinongko I had left the type of the prop out because it was always giving warnings/errors in the actual code, but I've just had another look and it seems to be fine with it now, so not sure what the issue was.

I also ran the linter locally and it threw up some errors which the travis-ci linter isn't throwing so i resolved these as well

@pareeohnos

@kevinongko just ran into the issue and the reason why I left it failing on the linter. If I leave in the type: [String, Number] attribute of the value prop, then Vue raises an error when the value is null because null is not a String or a Number. If I remove the type attribute and validate it manually, then I get around this but that is in turn causing the linter to fail.

Unsure how to proceed, as I can't see any work around without removing the type attribute

@fourstacks

Hey @pareeohnos

Thanks for submitting the PR.

Seeing as null is an Object in JS, could you just add Object to the array on line 135? e.g.

type: [String, Number, Object]

It looks as though you are doing a check to make sure that the value is specifically null a Number or a String in the validator so you shouldn't run into problems with people adding non-null objects in.

@pareeohnos

I had originally tried that approach but Vue still spits out a warning (though it's just a warning so wouldn't cause any real issues). I assume vue's type check is checking for null before the actual type check

@fourstacks

Ah right, that's a pity. Do you think it would it break BC to remove the required attribute for that prop and set default to null instead of an empty string?

@pareeohnos

Unsure really, I can't see why it would break anything. The only thing is it wouldn't give any warnings or errors if the value prop was not provided by the developer but that's not the biggest issue

@serudda

When do you have in mind to release this fix? Thanks in advance

@pareeohnos

I'm happy to make any change needed but can't personally get it merged released. Hopefully soon

@egeersoz

Need this fix for multiple projects. Any ETA?

@egeersoz

@kevinongko ?

@pareeohnos

@egeersoz I can't do anything about getting it merged unfortunately. @kevinongko any updates, would be good to be able to use this without needing to use my fork in our system

@egeersoz

@pareeohnos Your branch doesn't quite work for me. When the field has an empty value, it automatically takes 0 as a value. See here: https://s15.postimg.cc/xkyg20yi3/numeric.gif

@pareeohnos

@egeersoz have you added :empty-value="null" to the template? Otherwise it defaults to the normal behaviour

@egeersoz

@pareeohnos Yes I tried that already. It fails the typecheck when value is null because it expects a String or Number as the value.

@pareeohnos

@egeersoz hmm fair enough. I had originally removed this but it failed the linter specs and a PR won't be accepted with this, so not sure what to do here really

@tryforceful

What's the status on this? I'm still in need of this functionality

@Vacarov

Have anybody new's on that question ?

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

Successfully merging this pull request may close these issues.

Is null as a value possible?