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

Update to XF 3.5 #350

Merged
merged 21 commits into from May 16, 2019

Conversation

Projects
None yet
3 participants
@SergejDK
Copy link
Contributor

commented Mar 3, 2019

The following list shows which are missing right now. Will work on this the next days.

The questionmark behind few classes there I do not know if they are needed.
Interfaces, enums, abstract classes and static classes are not included.
I think that the Attributes and EventArgs do not need to be included, too ???

@TimLariviere could you maybe provide me with some hints for the classes below and the question above? I appreciate your help!

  • BaseShellItem
  • CarouselView
  • CollectionView
  • ItemsView
    ScrolledToRequest
    ScrollTo
  • ListItemsLayout - not needed?
  • MenuShellItem - not needed?
  • SearchHandler:
    ItemsSource
    ItemTemplate
    OnClearPlaceholderClicked
    OnItemSelected
    OnQueryChanged
    OnQueryConfirmed
  • SelectableItemsView
  • Shell
    GoToAsync
  • ShellContent
  • ShellGroupItem
  • ShellItem
  • ShellNavigationState
  • ShellSection

fix #329

Differences:
https://www.fuget.org/packages/Xamarin.Forms/3.5.0.129452/lib/netstandard2.0/diff/3.4.0.1009999/

SergejDK and others added some commits Mar 3, 2019

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Mar 5, 2019

I think that the Attributes and EventArgs do not need to be included, too ???

Yes, if we can reuse the existing classes, we do.
EventArgs are only a "bag" of data, so no need to wrap it in Fabulous.
Attributes as well, it's more related to making custom controls which is not something that concern Fabulous.

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Mar 5, 2019

I didn't had time to check XF 3.5 in details.
But I can say that we are only interested in actual controls, there's no need to wrap things like MenuItemCollection, NavigableElement, etc.

In the case of XXXCollection, Fabulous will expose a ViewElement list instead.

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

Great! Thanks for your feedback :)!

Sergej Dick added some commits Mar 6, 2019

@SergejDK SergejDK marked this pull request as ready for review Mar 6, 2019

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

I think i got everything...

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Mar 6, 2019

@SergejDK Do you know why CollectionView/CarouselView/Shell are in XF 3.5?
Everything I read about them is about Xamarin.Forms 4.0 Preview

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

They are already in 3.5 but only allowed to use when setting: global::Xamarin.Forms.Forms.SetFlags("Shell_Experimental");

https://docs.microsoft.com/de-de/xamarin/xamarin-forms/app-fundamentals/shell?tabs=android

The question why it is in 3.5 I think because of earlier feedback from the devs.

BONUS: Keep in mind that 3.5.0 also contains previews of other new features that the Xamarin team is currently developing,. We look forward to receiving your feedback on Shell, Visual, CollectionView, and CarouselView. As you explore these features, please take a moment to share with us your thoughts here.

https://blog.xamarin.com/5-things-youll-love-xamarin-forms-3-5/

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

@TimLariviere the fail on travis - is it the error where it only needs to rerun?

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Mar 6, 2019

Seems like it.
For some reasons, it failed to download Xamarin.Forms.
Will retrigger the CI

@TimLariviere TimLariviere reopened this Mar 6, 2019

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

@TimLariviere
Any other ideas? Most times the error goes away when rerun but right now not...
Maybe I missed a dependency?

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Mar 6, 2019

Apparently they changed their targeting framework with Xamarin.Forms 3.5.
In XF <= 3.4, Xamarin.Android was using the folder MonoAndroid10
But now, it's using either MonoAndroid81 or MonoAndroid90

image

So the links in both the samples app and the templates are no longer valid.

For example:

<Reference Include="Xamarin.Forms.Core">
    <HintPath>..\packages/Xamarin.Forms.XamarinFormsSdk/lib/MonoAndroid10/Xamarin.Forms.Core.dll</HintPath>
</Reference>

will become

<Reference Include="Xamarin.Forms.Core">
    <HintPath>..\packages/Xamarin.Forms.XamarinFormsSdk/lib/MonoAndroid90/Xamarin.Forms.Core.dll</HintPath>
</Reference>
@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

Thanks for looking into it! Could not do it when I was at work...
Will change it.

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Mar 11, 2019

Could you add an example of how to use the new controls in the AllControls samples?

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2019

Would it not be better to have a project for shell and not include it in AllControls?
I think the AllControls will be hard to read if we add everything in it.

And with the Shell it mabye easier to work on: #242

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Mar 11, 2019

I think we should still add the new controls to AllControls, it's more a smoke test than a sample anymore. That way, we can check the new controls are working and we don't break them later.

Though indeed, Shell would be easier to use in a new project and provides a good structure for new samples.

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2019

I will add simple parts in the allcontrols.

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Mar 11, 2019

Thanks!

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2019

Existing iOS and Android applications can adopt Shell and benefit immediately from navigation, performance, and extensibility improvements.

Because Shell seems to be only available on Android and iOS I have to ask for the platform. This is just a note that you won't be suprised about the addition to ask for android and ios.

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Mar 25, 2019

And if I am right I made more work than I had to do... I think I can delete the BaseShellItem?

Yes, I think you're right.
Some "abstract" controls don't need to be exposed in Fabulous. We only want to expose those we can use directly.

If I am correct then there is only CarouselView/CollectionView to make examples in allcontrols?

Yes, there seems to be only 3 new controls: Shell, CarouselView and CollectionView.
The others are more generic controls that serve as a base for those 3.

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Mar 26, 2019

Added examples for carousel and collectionview but I think there is something wrong.
If I try to add sth. like View.Label in the itemssource property it won't render.
So here I think I need to overwrite it somehow in the members or is there another way?

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Mar 28, 2019

@SergejDK The ItemsSource property on ItemsView (inherited by Carousel and CollectionView) is not enough for Fabulous. ViewElement won't render itself by default.

You need to do something like the ListView does:

  • Create a data holder with PropertyChanged support
    /// A custom data element for the ListView view element
    [<AllowNullLiteral>]
    type ListElementData(key) =
    let ev = new Event<_,_>()
    let mutable data = key
    interface IListElement with
    member x.Key = data
    [<CLIEvent>] member x.PropertyChanged = ev.Publish
    member x.Key
    with get() = data
    and set(value) =
    data <- value
    ev.Trigger(x, PropertyChangedEventArgs "Key")
  • Create a custom made control which will be responsible of rendering the related ViewElement
    /// A custom control for cells in the ListView view element
    type ViewElementCell() =
    inherit ViewCell()
    let mutable listElementOpt : IListElement option = None
    let mutable modelOpt : ViewElement option = None
    let createView (newModel: ViewElement) =
    match newModel.Create () with
    | :? View as v -> v
    | x -> failwithf "The cells of a ListView must each be some kind of 'View' and not a '%A'" (x.GetType())
    member x.OnDataPropertyChanged = PropertyChangedEventHandler(fun _ args ->
    match args.PropertyName, listElementOpt, modelOpt with
    | "Key", Some curr, Some prevModel ->
    curr.Key.UpdateIncremental (prevModel, x.View)
    modelOpt <- Some curr.Key
    | _ -> ()
    )
    override x.OnBindingContextChanged () =
    base.OnBindingContextChanged ()
    match x.BindingContext with
    | :? IListElement as curr ->
    let newModel = curr.Key
    match listElementOpt with
    | Some prev ->
    prev.PropertyChanged.RemoveHandler x.OnDataPropertyChanged
    curr.PropertyChanged.AddHandler x.OnDataPropertyChanged
    newModel.UpdateIncremental (prev.Key, x.View)
    | None ->
    curr.PropertyChanged.AddHandler x.OnDataPropertyChanged
    x.View <- createView newModel
    listElementOpt <- Some curr
    modelOpt <- Some curr.Key
    | _ ->
    match listElementOpt with
    | Some prev ->
    prev.PropertyChanged.RemoveHandler x.OnDataPropertyChanged
    listElementOpt <- None
    modelOpt <- None
    | None -> ()
  • Create a custom CarouselView/CollectionView initialized with a DataTemplate of the new control (which is responsible for rendering the ViewElement)
    /// A custom control for the ListView view element
    type CustomListView() =
    inherit ListView(ItemTemplate=DataTemplate(typeof<ViewElementCell>))
  • And finally, tell the Generator you want to instantiate your custom control whenever you call View.Carousel/View.CollectionView
    "name": "Xamarin.Forms.ListView",
    "customType": "Fabulous.DynamicViews.CustomListView",
    "members": [
    {
    "name": "ItemsSource",
    "uniqueName": "ListViewItems",
    "shortName": "items",
    "inputType": "seq<ViewElement>",
    "modelType": "seq<ViewElement>",
    "updateCode": "updateListViewItems"
    },

That way, when you add a View.Label in the ItemsSource property, Fabulous will wrap it in a ItemsViewElementData (1st step) and add it to the actual ItemsSource property.
Then when displaying, the Carousel/CollectionView will call ItemsViewItem to handle the rendering of the ItemsViewElementData.

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

@TimLariviere great explanation! Thanks!

As far as I see I can nearly copy everything except changing some names and inherited classes. Am I right?
-- Tried it that way but I get an InvalidCastException. Currently I am searching where this happens. It kinda does not show which line or anything else.

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Mar 29, 2019

As far as I see I can nearly copy everything except changing some names and inherited classes. Am I right?

Yes, it should mostly work as-is.
Just watch out for inherit ViewCell(), Carousel/CollectionView might not use this class for templating.

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

It seems like everythign can be used.
https://blog.xamarin.com/xamarin-forms-4-0-feature-preview-an-entirely-new-point-of-collectionview/

So maybe inherit from view directly? The difference to ListView is that listview uses ItemsView<Cell> but Collection/Carousel uses ItemsView class.

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented Apr 3, 2019

So maybe inherit from view directly?

@SergejDK You can inherit from ContentView and append the ViewElement's control to the Content property.

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2019

Seems to work but it does not display anything. It gets created correctly though...

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

The Labels seems to be created but not shown in the ui:
grafik

Is there still sth. wrong within ItemViewElementCell?
No Exception - nothing to check...
@TimLariviere Any hint on this?

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2019

Got time again.
Very interesting...
CollectionView on iOS is working on Android not.
CarouselView does not work. Will take a deeper look

2019-05-12 22:33:33.491404+0200 iOS[15591:395681] Unable to update view:: System.InvalidCastException: Specified cast is not valid. is the problem of carouselview.

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

@TimLariviere
if I get it right the CarouselView for iOS is not implemented in XF3.5: xamarin/Xamarin.Forms#5488
So this won't work no matter how much effort I put into it.

will test for android part.
Android carousel is working.
Only Collectionview is open now for android

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

@TimLariviere
it can be that with further updates maybe to 3.6/4.0 the issue won't be there anymore.

So maybe it is okay to merge here even though the examples for the 3 parts are currently not working.

For those example we can open each issue and work on the updates so we can catch up there and afterwards test the examples again.

What do you think?

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented May 15, 2019

@SergejDK Alright.
As those controls are preview only in XF 3.5, I think we can merge this PR for now and fix them once they're fully available (likely with XF 4.0).

I'll add a note for these controls in the docs saying that while we provide an initial support, there may be some issues with them.

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

@TimLariviere
Great Job! Thanks for keeping up with the work!

@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

@TimLariviere
I think it is the normal problem where we need to restart the build on travis?

@TimLariviere

This comment has been minimized.

Copy link
Collaborator

commented May 16, 2019

@SergejDK Thank you for your work in this long running PR. :)
I'm merging this now.

This evening I'll be releasing this in a new version on NuGet.

@TimLariviere TimLariviere merged commit 64595d4 into fsprojects:master May 16, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@SergejDK

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

@TimLariviere
always happy to help :)
I think everyone is waiting for the release

what do you think about making the step to 3.6 ? Maybe I can make it before evening and you could release it with 3.6?

@aspnetde

This comment has been minimized.

Copy link

commented May 16, 2019

what do you think about making the step to 3.6 ? Maybe I can make it before evening and you could release it with 3.6?

That would be awesome!

@TimLariviere TimLariviere referenced this pull request May 16, 2019

Merged

0.34.0 #410

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.