Windowscommunitytoolkit: Prevent a dozen of VisibilityConverters

[bartlannoeye]

It might be too late to change (since there's already a v1), but I'm a bit disappointed to see 3 different VisibilityConverters already, with the chance for others to be added as well in the future.

I've always be a fan of a single converter that supports booleans, strings, collections or any type of object.

On the other hand, do we still need these converters? If my memory serves me well, it was no longer required in the anniversary update, so they can be marked deprecated?

[JohnnyWestlake]

x:Bind auto conversion only work with x:Bind (and there is still a valid place for standard Binding), and only then when the MinVersion is >=14393, so no support for 10586. (I'm also not certain it can do inverse conversions?)

Though I agree there should be one converter base class for Visibility Conversions, that also allows for inverse conversions.

[ScottIsAFool]

As @JohnnyWestlake points out, x:bind auto conversions are only in the anniversary update, and this toolkit supports older versions of Windows 10.

As for the single converter, feel free to submit a pull request for it :)

[bartlannoeye]

Do I deprecate the old ones or can they be removed (since we released)?
Will be for tonight/tomorrow evening.

[ScottIsAFool]

For the moment, do it as an additional one. @deltakosh what do you think about this?

[deltakosh]

It this related to this PR? https://github.com/Microsoft/UWPCommunityToolkit/pull/164

I'm totally ok with deprecating the converters we have in v1.

For simplicity sake, I wonder if we should not also keep simple converter (which can use the main one internally)

[bartlannoeye]

Seems like #164 is adding in more BoolToXxx converters, while I'm trying to consolidate XxxToVisibility (see code linked in OP). As an empty string or an empty collection is just another boolean check.

Maybe we have to do both, consolidate the input object and differentiate the output:

• ObjectToVisibilityConverter (as in the VisibilityConverter linked in OP)
• ObjectToObjectConverter (to Brush, to Image, .. as implemented in #164 BoolToObjectConverter)

[deltakosh]

Adding @NatMarchand to this thread

[dotMorten]

Just reminder that 14393 has added a lot of options for converting using x:Bind. While I know this wouldn't solve scenarios for apps running on 10586, longer term this would become less of an issue.
I'd prefer we focus on providing converters that the x:Bind improvements doesn't help with.

[JohnnyWestlake]

In all fairness, normal Bindings still have perfectly valid uses in places where x:Bind doesn't currently work (ElementName bindings in DataTemplates, generic control templates, RelativeSource TemplatedParent, runtime bindings, etc.), so I don't think we should push aside certain converters even if x:Bind does offer some of their functionality - Bindings are still useful!

[dotMorten]

@JohnnyWestlake Good points!

[NatMarchand]

Well, you'd need to implement the ConvertBack and I think with only one converter it's going to be a challenge.
Personally, I've never used converters such as EmptyCollectionConverter or EmptyStringConverter so I have no real strong opinion about that ^^
I only did my changes on the three to have feature parity.

[bartlannoeye]

The convert back can be a challenge indeed.
I've used empty collection / string a lot in combination with visibility (as in hide if empty). My initial point was to consolidate the initial converters. While yours provide a real added value compared to those as well. So the follow-up idea is: can we combine best of both worlds. If not, do we still need my proposal?