-
Couldn't load subscription status.
- Fork 1.4k
Added CultureInfo to StringFormatConverter #4546
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
Conversation
|
Thanks tutkus for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌 |
|
|
|
Thanks for the contribution @tutkus, sorry for the delay. We're planning a hotfix release, so we'll validate the PR, and assuming no breaking API changes (as you call out), we'll be good to go to get it in. 🎉 |
|
Reviewing the unmodified code, I noticed something unusual here. WindowsCommunityToolkit/Microsoft.Toolkit.Uwp.UI/Converters/StringFormatConverter.cs Line 23 in fdaef47
You've made use of that, which is good -- but there's a lot of scaffolding that isn't needed as well. Digging into the docs for public class DateFormatter : IValueConverter
{
// This converts the DateTime object to the string to display.
public object Convert(object value, Type targetType,
object parameter, string language)
{
// Retrieve the format string and use it to format the value.
string formatString = parameter as string;
if (!string.IsNullOrEmpty(formatString))
{
return string.Format(
new CultureInfo(language), formatString, value);
}
// If the format string is null or empty, simply call ToString()
// on the value.
return value.ToString();
}
// No need to implement converting back on a one-way binding
public object ConvertBack(object value, Type targetType,
object parameter, string language)
{
throw new NotImplementedException();
}
}More importantly, the framework may be capable of handling this for us via To avoid changing our public API for this component, I'll investigate in a separate PR if we can solve your issue using the framework instead. |
|
Looks like the Example usage: <TextBlock Text="{x:Bind MyProperty, Converter={StaticResource MyConverter}, ConverterLanguage='en-US'}" />
<TextBlock Text="{Binding MyProperty, Converter={StaticResource MyConverter}, ConverterLanguage='en-US'}" /> |
|
Thanks @tutkus for the PR here, @Arlodotexe carried this forward and we merged your work and his in #4764, so closing this out now as resolved. |
Fixes #3243
Extended StringFormatConverter functionality to make it culture aware.
PR Type
What kind of change does this PR introduce?
Bugfix
Refactoring (no functional changes, no api changes)
What is the current behavior?
Currently it is not possible to use StringFormatConverter and eg. format number to represent its value as amount in specyfic currency.
What is the new behavior?
With the change converter is aware about a CultureInfo and can use it to format the text according to given setup. By default converter defaults the CultureInfo propety to CultureInfo.CurrentCulture which guarantees backward compatibility. Once the Convert is executed first it checkes if the language is passed to the method. If it's there (not null or empty), converters selects related CultureInfo otherwise coverter will take the CultureInfo already set in the propety. If that propety would be null then converter will apply InvariantCulture however this might happen only when the Converter.CultureInfo would be explicitly defaulted to null. Finally converter passes selected CultureInfo into the striong.Format to run finnal formatting.
PR Checklist
Please check if your PR fulfills the following requirements:
Other information