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

Commit 2202e3e

Browse files
PureWeenPureWeen
authored
When navigating between Shell Items disconnect the renderers from the xplat elements (#11791)
* Disconnect renderer before disposing of it * - fix ui test * - remove any processing animations * - cleanup code * - fix ui test and add null checks * - include android fixes #11784 fixes #11777
1 parent 4ca8e79 commit 2202e3e

File tree

13 files changed

+303
-81
lines changed

13 files changed

+303
-81
lines changed

‎Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue11723.cs‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Collections.ObjectModel;
4+
using System.Text;
5+
using Xamarin.Forms.CustomAttributes;
6+
using Xamarin.Forms.Internals;
7+
using System.Threading.Tasks;
8+
using System.Runtime.CompilerServices;
9+
using System.Linq;
10+
11+
12+
#if UITEST
13+
using Xamarin.UITest;
14+
using NUnit.Framework;
15+
using Xamarin.Forms.Core.UITests;
16+
#endif
17+
18+
namespace Xamarin.Forms.Controls.Issues
19+
{
20+
[Preserve(AllMembers = true)]
21+
[Issue(IssueTracker.Github, 11723, "[Bug] ContentPage in NavigationStack misplaced initially",
22+
PlatformAffected.iOS)]
23+
#if UITEST
24+
[NUnit.Framework.Category(Core.UITests.UITestCategories.Github10000)]
25+
[NUnit.Framework.Category(UITestCategories.Shell)]
26+
#endif
27+
public class Issue11723 : TestShell
28+
{
29+
int labelIndex = 0;
30+
ContentPage CreateContentPage()
31+
{
32+
var page = new ContentPage()
33+
{
34+
Content = new StackLayout()
35+
{
36+
Children =
37+
{
38+
new Label()
39+
{
40+
Text = "As you navigate this text should show up in the correct spot. If it's hidden and then shows up this test has failed.",
41+
AutomationId = $"InitialText{labelIndex}"
42+
},
43+
new Button()
44+
{
45+
Text = "Push Page",
46+
AutomationId = "PushPage",
47+
Command = new Command(async () =>
48+
{
49+
labelIndex++;
50+
await Navigation.PushAsync(CreateContentPage());
51+
})
52+
},
53+
new Button()
54+
{
55+
Text = "Pop Page",
56+
AutomationId = "PopPage",
57+
Command = new Command(async () =>
58+
{
59+
labelIndex--;
60+
await Navigation.PopAsync();
61+
})
62+
}
63+
}
64+
}
65+
};
66+
67+
SetNavBarIsVisible(page, false);
68+
PlatformConfiguration.iOSSpecific.Page.SetUseSafeArea(page, true);
69+
70+
return page;
71+
}
72+
73+
protected override void Init()
74+
{
75+
AddContentPage(CreateContentPage());
76+
}
77+
78+
79+
#if UITEST
80+
[Test]
81+
public void PaddingIsSetOnPageBeforeItsVisible()
82+
{
83+
var initialTextPosition = RunningApp.WaitForFirstElement($"InitialText0").Rect;
84+
RunningApp.Tap("PushPage");
85+
CompareTextLocation(initialTextPosition, 1);
86+
RunningApp.Tap("PushPage");
87+
CompareTextLocation(initialTextPosition, 2);
88+
RunningApp.Tap("PushPage");
89+
CompareTextLocation(initialTextPosition, 3);
90+
RunningApp.Tap("PopPage");
91+
CompareTextLocation(initialTextPosition, 2);
92+
RunningApp.Tap("PopPage");
93+
CompareTextLocation(initialTextPosition, 1);
94+
95+
}
96+
97+
void CompareTextLocation(UITest.Queries.AppRect initialRect, int i)
98+
{
99+
var newRect = RunningApp.WaitForFirstElement($"InitialText{i}").Rect;
100+
101+
Assert.AreEqual(newRect.X, initialRect.X, $"Error With Test :{i}");
102+
Assert.AreEqual(newRect.Y, initialRect.Y, $"Error With Test :{i}");
103+
Assert.AreEqual(newRect.CenterX, initialRect.CenterX, $"Error With Test :{i}");
104+
Assert.AreEqual(newRect.CenterY, initialRect.CenterY, $"Error With Test :{i}");
105+
}
106+
#endif
107+
}
108+
}

‎Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,6 +1445,7 @@
14451445
<Compile Include="$(MSBuildThisFileDirectory)Issue11430.cs" />
14461446
<Compile Include="$(MSBuildThisFileDirectory)Issue11247.cs" />
14471447
<Compile Include="$(MSBuildThisFileDirectory)Issue10608.cs" />
1448+
<Compile Include="$(MSBuildThisFileDirectory)Issue11723.cs" />
14481449
</ItemGroup>
14491450
<ItemGroup>
14501451
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla22229.xaml">

‎Xamarin.Forms.Platform.iOS/EventTracker.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,12 @@ ObservableCollection<IGestureRecognizer> ElementGestureRecognizers
5757
}
5858
}
5959

60+
internal void Disconnect()
61+
{
62+
if (ElementGestureRecognizers != null)
63+
ElementGestureRecognizers.CollectionChanged -= _collectionChangedHandler;
64+
}
65+
6066
public void Dispose()
6167
{
6268
if (_disposed)
@@ -75,8 +81,7 @@ public void Dispose()
7581

7682
_gestureRecognizers.Clear();
7783

78-
if (ElementGestureRecognizers != null)
79-
ElementGestureRecognizers.CollectionChanged -= _collectionChangedHandler;
84+
Disconnect();
8085

8186
_handler = null;
8287
}

‎Xamarin.Forms.Platform.iOS/Renderers/IDisconnectable.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
using System;
2+
namespace Xamarin.Forms.Platform.iOS
3+
{
4+
internal interface IDisconnectable
5+
{
6+
void Disconnect();
7+
}
8+
}

‎Xamarin.Forms.Platform.iOS/Renderers/PageRenderer.cs‎

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
namespace Xamarin.Forms.Platform.iOS
1111
{
12-
public class PageRenderer : UIViewController, IVisualElementRenderer, IEffectControlProvider, IAccessibilityElementsController, IShellContentInsetObserver
12+
public class PageRenderer : UIViewController, IVisualElementRenderer, IEffectControlProvider, IAccessibilityElementsController, IShellContentInsetObserver, IDisconnectable
1313
{
1414
bool _appeared;
1515
bool _disposed;
@@ -110,7 +110,7 @@ public UIView NativeView
110110
{
111111
get { return _disposed ? null : View; }
112112
}
113-
113+
114114
public void SetElement(VisualElement element)
115115
{
116116
VisualElement oldElement = Element;
@@ -172,7 +172,7 @@ public override void ViewDidLayoutSubviews()
172172
{
173173
base.ViewDidLayoutSubviews();
174174

175-
if (_disposed)
175+
if (_disposed || Element == null)
176176
return;
177177

178178
if (Element.Parent is BaseShellItem)
@@ -195,7 +195,7 @@ public override void ViewDidAppear(bool animated)
195195
{
196196
base.ViewDidAppear(animated);
197197

198-
if (_appeared || _disposed)
198+
if (_appeared || _disposed || Element == null)
199199
return;
200200

201201
_appeared = true;
@@ -213,15 +213,15 @@ public override void ViewDidDisappear(bool animated)
213213
{
214214
base.ViewDidDisappear(animated);
215215

216-
if (!_appeared || _disposed)
216+
if (!_appeared || _disposed || Element == null)
217217
return;
218218

219219
_appeared = false;
220220

221221
if (Element.Parent is CarouselPage)
222222
return;
223223

224-
Page.SendDisappearing();
224+
Page?.SendDisappearing();
225225
}
226226

227227
public override void ViewDidLoad()
@@ -260,6 +260,25 @@ public override void ViewWillDisappear(bool animated)
260260
NativeView?.Window?.EndEditing(true);
261261
}
262262

263+
void IDisconnectable.Disconnect()
264+
{
265+
if (_shellSection != null)
266+
{
267+
((IShellSectionController)_shellSection).RemoveContentInsetObserver(this);
268+
_shellSection = null;
269+
}
270+
271+
if (Element != null)
272+
{
273+
Element.PropertyChanged -= OnHandlePropertyChanged;
274+
Platform.SetRenderer(Element, null);
275+
Element = null;
276+
}
277+
278+
_events?.Disconnect();
279+
_packager?.Disconnect();
280+
_tracker?.Disconnect();
281+
}
263282

264283
protected override void Dispose(bool disposing)
265284
{
@@ -268,36 +287,19 @@ protected override void Dispose(bool disposing)
268287

269288
if (disposing)
270289
{
271-
if (_shellSection != null)
272-
{
273-
((IShellSectionController)_shellSection).RemoveContentInsetObserver(this);
274-
_shellSection = null;
275-
}
290+
var page = Page;
291+
(this as IDisconnectable).Disconnect();
276292

277-
Element.PropertyChanged -= OnHandlePropertyChanged;
278-
Platform.SetRenderer(Element, null);
279293
if (_appeared)
280-
Page.SendDisappearing();
294+
page?.SendDisappearing();
281295

282296
_appeared = false;
283-
284-
if (_events != null)
285-
{
286-
_events.Dispose();
287-
_events = null;
288-
}
289-
290-
if (_packager != null)
291-
{
292-
_packager.Dispose();
293-
_packager = null;
294-
}
295-
296-
if (_tracker != null)
297-
{
298-
_tracker.Dispose();
299-
_tracker = null;
300-
}
297+
_events?.Dispose();
298+
_packager?.Dispose();
299+
_tracker?.Dispose();
300+
_events = null;
301+
_packager = null;
302+
_tracker = null;
301303

302304
Element = null;
303305
Container?.Dispose();
@@ -396,9 +398,6 @@ void UpdateUseSafeArea()
396398
if (!IsPartOfShell && !Forms.IsiOS11OrNewer)
397399
return;
398400

399-
if (IsPartOfShell && !_appeared)
400-
return;
401-
402401
var tabThickness = _tabThickness;
403402
if (!_isInItems)
404403
tabThickness = 0;

‎Xamarin.Forms.Platform.iOS/Renderers/ShellItemRenderer.cs‎

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
namespace Xamarin.Forms.Platform.iOS
1212
{
13-
public class ShellItemRenderer : UITabBarController, IShellItemRenderer, IAppearanceObserver, IUINavigationControllerDelegate
13+
public class ShellItemRenderer : UITabBarController, IShellItemRenderer, IAppearanceObserver, IUINavigationControllerDelegate, IDisconnectable
1414
{
1515
#region IShellItemRenderer
1616

@@ -113,36 +113,60 @@ public override void ViewDidLoad()
113113
};
114114
}
115115

116-
protected override void Dispose(bool disposing)
116+
void IDisconnectable.Disconnect()
117117
{
118-
base.Dispose(disposing);
119-
120-
if (disposing && !_disposed)
118+
if (_sectionRenderers != null)
121119
{
122-
_disposed = true;
123120
foreach (var kvp in _sectionRenderers.ToList())
124121
{
125-
var renderer = kvp.Value;
126-
RemoveRenderer(renderer);
122+
var renderer = kvp.Value as IDisconnectable;
123+
renderer?.Disconnect();
124+
kvp.Value.ShellSection.PropertyChanged -= OnShellSectionPropertyChanged;
127125
}
126+
}
128127

129-
if (_displayedPage != null)
130-
_displayedPage.PropertyChanged -= OnDisplayedPagePropertyChanged;
128+
if (_displayedPage != null)
129+
_displayedPage.PropertyChanged -= OnDisplayedPagePropertyChanged;
131130

132-
if (_currentSection != null)
133-
((IShellSectionController)_currentSection).RemoveDisplayedPageObserver(this);
131+
if (_currentSection != null)
132+
((IShellSectionController)_currentSection).RemoveDisplayedPageObserver(this);
134133

135134

136-
_sectionRenderers.Clear();
135+
if(ShellItem != null)
137136
ShellItem.PropertyChanged -= OnElementPropertyChanged;
138-
((IShellController)_context.Shell).RemoveAppearanceObserver(this);
137+
138+
if(_context?.Shell is IShellController shellController)
139+
shellController.RemoveAppearanceObserver(this);
140+
141+
if(ShellItemController != null)
139142
ShellItemController.ItemsCollectionChanged -= OnItemsCollectionChanged;
143+
}
144+
145+
protected override void Dispose(bool disposing)
146+
{
147+
if (_disposed)
148+
return;
149+
150+
_disposed = true;
140151

152+
if (disposing)
153+
{
154+
(this as IDisconnectable).Disconnect();
155+
156+
foreach (var kvp in _sectionRenderers.ToList())
157+
{
158+
var renderer = kvp.Value;
159+
RemoveRenderer(renderer);
160+
}
161+
162+
_sectionRenderers.Clear();
141163
CurrentRenderer = null;
142164
_shellItem = null;
143165
_currentSection = null;
144166
_displayedPage = null;
145167
}
168+
169+
base.Dispose(disposing);
146170
}
147171

148172
protected virtual void OnElementPropertyChanged(object sender, PropertyChangedEventArgs e)

‎Xamarin.Forms.Platform.iOS/Renderers/ShellItemTransition.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ public Task Transition(IShellItemRenderer oldRenderer, IShellItemRenderer newRen
1010
TaskCompletionSource<bool> task = new TaskCompletionSource<bool>();
1111
var oldView = oldRenderer.ViewController.View;
1212
var newView = newRenderer.ViewController.View;
13+
oldView.Layer.RemoveAllAnimations();
1314
newView.Alpha = 0;
1415

1516
newView.Superview.InsertSubviewAbove(newView, oldView);

‎Xamarin.Forms.Platform.iOS/Renderers/ShellRenderer.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ protected virtual void OnElementSet(Shell element)
249249
protected async void SetCurrentShellItemController(IShellItemRenderer value)
250250
{
251251
var oldRenderer = _currentShellItemRenderer;
252+
(oldRenderer as IDisconnectable)?.Disconnect();
252253
var newRenderer = value;
253254

254255
_currentShellItemRenderer = value;
@@ -258,7 +259,7 @@ protected async void SetCurrentShellItemController(IShellItemRenderer value)
258259
View.SendSubviewToBack(newRenderer.ViewController.View);
259260

260261
newRenderer.ViewController.View.Frame = View.Bounds;
261-
262+
262263
if (oldRenderer != null)
263264
{
264265
var transition = CreateShellItemTransition();

0 commit comments

Comments
 (0)