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

Some more confusion about decimal separators #733

Closed
Jazzbeerman opened this issue Mar 12, 2023 · 5 comments · Fixed by #745
Closed

Some more confusion about decimal separators #733

Jazzbeerman opened this issue Mar 12, 2023 · 5 comments · Fixed by #745

Comments

@Jazzbeerman
Copy link

Some more confusion about decimal separators

Weekend, time to brew beer and do some new program tests! (I think I flooded you with messages, I'm sorry)
This time the problem of decimal separators arose in the priming calculator. If you enter the desired volumes with a separator in the form of a comma, then the calculator does not work correctly, giving negative values. If through a dot, then everything works correctly. Not a major problem, but a bit confusing as the rest of the numbers are entered separated by commas.
With commas.
1
With dots
2

It has also been noticed. that in the style editor the ABV range must also be written with a dot, otherwise it will not be saved. Again - if you know about it - it does not cause inconvenience, but of course it is confusing, since the rest of the fields work with commas. Here's what it looks like for me:
3

Just in case, system data: win10, 21H1, program version 3.0.5, 3.0.6, 3.0.7 versions appear the same.
It seems to be from the same area as #719

@matty0ung
Copy link
Contributor

Thanks for all the feedback -- much appreciated, and do keep it coming.

Will definitely take a look at these. I am hoping they are simple to fix!

@matty0ung
Copy link
Contributor

FYI I haven't forgotten about this! I have a fix that works on my machine, and that tidies up the code a bit, but I'm still thinking about whether to refactor the way we deal with input/edit fields. (We are about to add a lot of new ones for BeerJSON, so it's a good time to make a change.)

matty0ung pushed a commit to matty0ung/brewken that referenced this issue Mar 19, 2023
@Jazzbeerman
Copy link
Author

This sounds great!

@matty0ung
Copy link
Contributor

Making progress on the input/edit fields. The two main cases (one label and one edit field; one label for two edit fields) are handled, but I need to do some more work for the third case (label with no edit field because the "field" is a graphical "slider" bar). This should ultimately simplify the code, including allowing us to remove special handling for units of measurement for colors (because it is now handled generically).

The main complexity is that we allow you to change the units and scale on a field-by-field basis (as well as globally), so we need to make sure we are consistent in how we "remember" the settings for each field. Current system is a bit fragile/complex for edge cases (no edit fields and two edit fields). I think a small-ish change can make it more robust. Just have to make the small-ish change in a lot of places, but fortunately the compiler can find them all for us! 😄

@matty0ung
Copy link
Contributor

OK, sorry that took a bit longer than expected. Found lots of interesting different cases for display fields and labels. Have done a bit of rework which should make things a lot easier for all the new units. Anyway, the problems you raised here should be fixed in https://github.com/Brewtarget/brewtarget/releases/tag/v3.0.8.

If you find other similar problems with decimals etc, they should be a lot quicker to 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 a pull request may close this issue.

2 participants