Skip to content
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

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

Open
8 tasks
brminnick opened this issue Jan 8, 2024 · 4 comments
Labels
approved champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature help wanted This proposal has been approved and is ready to be implemented needs discussion The team will aim to discuss this at the next monthly standup proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail

Comments

@brminnick
Copy link
Collaborator

brminnick commented Jan 8, 2024

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 brminnick added proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail new labels Jan 8, 2024
@ghost ghost added this to Proposal Submitted in New Feature Proposals Jan 8, 2024
@egvijayanand
Copy link

@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.

@brminnick
Copy link
Collaborator Author

brminnick commented Jan 8, 2024

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));  
}

@brminnick brminnick moved this from Proposal Submitted to Proposal Championed in New Feature Proposals Jan 8, 2024
@brminnick brminnick added the needs discussion The team will aim to discuss this at the next monthly standup label Jan 8, 2024
@ghost ghost added champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature and removed new labels Jan 8, 2024
@egvijayanand
Copy link

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.

@pictos
Copy link
Member

pictos commented Jan 8, 2024

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

@brminnick brminnick moved this from Proposal Championed to Proposal Approved in New Feature Proposals Feb 1, 2024
@ghost ghost added approved help wanted This proposal has been approved and is ready to be implemented labels Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature help wanted This proposal has been approved and is ready to be implemented needs discussion The team will aim to discuss this at the next monthly standup proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail
Projects
New Feature Proposals
Proposal Approved
Development

No branches or pull requests

3 participants