CommunityToolkit/Maui.Markup

[Proposal] Add Overloaded Constructor to `Style<T>` that Accepts `Microsoft.Maui.Controls.Style`

Closed this issue · 4 comments

Feature name

Add Overloaded Constructor to Style<T> that Accepts Microsoft.Maui.Controls.Style

Link to discussion

dotnet/maui#19725 (comment)

Progress tracker

  • Android Implementation
  • iOS Implementation
  • MacCatalyst Implementation
  • Windows Implementation
  • Tizen Implementation
  • Unit Tests
  • Samples
  • Documentation

Summary

This Proposal adds another constructor to Style<T> that accepts. Microsoft.Maui.Controls.Style.

Motivation

The community has noted that it would be beneficial to easily convert Microsoft.Maui.Controls.Style to CommunityToolkit.Maui.Markup.Style<T>.

Detailed Design

// Additional Implicit Operator 
public static implicit operator CommunityToolkit.Maui.Markup.Style<T>(Microsoft.Maui.Controls.Style style) => new Style<T>(style);

// Additional Overloaded Constructor
public Style(Style mauiStyle)
{
  if (!mauiStyle.TargetType.IsAssignableTo(typeof(T)))
  {
	throw new ArgumentException($"Invalid type. The Type used in {nameof(mauiStyle)}.{nameof(mauiStyle.TargetType)} ({mauiStyle.TargetType.FullName}) must be assignable to the Type used in {nameof(Style<T>)} ({typeof(T).FullName})", nameof(mauiStyle));  
  }

  MauiStyle = mauiStyle;
}

Usage Syntax

Style mauiButtonStyle = new Style(typeof(Button));
Style<Button> markupButtonStyle_Constructor = new Style<Button>(mauiButtonStyle);
Style<Button> markupButtonStyle_Implicit = temp;

Drawbacks

I cannot think of any drawbacks at this time.

Alternatives

The current alternative to converting a Microsoft.Maui.Controls.Style to Style<T> is complicated and requires a for loop to move all of the BindableProperties from Style to Style<T>.

Unresolved Questions

Is there a valid use case where the type for MauiStyle.TargetType wouldn't match the type used for Style?

If not, I think it's best to throw an exception when !mauiStyle.TargetType.IsAssignableTo(typeof(T)).

E.g. The following code throws an InvalidArgumentException:

Style mauiLabelStyle = new Style(typeof(Label));

// Throws InvalidArgumentException
Style<Button> = mauiLabelStyle;

// Throws InvalidArgumentException
var buttonStyle = new Style<Button>(mauiLabelStyle);

@brminnick This would allow us to interop with generic and non-generic Styles and any enhancements made to the generic version would benefit the existing codebase as well.

Thanks @egvijayanand! What are your thoughts on validating the Type and throwing an ArgumentException to ensure they match?

I personally can't think of a valid use-case for something like this Style<Button> buttonStyle = new Style(typeof(Label));, which is why I designed the API to throw an ArgumentException.

if (!mauiStyle.TargetType.IsAssignableTo(typeof(T)))
{
	throw new ArgumentException($"Invalid type. The Type used in {nameof(mauiStyle)}.{nameof(mauiStyle.TargetType)} ({mauiStyle.TargetType.FullName}) must be assignable to the Type used in {nameof(Style<T>)} ({typeof(T).FullName})", nameof(mauiStyle));  
}

What are your thoughts on validating the Type and throwing an ArgumentException to ensure they match?

Thought of adding this when commenting on the original issue (inspired by the Assign() method). But as a practice, I prefer not to throw errors from the constructor but rather document its behavior on the way it works. Then only added the else part.

By doing this Style defined for base types, like View, can be reused in derived types as well. Makes life easy.

Thanks @egvijayanand! What are your thoughts on validating the Type and throwing an ArgumentException to ensure they match?

I personally can't think of a valid use-case for something like this Style<Button> buttonStyle = new Style(typeof(Label));, which is why I designed the API to throw an ArgumentException.

if (!mauiStyle.TargetType.IsAssignableTo(typeof(T)))
{
	throw new ArgumentException($"Invalid type. The Type used in {nameof(mauiStyle)}.{nameof(mauiStyle.TargetType)} ({mauiStyle.TargetType.FullName}) must be assignable to the Type used in {nameof(Style<T>)} ({typeof(T).FullName})", nameof(mauiStyle));  
}

It would be nice to try it... I remember, during my mobile dev days, that you can use Label style for Span and they're different types... Maybe other controls can behave like that