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

Decimal separator fix for Edit Alert #3819

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

Conversation

Navid200
Copy link
Collaborator

@Navid200 Navid200 commented Dec 17, 2024

This is an alternative approach with respect to: #2998

I have used tolerantParseDouble().
In the following table you can see the behaviors before and after.
Please pay attention at the very end to the fractional part, which is dropped before but, is preserved after.

Before After
https://github.com/user-attachments/assets/db8b902d-00de-4221-b43a-c728c86db490 https://github.com/user-attachments/assets/a7c622e9-d6be-48e0-b217-77efedba744a

@Navid200
Copy link
Collaborator Author

I tested using the alert that I created above with its threshold set to 4.5. It works as expected.

Screenshot_20241217-185407

Screenshot_20241217-185344

@Navid200 Navid200 changed the title Decimal separator - Edit Alert Decimal separator fix for Edit Alert Dec 18, 2024
@Navid200
Copy link
Collaborator Author

Navid200 commented Feb 8, 2025

I'm not a translation enthusiast with respect to xDrip because I worry that critical matters could get lost in translation.
But, this is an open issue with respect to translations with a fix proposal that was not approved.
I have followed your direction in the review of that PR to come up with this one.
Would you please tell me what you don't like about my approach in this PR?

@Navid200 Navid200 requested a review from jamorham February 18, 2025 13:26
@lz2jvc
Copy link

lz2jvc commented Feb 21, 2025

If this fix is working the same way when in the Bulgarian language, this will be a huge pain relief for many Bulgarian people. Almost all of the Bulgarian people are using the application on Bulgarian.

@Navid200
Copy link
Collaborator Author

@lz2jvc If you like, you can test this using the following. But, please note that it is only for testing and should not be shared. It is also not compatible with the official release channels. So, you will not be able to install over an official install.

The best course of action is to test on a test phone as opposed to on your main device.
Please don't disrupt an active device for testing.

https://drive.google.com/file/d/1aolFhU0Tinuk_ZePCyE0ZVEUxZu9hOjd/view?usp=sharing

@lz2jvc
Copy link

lz2jvc commented Feb 24, 2025

@lz2jvc If you like, you can test this using the following. But, please note that it is only for testing and should not be shared. It is also not compatible with the official release channels. So, you will not be able to install over an official install.

The best course of action is to test on a test phone as opposed to on your main device. Please don't disrupt an active device for testing.

https://drive.google.com/file/d/1aolFhU0Tinuk_ZePCyE0ZVEUxZu9hOjd/view?usp=sharing

I'm currently trying your test implemented version of xDrip+ on a tracking phone. To be honest, it behaves perfectly. There are no bugs with the introduction of (.) and (,) in Cyrillic in the entry of alarm profiles. For the moment, sincere thanks from me for this work. I hope sooner or later it will come out as an official release to stop this pain when entering data in Cyrillic

@Navid200
Copy link
Collaborator Author

@lz2jvc Thanks a lot for testing.

@Navid200
Copy link
Collaborator Author

Navid200 commented Mar 8, 2025

@jamorham Would you please review this?

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