Skip to content

Commit

Permalink
[android] CollectionView logical children grows with Header/Footer (#…
Browse files Browse the repository at this point in the history
…17012)

* [android] CollectionView logical children grows with Header/Footer

Fixes: #16961

Debugging the `ItemsSourceDoesNotLeak()` test with a new `Header` and
`Footer` on Android, there was some very odd behavior...

At the end of the test, `_logicalChildren` contained 9 items!

    Header
    Footer
    Header
    Footer
    Header
    null
    null
    null
    Footer

Where I would expect it to contain 5?!?

It appears the problem is:

* `CollectionView.ItemsSource` changes

* We notify Android's `RecyclerView` to refresh

* `RecyclerView` calls `OnCreateViewHolder()` for the header, footer,
  and all rows.

* `CreateHeaderFooterViewHolder()` is called again for the header &
  footer.

* Duplicate `viewHolder.View` items are added as logical children.

To fix this, an idea:

* Create a new `internal ContainsLogicalChild()` method

* Call this before adding the same `View` in
  `CreateHeaderFooterViewHolder()`

This isn't the *best*, because it will iterate over the
`_logicalChildren` to decide if the list contains the view. But this is
better than what strange behavior might result from this?

* Check `viewHolder.View.Parent != ItemsView`
  • Loading branch information
jonathanpeppers committed Aug 28, 2023
1 parent 60bdea7 commit df1fb2d
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ protected RecyclerView.ViewHolder CreateHeaderFooterViewHolder(object content, D
var viewHolder = SimpleViewHolder.FromFormsView(formsView, context, ItemsView);

// Propagate the binding context, visual, etc. from the ItemsView to the header/footer
ItemsView.AddLogicalChild(viewHolder.View);
if (viewHolder.View.Parent != ItemsView)
{
ItemsView.AddLogicalChild(viewHolder.View);
}

return viewHolder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public async Task ItemsSourceDoesNotLeak()
WeakReference weakReference = null;
var collectionView = new CollectionView
{
Header = new Label { Text = "Header" },
Footer = new Label { Text = "Footer" },
ItemTemplate = new DataTemplate(() => new Label())
};

Expand Down Expand Up @@ -75,7 +77,7 @@ public async Task ItemsSourceDoesNotLeak()
await AssertionExtensions.WaitForGC(weakReference);
Assert.False(weakReference.IsAlive, "ObservableCollection should not be alive!");
Assert.NotNull(logicalChildren);
Assert.True(logicalChildren.Count <= 3, "_logicalChildren should not grow in size!");
Assert.True(logicalChildren.Count <= 5, "_logicalChildren should not grow in size!");
}

[Theory]
Expand Down

0 comments on commit df1fb2d

Please sign in to comment.