Skip to content
This repository has been archived by the owner on Apr 2, 2022. It is now read-only.

Add support for Label Styles V1 like #13

Merged
merged 2 commits into from
Nov 27, 2021
Merged

Add support for Label Styles V1 like #13

merged 2 commits into from
Nov 27, 2021

Conversation

edgarfgp
Copy link
Contributor

This PR Adds initial support for #10 . This add a v1 like way of defining styles.

The original idea was to have a dedicated widgets like LabelStyle(), but for this there are some changes required on how Fabulous handles that.

I suggest we support both :

style(myStyle) This PR
.style( LabelStyle() .textColor )

Happy to try and and achieve the second one with some assistance 😀

Copy link
Owner

@TimLariviere TimLariviere left a comment

Choose a reason for hiding this comment

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

Nice work!
Regarding support for both XF.Style and style widgets, I want to keep Fabulous the easiest to use possible. We should avoid adding too many ways of doing the same thing.

So I think we should aim to support style widgets by default.
This extension will be useful for the compatibility package to migrate from v1 to v2. This will let people slowly move to v2.

So for now, I'll accept your PR but it'll be removed after we implement style widgets.

I'll try to find a solution for the Widgets.register<Xamarin.Forms.Style> that is not possible, once solved, this should enable you to implement style widgets :)

@@ -50,6 +50,7 @@ module View =

module Label =
let Text = Attributes.defineBindable<string> Xamarin.Forms.Label.TextProperty
let Style = Attributes.defineBindable<Style> Xamarin.Forms.Label.StyleProperty
Copy link
Owner

Choose a reason for hiding this comment

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

The StyleProperty is actually coming from Xamarin.Forms.NavigableElement, you can put it in its own module module NavigableElement.
This way your extension can be applied to IViewWidgetBuilder and be available to all controls :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review. Updated following your suggestions :)

@edgarfgp
Copy link
Contributor Author

Nice work! Regarding support for both XF.Style and style widgets, I want to keep Fabulous the easiest to use possible. We should avoid adding too many ways of doing the same thing.

Yeah Agreed

Copy link
Owner

@TimLariviere TimLariviere left a comment

Choose a reason for hiding this comment

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

Perfect, thanks!

@TimLariviere TimLariviere merged commit 7fb08aa into TimLariviere:main Nov 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants