Xamarin.forms: [Android] SeparatorColor of ListView is NOT updated dynamically

[bill2004158]

Description

SeparatorColor of ListView is NOT updated dynamically.

1. when SeparatorColor value changed, the Renderer will refresh native listview control
   > else if (e.PropertyName == ListView.SeparatorColorProperty.PropertyName || e.PropertyName == ListView.SeparatorVisibilityProperty.PropertyName)
   > _adapter.NotifyDataSetChanged();

https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Platform.Android/Renderers/ListViewRenderer.cs#L197

1. then it will re-draw the control

      AView bline;
      UpdateSeparatorVisibility(cell, cellIsBeingReused, isHeader, nextCellIsHeader, layout, out bline);
      UpdateSeparatorColor(isHeader, bline);

https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Platform.Android/Renderers/ListViewAdapter.cs#L316

1. the problem is in UpdateSeparatorVisibility(), it will not set the value for bline, and return directly if reused.
   > void UpdateSeparatorVisibility(Cell cell, bool cellIsBeingReused, bool isHeader, bool nextCellIsHeader, ConditionalFocusLayout layout, out AView bline)
   > {
   > bline = null;
   > if (cellIsBeingReused)
   > return;

https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Platform.Android/Renderers/ListViewAdapter.cs#L659

1. thus, in UpdateSeparatorColor(), it will not update the color as bline is null:
   > void UpdateSeparatorColor(bool isHeader, AView bline)
   > {
   > if (bline == null)
   > return;

https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Platform.Android/Renderers/ListViewAdapter.cs#L677

[bill2004158]

sample.zip

  void OnItemTapped(object sender, ItemTappedEventArgs e)
  {
      if (e == null) return; // has been set to null, do not 'process' tapped event

      Debug.WriteLine("Tapped: " + e.Item);

      _showRedSeparator = !_showRedSeparator;
      listView.SeparatorColor = _showRedSeparator ? Color.Red : Color.Green;

       ((ListView)sender).SelectedItem = null; // de-select the row
  }

the separator color is not changed in android if tapp on list item

[bill2004158]

any update?

[techduggu]

Hi Team / @hartez ,

I'd like to work on this and fix it. I've gone through its existing implementation (thanks to @bill2004158 for providing excellent-report of this issue) and noticed few things (seems more of bugs even with current version of Xamarin.Forms):

• First of all, the current issue (mentioned by @bill2004158 ) is not limited to SeparatorColor but also impacts SeparatorVisibility (this also won't update dynamically).

• This is due to the cellIsBeingReused code logic written before checking the current value of _listView.SeparatorVisibility or _listView.SeparatorColor.
  https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Platform.Android/Renderers/ListViewAdapter.cs#L668

• We can actually improve the way we call UpdateSeparatorVisibility and UpdateSeparatorColor methods to draw the divider control by checking the current state of Separator before calling it, something like this:

AView bline;
bool isSeparatorVisible = _listView.SeparatorVisibility == SeparatorVisibility.Default;
if (isSeparatorVisible)
{
    UpdateSeparatorVisibility(cell, cellIsBeingReused, isHeader, nextCellIsHeader, layout, out bline);

    UpdateSeparatorColor(isHeader, bline);
}

• And inside UpdateSeparatorVisibility method, in the event where lets say cellIsBeingReused is true (i.e. ListView has been rendered), we should either fetch existing divider view (if possible to do) or create new (similar to existing LOC#674) and assign it to bline before return; statement. We should be also removing the below line from this method as it is already being verified as a boundary condition (in previous step):
  bool isSeparatorVisible = _listView.SeparatorVisibility == SeparatorVisibility.Default;

Basically the improvements are around the way we call Update Separator methods. We should ideally be checking if there's something to update (considering its current value) before calling it and then reusing the cellIsBeingReused logic for performance improvement. Please provide your feedback on the above specifications and if I can proceed further to fix this.

Thanks!

[hartez]

@techduggu That all sounds reasonable to me. If you want to take a crack at it, we'd love to see the PR!

[techduggu]

Already on it. Will be raising it soon!