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

Small improvements #6

Merged
merged 3 commits into from
May 5, 2020
Merged

Conversation

domagojmedo
Copy link

  • Observable collections don't need backing fields

  • resized confirmed.png

  • you can directly reference view model properties from view

@domagojmedo
Copy link
Author

Items spacing in collection view seems to be broken, I filed a bug on xamarin forms repo, we will see what they say about it.

xamarin/Xamarin.Forms#10585

CommandParameter="{Binding .}"
BackgroundColor="Red"/>
</SwipeItems>
</SwipeView.LeftItems>
<SwipeView.RightItems>
<SwipeItems Mode="Execute">
<SwipeItem Text="View"
Command="{Binding Source={x:Reference HomePageView}, Path=BindingContext.ViewWidgetCommand}"
Command="{Binding Source={x:Reference WidgetCollection}, Path=BindingContext.ViewWidgetCommand}"
Copy link
Owner

Choose a reason for hiding this comment

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

Does referencing the WidgetCollection and calling the Command really work or is it a typo ?

Copy link
Author

Choose a reason for hiding this comment

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

This works, we are accessing the binding context of the collection view which is the binding context of the page

/// Force layout for collectionview
/// </summary>
/// <returns></returns>
private async Task ForceCollectionLayout(int delay = 400)
Copy link
Owner

@gabrielfreire gabrielfreire May 5, 2020

Choose a reason for hiding this comment

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

Didn't you get display bugs while Adding/Removing widgets here ? I mean, the reason i'm doing this is related to the Issue you opened with Xamarin.Forms where the Spacing between the items breaks when you add/remove items, but i wonder if you managed to get around that issue even without forcing layout

Copy link
Author

Choose a reason for hiding this comment

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

I changes it to not use spacing and instead sets small margin on each item for spacing. It's not ideal but it looks almost the same and you don't need to call layout and calculate height

</CollectionView.ItemsLayout>
<CollectionView.ItemTemplate>
<DataTemplate>
<SwipeView BackgroundColor="Transparent">
<ContentView>
<SwipeView Margin="5">
Copy link
Owner

Choose a reason for hiding this comment

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

Ahhh just saw it, lovely

@gabrielfreire gabrielfreire merged commit 4364baa into gabrielfreire:master May 5, 2020
@gabrielfreire
Copy link
Owner

That was a lovely contribution, thank you very much @domagojmedo

@gabrielfreire
Copy link
Owner

gabrielfreire commented May 5, 2020

Just tested on my Device, the Margin really did the trick, great stuff, app feels much smoother on Android now.

Will release a new version.

Did you get improvements on your device ?

@domagojmedo
Copy link
Author

It's smooth on my device, but some of the images are still really huge. You should make them smaller. Also check out https://github.com/Redth/ResizetizerNT/blob/master/README.md for easy way to create images for multiple resolutions at once. Haven't used it personally but that guy works on Xamarin so he probably knows what he's doing 🙂

There is also https://github.com/luberda-molinet/FFImageLoading/blob/master/README.md which further optimizes images on Android

@gabrielfreire
Copy link
Owner

It's smooth on my device

That's awesome.

thanks for the links, haven't thought of optimizing the images, will have to do it indeed.

gabrielfreire added a commit that referenced this pull request May 5, 2020
freiregabriel added a commit to freiregabriel/IMSCovid19Tracker that referenced this pull request Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants