-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Core] Fixes conversion binding a DateTime to a string if the locale is not the US #3700
Changes from 2 commits
819af34
11da699
ccab2d4
97f9e34
8e8d883
1a22bb5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
using Xamarin.Forms.CustomAttributes; | ||
using Xamarin.Forms.Internals; | ||
using System; | ||
using System.Threading; | ||
using System.Globalization; | ||
|
||
#if UITEST | ||
using Xamarin.UITest; | ||
using NUnit.Framework; | ||
#endif | ||
|
||
namespace Xamarin.Forms.Controls.Issues | ||
{ | ||
[Preserve(AllMembers = true)] | ||
[Issue(IssueTracker.Github, 2049, "Bound DateTimes Display in en-US Format Not Local Format", PlatformAffected.All)] | ||
public class Issue2049 : TestContentPage | ||
{ | ||
[Preserve(AllMembers = true)] | ||
public class Model | ||
{ | ||
public DateTime TheDate { get; set; } | ||
public float FloatValue { get; set; } | ||
|
||
decimal _decimal; | ||
public decimal Decimal | ||
{ | ||
get | ||
{ | ||
return _decimal; | ||
} | ||
set | ||
{ | ||
_decimal = value; | ||
DecimalChange?.Invoke(value); | ||
} | ||
} | ||
|
||
public static Action<decimal> DecimalChange; | ||
} | ||
|
||
DateTime testDate = DateTime.ParseExact("2077-12-31T13:55:56", "yyyy-MM-ddTHH:mm:ss", CultureInfo.InvariantCulture); | ||
int _localeIndex = 0; | ||
string[] _localeIds = new[] { "en-US", "ru-RU", "en-AU", "zh-Hans" }; | ||
string _instuctions = $"When you change the locale, the date format must change.{Environment.NewLine}Current locale: "; | ||
Label descLabel = new Label(); | ||
Label decimalResult = new Label(); | ||
|
||
protected override void Init() | ||
{ | ||
UpdateContext(); | ||
|
||
var label = new Label(); | ||
label.SetBinding(Label.TextProperty, nameof(Model.TheDate)); | ||
var labelFloat = new Label(); | ||
labelFloat.SetBinding(Label.TextProperty, nameof(Model.Decimal)); | ||
var entry = new Entry | ||
{ | ||
Keyboard = Keyboard.Numeric | ||
}; | ||
entry.SetBinding(Entry.TextProperty, nameof(Model.Decimal)); | ||
Model.DecimalChange = (v) => { | ||
decimalResult.Text = v.ToString(CultureInfo.CurrentCulture); | ||
}; | ||
|
||
UpdateDescLabel(); | ||
|
||
var layout = new StackLayout | ||
{ | ||
Children = { | ||
descLabel, | ||
new Button() | ||
{ | ||
Text = "Change Locale", | ||
Command = new Command(() => ChangeLocale()) | ||
}, | ||
label, | ||
labelFloat, | ||
entry, | ||
decimalResult | ||
} | ||
}; | ||
|
||
Content = layout; | ||
} | ||
|
||
void UpdateContext() | ||
{ | ||
BindingContext = new Model | ||
{ | ||
TheDate = DateTime.ParseExact("2077-12-31T13:55:56", "yyyy-MM-ddTHH:mm:ss", CultureInfo.InvariantCulture), | ||
FloatValue = 123.456f, // in some locales, the decimal symbol may not be a dot. | ||
Decimal = 456.789m | ||
}; | ||
} | ||
|
||
void ChangeLocale() | ||
{ | ||
if (++_localeIndex > _localeIds.Length - 1) | ||
_localeIndex = 0; | ||
var locale = _localeIds[_localeIndex]; | ||
|
||
Thread.CurrentThread.CurrentCulture = new CultureInfo(locale); | ||
UpdateDescLabel(); | ||
UpdateContext(); | ||
} | ||
|
||
void UpdateDescLabel() => descLabel.Text = $"{_instuctions}{Thread.CurrentThread.CurrentCulture.DisplayName}"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I run this on Android, I don't see the current culture on the page. |
||
|
||
#if UITEST | ||
[Test] | ||
public void Issue2049Test () | ||
{ | ||
foreach (var locale in _localeIds) | ||
{ | ||
if (RunningApp.Query(query => query.Text(testDate.ToString(new CultureInfo(locale)))).Length != 1) | ||
Assert.Fail(); | ||
RunningApp.Tap("Change Locale"); | ||
} | ||
} | ||
#endif | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,7 +51,8 @@ public sealed class BindableProperty | |
{ typeof(long), new[] { typeof(string), typeof(float), typeof(double), typeof(decimal) } }, | ||
{ typeof(char), new[] { typeof(string), typeof(ushort), typeof(int), typeof(uint), typeof(long), typeof(ulong), typeof(float), typeof(double), typeof(decimal) } }, | ||
{ typeof(float), new[] { typeof(string), typeof(double) } }, | ||
{ typeof(ulong), new[] { typeof(string), typeof(float), typeof(double), typeof(decimal) } } | ||
{ typeof(ulong), new[] { typeof(string), typeof(float), typeof(double), typeof(decimal) } }, | ||
{ typeof(DateTime), new[] { typeof(string) } } | ||
}; | ||
|
||
BindableProperty(string propertyName, Type returnType, Type declaringType, object defaultValue, BindingMode defaultBindingMode = BindingMode.OneWay, | ||
|
@@ -317,7 +318,7 @@ internal bool TryConvert(ref object value) | |
TypeConverter typeConverterTo; | ||
if (SimpleConvertTypes.TryGetValue(valueType, out convertableTo) && Array.IndexOf(convertableTo, type) != -1) | ||
{ | ||
value = Convert.ChangeType(value, type); | ||
value = Convert.ChangeType(value, type, System.Globalization.CultureInfo.CurrentCulture); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With any luck the feature "Use UI Thread Culture When Preforming Conversions" would only require changes in this function. Who knows, maybe this is the only spot! However, right below, we have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Below seems to be a mistake, because the |
||
} | ||
else if (WellKnownConvertTypes.TryGetValue(type, out typeConverterTo) && typeConverterTo.CanConvertFrom(valueType)) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -423,19 +423,26 @@ bool TryConvert(BindingExpressionPart part, ref object value, Type convertTo, bo | |
return true; | ||
|
||
object original = value; | ||
var locale = CultureInfo.InvariantCulture; | ||
try | ||
{ | ||
var stringValue = value as string ?? string.Empty; | ||
// see: https://bugzilla.xamarin.com/show_bug.cgi?id=32871 | ||
// do not canonicalize "*.[.]"; "1." should not update bound BindableProperty | ||
if (stringValue.EndsWith(".") && DecimalTypes.Contains(convertTo)) | ||
throw new FormatException(); | ||
|
||
// do not canonicalize "-0"; user will likely enter a period after "-0" | ||
if (stringValue == "-0" && DecimalTypes.Contains(convertTo)) | ||
throw new FormatException(); | ||
if (DecimalTypes.Contains(convertTo)) | ||
{ | ||
// see: https://bugzilla.xamarin.com/show_bug.cgi?id=32871 | ||
// do not canonicalize "*.[.]"; "1." should not update bound BindableProperty | ||
if (stringValue.EndsWith(".")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic should probably be updated to test if the string value ends with the |
||
throw new FormatException(); | ||
|
||
// do not canonicalize "-0"; user will likely enter a period after "-0" | ||
if (stringValue == "-0") | ||
throw new FormatException(); | ||
|
||
locale = CultureInfo.CurrentCulture; | ||
} | ||
|
||
value = Convert.ChangeType(value, convertTo, CultureInfo.InvariantCulture); | ||
value = Convert.ChangeType(value, convertTo, locale); | ||
return true; | ||
} | ||
catch (InvalidCastException) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_instuctions >> _instructions