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

Allow string property binding as Color source #9814

Merged
merged 4 commits into from
Mar 2, 2023
Merged

Allow string property binding as Color source #9814

merged 4 commits into from
Mar 2, 2023

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Aug 31, 2022

Description of Change

Allow string property binding as Color source.

image

Issues Fixed

Fixes #9662
Fixes #8390
Fixes #7683

@jsuarezruiz jsuarezruiz added t/bug Something isn't working area-xaml XAML, CSS, Triggers, Behaviors labels Aug 31, 2022
@@ -41,6 +41,7 @@ public sealed class BindableProperty
static readonly Dictionary<Type, TypeConverter> KnownTypeConverters = new Dictionary<Type, TypeConverter>
{
{ typeof(Uri), new UriTypeConverter() },
{ typeof(Maui.Graphics.Color), new ColorTypeConverter() },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That list is only for types we can’t attribute. Don’t we have a converter attribute on colour?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked about this with Stephane and is ok to merge, Shane could you review it again?

@rmarinho
Copy link
Member

can we add a xaml test?

@jsuarezruiz jsuarezruiz marked this pull request as draft November 3, 2022 08:55
@jsuarezruiz jsuarezruiz changed the base branch from net6.0 to main November 3, 2022 10:58
@jsuarezruiz jsuarezruiz marked this pull request as ready for review November 22, 2022 12:30
@jsuarezruiz jsuarezruiz self-assigned this Nov 23, 2022
hartez
hartez previously approved these changes Nov 23, 2022
@StephaneDelcroix
Copy link
Contributor

Doesn’t look like the correct way to fix this

@hartez hartez self-requested a review November 23, 2022 21:38
@hartez hartez dismissed their stale review November 23, 2022 21:38

Stephane knows more about this than I do :)

@jsuarezruiz
Copy link
Contributor Author

Doesn’t look like the correct way to fix this

Could you share more details?. Thanks!

@StephaneDelcroix
Copy link
Contributor

Doesn’t look like the correct way to fix this

Could you share more details?. Thanks!

#9814 (comment)

Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hartez
Copy link
Contributor

hartez commented Jan 11, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@PureWeen PureWeen removed their request for review January 25, 2023 17:06
@jsuarezruiz jsuarezruiz requested review from PureWeen and StephaneDelcroix and removed request for PureWeen, hartez and StephaneDelcroix February 23, 2023 13:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-xaml XAML, CSS, Triggers, Behaviors t/bug Something isn't working
Projects
None yet
6 participants