Added new Converters. #462

TommasoScalici · 2016-10-08T14:26:51Z

Added BoolNegationConverter, DateTimeToStringConverter and ResourceNameToResourceStringConverter.

I use very often these converters during my UWA development, so I though that may be useful to have them in the community toolkit.
Names and XML summaries should be enough self-explainatory on what they do.

Let me know what do you think.

…meToResourceStringConverter.

dnfclas · 2016-10-08T14:26:55Z

Hi @TommasoScalici, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

ScottIsAFool

Not sure I agree with these converters being sealed either.

ScottIsAFool · 2016-10-08T16:26:37Z

Microsoft.Toolkit.Uwp.UI/Converters/DateTimeToStringConverter.cs

@@ -0,0 +1,62 @@
+// ******************************************************************


I'm not sure I see the point of this converter. Why not just do the formatting in the viewmodel, that's what it's for.

Well, actually I usually use this to comply the absence of the StringFormat Markup Extension. Usually in WPF you format data in XAML binding in that way, and honestly, I always thought that formatting data is something more related to the View than the ViewModel, since it deals with how you present data and not how you stored it.
By the way this can be more useful if generalized to be used with any data type.
But I can also remove it if anyone doesn't feel the need of a StringFormat substitute.
I agree with you on classes not to be sealed, so I removed the sealed modifier.

The good thing about binding is that view just consumes what viewmodel exposes. yes often it is painful.. we have come a long way from WPF (in ways).. XAML is somewhat different plus default patterns is MVVM which tends to take a lot of load off the Views.

Since data is being bound to controls, i'd imagine most work is being done on UI thread. You dont really need StringFormat to run on UI thread.

I find converters like this super useful. It seems silly to me to put all of these formatted strings into my ViewModel code. The ViewModel holds the absolute data, and then the View decides "Oh hey, this number will only have 2 decimal points of precision" or "this date will just say Mon rather than Monday"

My apps have always had converters like this. So I'm throwing my vote in for including this converter! But if this is actually a bad pattern and those conversions are actually supposed to happen in the ViewModel (despite how unintuitive that seems to me), maybe I'm wrong.

But the ViewModel is supposed to be where what controls how the data is presented to the view.

I agree with Scott but I do not want the toolkit to be MVVM only or whatever. So for people not using MVVM or not wanting to do conversion in the ViewModel I'm also voting to add these converters to the toolkit

But you will need to update the doc to get the final validation :)

Thanks for all your opinions, I updated the docs, tell me if there's something else I'm missing.
Also, after this discussion and what @Anbare said, I thought it would have more sense to have a converter that can format any kind of type, not only DateTime/DateTimeOffset. So I came up with this: FormatStringConverter.

ScottIsAFool · 2016-10-08T16:27:06Z

Microsoft.Toolkit.Uwp.UI/Converters/ResourceNameToResourceStringConverter.cs

+using Windows.ApplicationModel.Resources;
+using Windows.UI.Xaml.Data;
+
+namespace TommasoScalici.UWPConvertersAndUtilities


Namespace needs correcting

Namespace corrected.

ScottIsAFool · 2016-10-08T16:27:48Z

Microsoft.Toolkit.Uwp.UI/Converters/ResourceNameToResourceStringConverter.cs

+    /// </summary>
+    public sealed class ResourceNameToResourceStringConverter : IValueConverter
+    {
+        private readonly ResourceLoader resourceLoader = ResourceLoader.GetForViewIndependentUse();


private fields should begin with a _

Added underscore.

ScottIsAFool · 2016-10-08T16:28:31Z

Microsoft.Toolkit.Uwp.UI/Converters/ResourceNameToResourceStringConverter.cs

+        /// <returns>The string corresponding to the resource name.</returns>
+        public object Convert(object value, Type targetType, object parameter, string language)
+        {
+            return resourceLoader.GetString(value?.ToString());


Is GetString safe to pass null into it?

No, it's not safe at all since the method will throw an exception if null is passed. I'll add a null check.

dnfclas · 2016-10-09T09:05:19Z

@TommasoScalici, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

Updated docs.

hermitdave · 2016-10-11T16:57:50Z

Like the FormatStringConverter..

andrewleader · 2016-10-11T16:53:30Z

Microsoft.Toolkit.Uwp.UI/Converters/FormatStringConverter.cs

+                return value;
+            }
+
+            return formattableValue.ToString(parameter as string, null);


If parameter is null or not a string, will this explode? If so, we should probably either return the value if parameter isn't a string, or have it throw an ArgumentException... I'd vote for returning the value (same as if formattableValue == null).

if (formattableValue == null || !(parameter is string))

You're right, didn't think of this possibility. I'd also vote to return the value.

hermitdave · 2016-10-11T17:11:14Z

Besides formattableValue, create
var format = parameter as string.

If format is null, return original value

Cast using as save check for null is somewhat better than
is check followed by as cast

TommasoScalici · 2016-10-12T10:40:24Z

Cast using as save check for null is somewhat better than
is check followed by as cast

I agree, this is usually the way I do.

JulienTheron · 2017-02-14T21:15:36Z

Hi, I just noticed that the new converters are nice and all but they're not built :).
Will they be added in the .csproj file?

hermitdave · 2017-02-14T21:28:36Z

they are not linked.. they are excluded but in folder

hermitdave · 2017-02-14T21:29:25Z

deltakosh · 2017-02-14T21:30:29Z

Woot..This is a big miss
I don't know why we removed it...Must be a mistake

deltakosh · 2017-02-14T21:30:38Z

We need to do a PR to merge them back

hermitdave · 2017-02-14T21:34:36Z

okay on it

Missing converters from PR #462

hermitdave · 2017-02-14T21:41:48Z

good spot @JulienTheron these are now include.. you could get a myget build soon with these inside

JulienTheron · 2017-02-14T21:44:21Z

Nice! Thanks.

Added BoolNegationConverter, DateTimeToStringConverter and ResourceNa…

9b209a2

…meToResourceStringConverter.

dnfclas added the cla-required label Oct 8, 2016

ScottIsAFool suggested changes Oct 8, 2016

View reviewed changes

dnfclas added cla-signed and removed cla-required labels Oct 9, 2016

Tommaso Scalici added 2 commits October 9, 2016 13:13

Added null-check to ResourceNameToResourceStringConverter.

0225804

DateTimeToStringConverter is now FormatStringConverter.

dc10762

Updated docs.

andrewleader suggested changes Oct 11, 2016

View reviewed changes

deltakosh added the reviewers wanted label Oct 11, 2016

Added cast to string for parameter and null-check.

86478ed

ScottIsAFool approved these changes Oct 12, 2016

View reviewed changes

andrewleader approved these changes Oct 12, 2016

View reviewed changes

hermitdave merged commit ae9656b into CommunityToolkit:dev Oct 12, 2016

hermitdave added a commit that referenced this pull request Feb 14, 2017

Missing converters from PR #462

0472b37

hermitdave mentioned this pull request Feb 14, 2017

Missing converters from PR #462 #937

Merged

hermitdave added a commit that referenced this pull request Feb 14, 2017

Merge pull request #937 from Microsoft/Missing-Converters

8d2e9ca

Missing converters from PR #462

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added new Converters. #462

Added new Converters. #462

TommasoScalici commented Oct 8, 2016 •

edited

Loading

dnfclas commented Oct 8, 2016

ScottIsAFool left a comment

ScottIsAFool Oct 8, 2016

TommasoScalici Oct 9, 2016 •

edited

Loading

hermitdave Oct 10, 2016

andrewleader Oct 10, 2016 •

edited

Loading

ScottIsAFool Oct 10, 2016

deltakosh Oct 10, 2016

TommasoScalici Oct 11, 2016

ScottIsAFool Oct 8, 2016

TommasoScalici Oct 9, 2016

ScottIsAFool Oct 8, 2016

TommasoScalici Oct 9, 2016

ScottIsAFool Oct 8, 2016

TommasoScalici Oct 9, 2016

dnfclas commented Oct 9, 2016

hermitdave commented Oct 11, 2016

andrewleader Oct 11, 2016

TommasoScalici Oct 12, 2016

hermitdave commented Oct 11, 2016 •

edited

Loading

TommasoScalici commented Oct 12, 2016

JulienTheron commented Feb 14, 2017

hermitdave commented Feb 14, 2017

hermitdave commented Feb 14, 2017

deltakosh commented Feb 14, 2017

deltakosh commented Feb 14, 2017

hermitdave commented Feb 14, 2017

hermitdave commented Feb 14, 2017

JulienTheron commented Feb 14, 2017

		@@ -0,0 +1,62 @@
		// ******************************************************************

Added new Converters. #462

Added new Converters. #462

Conversation

TommasoScalici commented Oct 8, 2016 • edited Loading

dnfclas commented Oct 8, 2016

ScottIsAFool left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TommasoScalici Oct 9, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewleader Oct 10, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnfclas commented Oct 9, 2016

hermitdave commented Oct 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hermitdave commented Oct 11, 2016 • edited Loading

TommasoScalici commented Oct 12, 2016

JulienTheron commented Feb 14, 2017

hermitdave commented Feb 14, 2017

hermitdave commented Feb 14, 2017

deltakosh commented Feb 14, 2017

deltakosh commented Feb 14, 2017

hermitdave commented Feb 14, 2017

hermitdave commented Feb 14, 2017

JulienTheron commented Feb 14, 2017

TommasoScalici commented Oct 8, 2016 •

edited

Loading

TommasoScalici Oct 9, 2016 •

edited

Loading

andrewleader Oct 10, 2016 •

edited

Loading

hermitdave commented Oct 11, 2016 •

edited

Loading