Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

[iOS] Unsubscribe CellPropertyChanged when SwitchCellRenderer is disposed #3518

Merged
merged 15 commits into from
Sep 5, 2018
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
using Xamarin.Forms.CustomAttributes;
using Xamarin.Forms.Internals;
using System;
using System.Collections.Generic;
using System.Linq;
using static Xamarin.Forms.Controls.Issues.Issue3408;

#if UITEST
using Xamarin.UITest;
using NUnit.Framework;
#endif

namespace Xamarin.Forms.Controls.Issues
{
// This may crash for you on Android if you click too many buttons
// https://github.com/xamarin/Xamarin.Forms/issues/3603
[Preserve(AllMembers = true)]
[Issue(IssueTracker.Github, 3408, "System.ObjectDisposedException: from SwitchCellRenderer when changing ItemSource", PlatformAffected.iOS)]
public class Issue3408 : TestContentPage
{
public static List<Recommendation> GetRecommendations(object e)
{
switch (e)
{
case List<RecommendationsViewModel> pc: return pc.First().Recommendations;
case List<RecommendationsViewModel2> pc: return pc.First().Recommendations;
default: return null;
}
}
protected override void Init()
{

var grd = new Grid();

var aacountListView = new ListView();
aacountListView.HasUnevenRows = true;
aacountListView.ItemTemplate = new AccountDetailsDataTemplateSelector();
aacountListView.BindingContext = new List<RecommendationsViewModel> { new RecommendationsViewModel() };

aacountListView.SetBinding(ListView.ItemsSourceProperty, ".");
var btn = new Button
{
Text = "Change Source",
AutomationId = "btn1",
Command = new Command(() =>
{
aacountListView.BindingContext = new List<RecommendationsViewModel2> { new RecommendationsViewModel2() };
})
};
var btn2 = new Button
{
Text = "Change Property",
AutomationId = "btn2",
Command = new Command(() =>
{

foreach (var item in GetRecommendations(aacountListView.BindingContext))
{
item.Name = "New Item Name";
item.IsBusy = !item.IsBusy;
}

})
};
grd.Children.Add(aacountListView);
Grid.SetRow(aacountListView, 0);
grd.Children.Add(btn);
Grid.SetRow(btn, 1);
grd.Children.Add(btn2);
Grid.SetRow(btn2, 2);
Content = grd;
}

#if UITEST
[Test]
public void Issue3408Test ()
{
RunningApp.WaitForElement (q => q.Marked ("btn1"));
RunningApp.WaitForElement (q => q.Marked ("Click to Change"));
RunningApp.Tap(q => q.Marked("btn1"));
RunningApp.WaitForElement(q => q.Marked("This should have changed"));
RunningApp.Tap(q => q.Marked("btn2"));
RunningApp.WaitForElement(q => q.Marked("New Item Name"));
}
#endif

[Preserve(AllMembers = true)]
public class RecommendationsBaseViewModel : ViewModelBase
{
public string AccountName => $"";
public List<Recommendation> Recommendations { get; set; }
}

[Preserve(AllMembers = true)]
public class RecommendationsViewModel : RecommendationsBaseViewModel
{
public string AccountName => $"Recommendations";

public RecommendationsViewModel()
{
Recommendations = new List<Recommendation>()
{
new Recommendation(){ Name = "Click to Change"} ,
new Recommendation(){ Name = "Recommendations"} ,
new Recommendation(){ Name = "Recommendations"} ,
};
}
}

[Preserve(AllMembers = true)]
public class RecommendationsViewModel2 : RecommendationsBaseViewModel
{
public string AccountName => $"Recommendations 2";
public RecommendationsViewModel2()
{
Recommendations = new List<Recommendation>()
{
new Recommendation(){ Name = "This should have changed"} ,
new Recommendation(){ Name = "Recommendations 2"} ,
new Recommendation(){ Name = "Recommendations 2", IsBusy = true } ,
};
}
}

[Preserve(AllMembers = true)]
public class Recommendation : ViewModelBase
{
string _name;
public string Name
{
get { return _name; }
set
{
if (_name == value)
return;
_name = value;
OnPropertyChanged();
}
}
}

}

[Preserve(AllMembers = true)]
public class RecommendationsView : ContentView
{
public RecommendationsView()
{
Grid grd = new Grid();
var lst = new ListView
{
ItemTemplate = new DataTemplate(() =>
{
var swittch = new SwitchCell();
swittch.SetBinding(SwitchCell.TextProperty, new Binding("Name"));
swittch.SetBinding(SwitchCell.OnProperty, new Binding("IsBusy"));
return swittch;
})

};

lst.SetBinding(ListView.ItemsSourceProperty, new Binding("Recommendations"));
grd.Children.Add(lst);
Content = grd;
}

// This work around exists because of this issue
// https://github.com/xamarin/Xamarin.Forms/issues/3602
object context = null;
protected override void OnBindingContextChanged()
{
base.OnBindingContextChanged();
if (BindingContext == null)
Device.BeginInvokeOnMainThread(() => BindingContext = context);
else
context = BindingContext;
}
}

[Preserve(AllMembers = true)]
public class AccountDetailsDataTemplateSelector : DataTemplateSelector
{
public Lazy<DataTemplate> RecommendationsViewDataTemplate { get; }
public Lazy<ViewCell> RecommendationsView { get; }

public Lazy<DataTemplate> RecommendationsViewDataTemplate2 { get; }
public Lazy<ViewCell> RecommendationsView2 { get; }

public AccountDetailsDataTemplateSelector()
{
RecommendationsView = new Lazy<ViewCell>(() => new ViewCell() { View = new RecommendationsView() });
RecommendationsViewDataTemplate = new Lazy<DataTemplate>(() => new DataTemplate(() => RecommendationsView.Value));


RecommendationsView2 = new Lazy<ViewCell>(() => new ViewCell() { View = new RecommendationsView() });
RecommendationsViewDataTemplate2 = new Lazy<DataTemplate>(() => new DataTemplate(() => RecommendationsView2.Value));
}

protected override DataTemplate OnSelectTemplate(object item, BindableObject container)
{
if (item == null)
{
return null;
}

if (item is RecommendationsViewModel)
{
RecommendationsView.Value.BindingContext = item;
return RecommendationsViewDataTemplate.Value;
}

if (item is RecommendationsViewModel2)
{
RecommendationsView2.Value.BindingContext = item;
return RecommendationsViewDataTemplate2.Value;
}

throw new ArgumentException("Invalid ViewModel Type");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,7 @@
<Compile Include="$(MSBuildThisFileDirectory)Issue2728.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue1667.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue3012.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue3408.cs" />
</ItemGroup>
<ItemGroup>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla22229.xaml">
Expand Down Expand Up @@ -944,7 +945,7 @@
<ItemGroup>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue2858.xaml">
<SubType>Designer</SubType>
<Generator>MSBuild:Compile</Generator>
<Generator>MSBuild:UpdateDesignTimeXaml</Generator>
</EmbeddedResource>
</ItemGroup>
</Project>
20 changes: 14 additions & 6 deletions Xamarin.Forms.Platform.iOS/Cells/CellTableViewCell.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Xamarin.Forms.Platform.iOS
public class CellTableViewCell : UITableViewCell, INativeElementView
{
Cell _cell;
public Action<object, PropertyChangedEventArgs> PropertyChanged;
public Action<object, PropertyChangedEventArgs> CellPropertyChanged;
bool _disposed;

public CellTableViewCell(UITableViewCellStyle style, string key) : base(style, key)
Expand All @@ -23,22 +23,26 @@ public Cell Cell
return;

if (_cell != null)
{
_cell.PropertyChanged -= HandleCellPropertyChanged;
Device.BeginInvokeOnMainThread(_cell.SendDisappearing);

}
this._cell = value;
_cell = value;
Copy link
Contributor

Choose a reason for hiding this comment

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

are both of these needed?


if (_cell != null)
{
_cell.PropertyChanged += HandleCellPropertyChanged;
Device.BeginInvokeOnMainThread(_cell.SendAppearing);
}
}
}

public Element Element => Cell;

public void HandlePropertyChanged(object sender, PropertyChangedEventArgs e)
public void HandleCellPropertyChanged(object sender, PropertyChangedEventArgs e)
{
if (PropertyChanged != null)
PropertyChanged(this, e);
CellPropertyChanged?.Invoke(sender, e);
}

internal static UITableViewCell GetNativeCell(UITableView tableView, Cell cell, bool recycleCells = false, string templateId = "")
Expand Down Expand Up @@ -101,7 +105,11 @@ protected override void Dispose(bool disposing)

if (disposing)
{
PropertyChanged = null;
CellPropertyChanged = null;
if (_cell != null)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

no braces needed

_cell.PropertyChanged -= HandleCellPropertyChanged;
}
_cell = null;
}

Expand Down
4 changes: 2 additions & 2 deletions Xamarin.Forms.Platform.iOS/Cells/EntryCellRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ public override UITableViewCell GetCell(Cell item, UITableViewCell reusableCell,
tvc = new EntryCellTableViewCell(item.GetType().FullName);
else
{
tvc.Cell.PropertyChanged -= OnCellPropertyChanged;
tvc.CellPropertyChanged -= OnCellPropertyChanged;
tvc.TextFieldTextChanged -= OnTextFieldTextChanged;
tvc.KeyboardDoneButtonPressed -= OnKeyBoardDoneButtonPressed;
}

SetRealCell(item, tvc);

tvc.Cell = item;
tvc.Cell.PropertyChanged += OnCellPropertyChanged;
tvc.CellPropertyChanged += OnCellPropertyChanged;
tvc.TextFieldTextChanged += OnTextFieldTextChanged;
tvc.KeyboardDoneButtonPressed += OnKeyBoardDoneButtonPressed;

Expand Down
8 changes: 4 additions & 4 deletions Xamarin.Forms.Platform.iOS/Cells/ImageCellRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ public override UITableViewCell GetCell(Cell item, UITableViewCell reusableCell,
return result;
}

protected override void HandlePropertyChanged(object sender, PropertyChangedEventArgs args)
protected override void HandleCellPropertyChanged(object sender, PropertyChangedEventArgs args)
{
var tvc = (CellTableViewCell)sender;
var imageCell = (ImageCell)tvc.Cell;
var imageCell = (ImageCell)sender;
var tvc = (CellTableViewCell)GetRealCell(imageCell);

base.HandlePropertyChanged(sender, args);
base.HandleCellPropertyChanged(sender, args);

if (args.PropertyName == ImageCell.ImageSourceProperty.PropertyName)
SetImage(imageCell, tvc);
Expand Down
4 changes: 2 additions & 2 deletions Xamarin.Forms.Platform.iOS/Cells/SwitchCellRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public override UITableViewCell GetCell(Cell item, UITableViewCell reusableCell,
else
{
uiSwitch = tvc.AccessoryView as UISwitch;
tvc.Cell.PropertyChanged -= OnCellPropertyChanged;
tvc.CellPropertyChanged -= OnCellPropertyChanged;
}

SetRealCell(item, tvc);
Expand All @@ -33,7 +33,7 @@ public override UITableViewCell GetCell(Cell item, UITableViewCell reusableCell,
var boolCell = (SwitchCell)item;

tvc.Cell = item;
tvc.Cell.PropertyChanged += OnCellPropertyChanged;
tvc.CellPropertyChanged += OnCellPropertyChanged;
tvc.AccessoryView = uiSwitch;
tvc.TextLabel.Text = boolCell.Text;

Expand Down
14 changes: 8 additions & 6 deletions Xamarin.Forms.Platform.iOS/Cells/TextCellRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ public override UITableViewCell GetCell(Cell item, UITableViewCell reusableCell,
if (tvc == null)
tvc = new CellTableViewCell(UITableViewCellStyle.Subtitle, item.GetType().FullName);
else
tvc.Cell.PropertyChanged -= tvc.HandlePropertyChanged;
tvc.CellPropertyChanged -= HandleCellPropertyChanged;

SetRealCell(item, tvc);

tvc.Cell = textCell;
textCell.PropertyChanged += tvc.HandlePropertyChanged;
tvc.PropertyChanged = HandlePropertyChanged;
tvc.CellPropertyChanged = HandleCellPropertyChanged;

tvc.TextLabel.Text = textCell.Text;
tvc.DetailTextLabel.Text = textCell.Detail;
Expand All @@ -36,10 +37,11 @@ public override UITableViewCell GetCell(Cell item, UITableViewCell reusableCell,
return tvc;
}

protected virtual void HandlePropertyChanged(object sender, PropertyChangedEventArgs args)
protected virtual void HandleCellPropertyChanged(object sender, PropertyChangedEventArgs args)
{
var tvc = (CellTableViewCell)sender;
var textCell = (TextCell)tvc.Cell;
var textCell = (TextCell)sender;
var tvc = (CellTableViewCell)GetRealCell(textCell);

if (args.PropertyName == TextCell.TextProperty.PropertyName)
{
tvc.TextLabel.Text = ((TextCell)tvc.Cell).Text;
Expand Down