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

removed cost input restriction #287

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bjartur2004
Copy link

As discribed in issue #285 i had issues with inputting cost for very low cost items, due to reformatting clipping off anything after 2 decemalplaces.

in my tests this change is only for the better. maybe something to be aware of is if very very small numbers are inputted it will now format them into scientific notation, but this did not seem to cause any issues

@Sylensky
Copy link

I am not a maintainer of the project but was recently working on couple features for it. And from my point of view i wouldn't remove the restriction and rather find the cheapest part possible and increase the toFixed property accordingly.

Realistically an item for example below 0.0000001 $ does not make sense to track unless you have such a huge storage to house it.

@bjartur2004
Copy link
Author

Ok i can agree another solution would to have it fixed to 4 decimals but i didint really see a reason to limit the user.. like yes theres not much point in tracking tiny tiny values, but therefore i dont think people are typing in tiny values, so theres no harm in supporting them.

This formatting function is pretty whacky in more ways tho. It also seems to run at pretty whacky times, seems like every time you edit any other field than the cost or click save it formats, but i think its more senseble it would run when you open a part and when editinng only the cost field.

i could look into polishing it up more instead of just this quick fix

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.

2 participants