vue-numeric icon indicating copy to clipboard operation
vue-numeric copied to clipboard

Allow empty fields

Open pareeohnos opened this issue 6 years ago • 19 comments

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.

pareeohnos avatar May 26 '18 17:05 pareeohnos

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

kevinongko avatar Jun 28 '18 14:06 kevinongko

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 Files Coverage Δ
src/vue-numeric.vue 98.21% <100%> (+0.25%) :arrow_up:

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.

codecov[bot] avatar Jun 29 '18 07:06 codecov[bot]

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 avatar Jun 29 '18 07:06 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

pareeohnos avatar Jul 05 '18 14:07 pareeohnos

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.

fourstacks avatar Jul 10 '18 12:07 fourstacks

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

pareeohnos avatar Jul 10 '18 12:07 pareeohnos

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?

fourstacks avatar Jul 10 '18 13:07 fourstacks

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

pareeohnos avatar Jul 10 '18 13:07 pareeohnos

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

serudda avatar Jul 11 '18 19:07 serudda

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

pareeohnos avatar Jul 11 '18 21:07 pareeohnos

Need this fix for multiple projects. Any ETA?

egeersoz avatar Aug 25 '18 15:08 egeersoz

@kevinongko ?

egeersoz avatar Aug 27 '18 02:08 egeersoz

@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

pareeohnos avatar Aug 27 '18 07:08 pareeohnos

@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

egeersoz avatar Sep 05 '18 19:09 egeersoz

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

pareeohnos avatar Sep 06 '18 08:09 pareeohnos

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

egeersoz avatar Sep 06 '18 19:09 egeersoz

@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

pareeohnos avatar Sep 11 '18 14:09 pareeohnos

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

tryforceful avatar Jan 02 '20 17:01 tryforceful

Have anybody new's on that question ?

Vacarov avatar Aug 24 '20 13:08 Vacarov