Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Fix input for decimal/float/double and nullable #11815

Merged
merged 5 commits into from
Jan 14, 2022

Conversation

jonkas2211
Copy link
Contributor

Description of Change

The input for floating numbers had some errors in different cultures.
When using german for example, the decimal seperator is a ',' and not a dot.

You can also use Nullable Types now. Before the Decimal Seperator would be cut off for Nullables.

Issues Resolved

API Changes

None

Platforms Affected

  • Core/XAML (all platforms)

Behavioral/Visual Changes

The user of an app can now use the correct decimal sperator for his language.

Before/After Screenshots

Not applicable

Testing Procedure

I have tested this on android.
I added a UI Test to the Gallery (G7996).
Unfortunately i cant test this on other platforms right now.
I also added a new Unit Test to prove the behaviour.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@dnfadmin
Copy link

dnfadmin commented Aug 17, 2020

CLA assistant check
All CLA requirements met.

@samhouts samhouts added the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Aug 17, 2020
@rmarinho
Copy link
Member

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@rmarinho
Copy link
Member

@jonkas2211
Copy link
Contributor Author

jonkas2211 commented Aug 18, 2020

Hello @rmarinho,

may this caused by this line CultureInfo.CurrentUICulture = culture; of the Unit Test?

EDIT: This is caused, because the Decimal Separator in this culture is a ',' and not a '.'

But the test tries to set to "0.5" instead of "0,5".

@memu8
Copy link
Contributor

memu8 commented Aug 20, 2020

In your UI tests, there's currently no way to know see the binding value or recognise if there was a binding error. You might want to add another control (maybe a label) that the displays the current value of my decimal so users can see what the entry is converting to and from in the tests.

@jonkas2211
Copy link
Contributor Author

I have made an update to the UI Test.

@samhouts samhouts changed the base branch from main to 5.0.0 August 31, 2020 23:35
@samhouts samhouts removed the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Aug 31, 2020
@samhouts samhouts added retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. and removed retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. labels Aug 31, 2020
@jonkas2211
Copy link
Contributor Author

So whats the status on this PR?

This commit fixes the input of decimal/float/double and their
nullable equivalents in different cultures.

Issue xamarin#7996
Makes the UI test more understandable. It shows now a label with the actual resolved binding value. In the entry you can now see the value you provided.
@jfversluis
Copy link
Member

jfversluis commented Jan 3, 2022

/azp run

@azure-pipelines

This comment has been minimized.

@jfversluis
Copy link
Member

Hey @jonkas2211 I'm truly sorry this was kept around for so long. Just updated this to work with our latest code and see if I can get it to work. If you're still around and interested please let me know, if not, I will take over and see to get this merged.

In either case: thank you so much for your time and effort on this. Although it doesn't show, I know I and all of use appreciate this.

@azure-pipelines

This comment has been minimized.

@azure-pipelines

This comment has been minimized.

@jonkas2211
Copy link
Contributor Author

Hello @jfversluis,
thanks for finally looking at his PR!
If there is any thing open let me know, but i think there shouldnt be anymore work here.
We need to make sure, that apps that are arround are not breaking by this change since they might expect the old behaviour.

@jfversluis
Copy link
Member

Yeah from some quick testing it seems that the code looks alright, I had to fix up some old tests to take into account this new behavior.

If you want, you could test this functionality on your own scenarios by downloading the NuGets that resulted from this build as described here. If you decide to do so, please let me know what you find, thanks!

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Xamarin.Forms.Entry does not enter decimal when binding a float/double and decimal to it
8 participants