search

TimLariviere / Fabulous-new

Fabulous v2 - Work in progress
https://timothelariviere.com/Fabulous-new/
Other
41 stars 3 forks source link

Add image extensions based on the specification #58

Closed edgarfgp closed 2 years ago

edgarfgp commented 2 years ago

This PR add Image extensions based on https://docs.microsoft.com/en-us/dotnet/api/xamarin.forms.image?view=xamarin-forms :

edgarfgp commented 2 years ago

@TimLariviere This is ready for another review. Thanks for you patience 😀

edgarfgp commented 2 years ago

@TimLariviere I think I have addressed all you suggestions 😀 . I had to put aspect on the image as the first parameter for convenience with the dark support. Let me know if there is anything I can improve .

TimLariviere commented 2 years ago

Thanks!

I had to put aspect on the image as the first parameter for convenience with the dark support

I'm ok with that.

Btw Attributes.getAppTheme was not making much sense given AppThemeValues.create was already checking for Some | None, and you also were checking for this inside the extension methods. So I moved create into a module (to avoid having to specify the generic type) and changed the extension methods a bit to only convert the dark value when needed.

Inlining will transform match dark with None -> None | Some v -> (...) to simply v anyway

edgarfgp commented 2 years ago

Thanks for the explanation. Yeah it look much better now . I will keep in mind for my next PR’s 😀

Edit: I forgot to update the samples . I will make sure this does not Happen from now on