unoplatform / uno

Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported.
https://platform.uno
Apache License 2.0
8.76k stars 706 forks source link

Android ListView - Restore Add/Remove item animations #739

Open MatFillion opened 5 years ago

MatFillion commented 5 years ago

I'm submitting a...

Current behavior

Adding or removing items to ListView does not animate

Expected behavior

Adding or removing items to ListView animates

Minimal reproduction of the problem with instructions

Change feature flag FeatureConfiguration.NativeListViewBase.RemoveItemAnimator to false (default is true) to compare behaviour change

Environment

Nuget Package: 

Package Version(s): 

Affected platform(s):
- [ ] iOS
- [x] Android
- [ ] WebAssembly
- [ ] Windows
- [ ] Build tasks

Related to #702

proberge-dev commented 3 years ago

I am facing this issue on Android right now. Here is the code I've written that should trigger the delete animation.

public ObservableCollection<Message> Messages { get; } = new ObservableCollection<Message>();

// list is populated...

public IDynamicCommand DeleteMessage => this.GetCommandFromTask<Message>(async (ct, message) =>
{
    await RunOnDispatcher(ct, async _ =>
    {
        if (await message.CanBeDeleted())
        {
            Messages.Remove(message); // This should trigger animation
        }
    });
});

The Messages are used in a ListView and everything works well on iOS and UWP, but our client is expecting the animation to work on Android as well, so this is a blocker for us.

davidjohnoliver commented 3 years ago

@philipe-roberge Does setting the flag mentioned in the issue help?

Ie if you add the following to your app startup (eg ctor of App.xaml.cs):

#if __ANDROID__
    Uno.UI.FeatureConfiguration.NativeListViewBase.RemoveItemAnimator = false;
#endif

do you get item animations?

proberge-dev commented 3 years ago

Yes @davidjohnoliver, this did enable the animations for us. Was wondering if there was a reason this flag would be set to true by default?

davidjohnoliver commented 3 years ago

Yes @davidjohnoliver, this did enable the animations for us. Was wondering if there was a reason this flag would be set to true by default?

It was disabled as a workaround for a crash that was occurring under particular circumstances: https://github.com/unoplatform/uno/pull/702

@Xiaoy312 may have more details.

Xiaoy312 commented 3 years ago

The original crash was pretty elusive, I am having trouble reproducing it now.

We can restore flag for now, and see if the problem resurges somehow. It should be a safe bet. If it comes back, we will have a repro and existing workaround. If not that will be perfect.

Xiaoy312 commented 3 years ago

Currently removing this workaround causes a weird bug on android where a ListView inside a StackPanel would not re-measure properly when new item(s) are added, causing a ScrollViewer to appears instead of the ListView resizing when there is enough space to use.

This issue can be easily reproduced with the ListView_ObservableCollection_Unused_Space test in uno, by adding this line to its .ctor:

public ListView_ObservableCollection_Unused_Space()
{
+    Uno.UI.FeatureConfiguration.NativeListViewBase.RemoveItemAnimator = false;
jeromelaban commented 3 years ago

@Xiaoy312 Is it something you can show in a video/screenshot ?

davidjohnoliver commented 2 years ago

Note to contributors:

The issue relates to this code path:

https://github.com/unoplatform/uno/blob/68f1f7de107c26179350b7ca2ae0c9506b02d776/src/Uno.UI/UI/Xaml/Controls/ListViewBase/VirtualizingPanelLayout.Android.cs#L1184-L1190

When an item animation occurs, we do the measure without adding any new items - this allows the RecyclerView to know which of the items should be animated in. However in the case of a non-stretched list specifically, this means that it doesn't correctly request additional space to arrange in.

One potential fix is to add new items, measure the new size, and remove the items again before returning from UpdateLayout().