WindowsCommunityToolkit
WindowsCommunityToolkit copied to clipboard
Added CultureInfo to StringFormatConverter
Fixes #3243
Extended StringFormatConverter functionality to make it culture aware.
New property (CultureInfo) was added to the converter. Added support for language argument in Convert method.
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:
- [x] Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
- [x] Based off latest main branch of toolkit
- [x] Tested code with current supported SDKs
- [x] Contains NO breaking changes
Other information
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 🙌
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
:x: tutkus sign now
You have signed the CLA already but the status is still pending? Let us recheck it.
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. IValueConverter
is supplying a language
parameter, and we're ignoring it.
https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/fdaef4750236713bd788f4c1d6162a4ea5959242/Microsoft.Toolkit.Uwp.UI/Converters/StringFormatConverter.cs#L23
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 IValueConverter.Convert
, they supply a great sample that could be used as a generic StringFormatConverter
, and it's much cleaner.
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 FrameworkElement.Language
.
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 language
parameter is provided by the ConverterLanguage
binding property.
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.