Skip to content

Commit c9e29aa

Browse files
NathanWalkerNathanWalker
authored
fix(core): improve loaded/unloaded handling to be stable and consistent (#10170)
1 parent a69a9d6 commit c9e29aa

File tree

12 files changed

+122
-167
lines changed

12 files changed

+122
-167
lines changed

‎packages/core/data/observable/index.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -291,22 +291,24 @@ export class Observable implements ObservableDefinition {
291291
}
292292
for (let i = observers.length - 1; i >= 0; i--) {
293293
const entry = observers[i];
294-
if (entry.once) {
295-
observers.splice(i, 1);
296-
}
294+
if (entry) {
295+
if (entry.once) {
296+
observers.splice(i, 1);
297+
}
297298

298-
let returnValue;
299-
if (entry.thisArg) {
300-
returnValue = entry.callback.apply(entry.thisArg, [data]);
301-
} else {
302-
returnValue = entry.callback(data);
303-
}
299+
let returnValue;
300+
if (entry.thisArg) {
301+
returnValue = entry.callback.apply(entry.thisArg, [data]);
302+
} else {
303+
returnValue = entry.callback(data);
304+
}
304305

305-
// This ensures errors thrown inside asynchronous functions do not get swallowed
306-
if (returnValue && returnValue instanceof Promise) {
307-
returnValue.catch((err) => {
308-
console.error(err);
309-
});
306+
// This ensures errors thrown inside asynchronous functions do not get swallowed
307+
if (returnValue && returnValue instanceof Promise) {
308+
returnValue.catch((err) => {
309+
console.error(err);
310+
});
311+
}
310312
}
311313
}
312314
}

‎packages/core/ui/button/index.ios.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,8 @@ export class Button extends ButtonBase {
2121

2222
public initNativeView(): void {
2323
super.initNativeView();
24-
const nativeView = this.nativeViewProtected;
2524
this._tapHandler = TapHandlerImpl.initWithOwner(new WeakRef(this));
26-
nativeView.addTargetActionForControlEvents(this._tapHandler, 'tap', UIControlEvents.TouchUpInside);
25+
this.nativeViewProtected.addTargetActionForControlEvents(this._tapHandler, 'tap', UIControlEvents.TouchUpInside);
2726
}
2827

2928
public disposeNativeView(): void {

‎packages/core/ui/core/view/index.android.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -430,14 +430,6 @@ export class View extends ViewCommon {
430430

431431
@profile
432432
public onUnloaded() {
433-
if (this.touchListenerIsSet) {
434-
this.touchListenerIsSet = false;
435-
if (this.nativeViewProtected) {
436-
this.nativeViewProtected.setOnTouchListener(null);
437-
this.nativeViewProtected.setClickable(this._isClickable);
438-
}
439-
}
440-
441433
this._manager = null;
442434
this._rootManager = null;
443435
super.onUnloaded();
@@ -474,7 +466,6 @@ export class View extends ViewCommon {
474466
public initNativeView(): void {
475467
super.initNativeView();
476468
this._isClickable = this.nativeViewProtected.isClickable();
477-
478469
if (this.needsOnLayoutChangeListener()) {
479470
this.setOnLayoutChangeListener();
480471
}
@@ -486,15 +477,20 @@ export class View extends ViewCommon {
486477

487478
public disposeNativeView(): void {
488479
super.disposeNativeView();
489-
480+
if (this.touchListenerIsSet) {
481+
this.touchListenerIsSet = false;
482+
if (this.nativeViewProtected) {
483+
this.nativeViewProtected.setOnTouchListener(null);
484+
}
485+
}
490486
if (this.layoutChangeListenerIsSet) {
491487
this.layoutChangeListenerIsSet = false;
492488
this.nativeViewProtected.removeOnLayoutChangeListener(this.layoutChangeListener);
493489
}
494490
}
495491

496492
setOnTouchListener() {
497-
if (!this.nativeViewProtected || !this.hasGestureObservers()) {
493+
if (this.touchListenerIsSet || !this.nativeViewProtected || !this.hasGestureObservers()) {
498494
return;
499495
}
500496

‎packages/core/ui/core/view/index.ios.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -479,12 +479,14 @@ export class View extends ViewCommon implements ViewDefinition {
479479
} else {
480480
//use CSS & attribute width & height if option is not provided
481481
const handler = () => {
482-
const w = <number>(this.width || this.style.width);
483-
const h = <number>(this.height || this.style.height);
484-
485-
//TODO: only numeric value is supported, percentage value is not supported like Android
486-
if (w > 0 && h > 0) {
487-
controller.preferredContentSize = CGSizeMake(w, h);
482+
if (controller) {
483+
const w = <number>(this.width || this.style.width);
484+
const h = <number>(this.height || this.style.height);
485+
486+
//TODO: only numeric value is supported, percentage value is not supported like Android
487+
if (w > 0 && h > 0) {
488+
controller.preferredContentSize = CGSizeMake(w, h);
489+
}
488490
}
489491

490492
this.off(View.loadedEvent, handler);

‎packages/core/ui/list-picker/index.ios.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ export class ListPicker extends ListPickerBase {
1717

1818
initNativeView() {
1919
super.initNativeView();
20-
const nativeView = this.nativeViewProtected;
21-
nativeView.dataSource = this._dataSource = ListPickerDataSource.initWithOwner(new WeakRef(this));
20+
this.nativeViewProtected.dataSource = this._dataSource = ListPickerDataSource.initWithOwner(new WeakRef(this));
2221
this._delegate = ListPickerDelegateImpl.initWithOwner(new WeakRef(this));
22+
this.nativeViewProtected.delegate = this._delegate;
2323
}
2424

2525
public disposeNativeView() {
@@ -33,17 +33,6 @@ export class ListPicker extends ListPickerBase {
3333
return this.nativeViewProtected;
3434
}
3535

36-
@profile
37-
public onLoaded() {
38-
super.onLoaded();
39-
this.ios.delegate = this._delegate;
40-
}
41-
42-
public onUnloaded() {
43-
this.ios.delegate = null;
44-
super.onUnloaded();
45-
}
46-
4736
[selectedIndexProperty.getDefault](): number {
4837
return -1;
4938
}

‎packages/core/ui/scroll-view/index.android.ts

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -118,17 +118,10 @@ export class ScrollView extends ScrollViewBase {
118118
this.nativeViewProtected.setId(this._androidViewId);
119119
}
120120

121-
public _onOrientationChanged() {
122-
if (this.nativeViewProtected) {
123-
const parent = this.parent;
124-
if (parent) {
125-
parent._removeView(this);
126-
parent._addView(this);
127-
}
121+
protected addNativeListener() {
122+
if (!this.nativeViewProtected) {
123+
return;
128124
}
129-
}
130-
131-
protected attachNative() {
132125
const that = new WeakRef(this);
133126
if (this.orientation === 'vertical') {
134127
this.scrollChangeHandler = new androidx.core.widget.NestedScrollView.OnScrollChangeListener({
@@ -144,7 +137,7 @@ export class ScrollView extends ScrollViewBase {
144137
}
145138
},
146139
});
147-
this.nativeView.setOnScrollChangeListener(this.scrollChangeHandler);
140+
this.nativeViewProtected.setOnScrollChangeListener(this.scrollChangeHandler);
148141
} else {
149142
this.handler = new android.view.ViewTreeObserver.OnScrollChangedListener({
150143
onScrollChanged: function () {
@@ -158,6 +151,35 @@ export class ScrollView extends ScrollViewBase {
158151
}
159152
}
160153

154+
protected removeNativeListener() {
155+
if (!this.nativeViewProtected) {
156+
return;
157+
}
158+
if (this.handler) {
159+
this.nativeViewProtected?.getViewTreeObserver().removeOnScrollChangedListener(this.handler);
160+
this.handler = null;
161+
}
162+
if (this.scrollChangeHandler) {
163+
this.nativeView?.setOnScrollChangeListener(null);
164+
this.scrollChangeHandler = null;
165+
}
166+
}
167+
168+
disposeNativeView() {
169+
super.disposeNativeView();
170+
this.removeNativeListener();
171+
}
172+
173+
public _onOrientationChanged() {
174+
if (this.nativeViewProtected) {
175+
const parent = this.parent;
176+
if (parent) {
177+
parent._removeView(this);
178+
parent._addView(this);
179+
}
180+
}
181+
}
182+
161183
private _lastScrollX = -1;
162184
private _lastScrollY = -1;
163185
private _onScrollChanged() {
@@ -179,17 +201,6 @@ export class ScrollView extends ScrollViewBase {
179201
}
180202
}
181203
}
182-
183-
protected dettachNative() {
184-
if (this.handler) {
185-
this.nativeViewProtected?.getViewTreeObserver().removeOnScrollChangedListener(this.handler);
186-
this.handler = null;
187-
}
188-
if (this.scrollChangeHandler) {
189-
this.nativeView?.setOnScrollChangeListener(null);
190-
this.scrollChangeHandler = null;
191-
}
192-
}
193204
}
194205

195206
ScrollView.prototype.recycleNativeView = 'never';

‎packages/core/ui/scroll-view/index.ios.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,7 @@ export class ScrollView extends ScrollViewBase {
4040
private _delegate: UIScrollViewDelegateImpl;
4141

4242
public createNativeView() {
43-
const view = UIScrollView.new();
44-
45-
return view;
43+
return UIScrollView.new();
4644
}
4745

4846
initNativeView() {
@@ -51,20 +49,34 @@ export class ScrollView extends ScrollViewBase {
5149
this._setNativeClipToBounds();
5250
}
5351

54-
_setNativeClipToBounds() {
55-
// Always set clipsToBounds for scroll-view
56-
this.nativeViewProtected.clipsToBounds = true;
52+
disposeNativeView() {
53+
this._delegate = null;
54+
super.disposeNativeView();
5755
}
5856

59-
protected attachNative() {
57+
protected addNativeListener() {
58+
if (!this.nativeViewProtected) {
59+
return;
60+
}
6061
this._delegate = UIScrollViewDelegateImpl.initWithOwner(new WeakRef(this));
6162
this.nativeViewProtected.delegate = this._delegate;
6263
}
6364

64-
protected dettachNative() {
65+
protected removeNativeListener() {
66+
if (!this.nativeViewProtected) {
67+
return;
68+
}
6569
this.nativeViewProtected.delegate = null;
6670
}
6771

72+
_setNativeClipToBounds() {
73+
if (!this.nativeViewProtected) {
74+
return;
75+
}
76+
// Always set clipsToBounds for scroll-view
77+
this.nativeViewProtected.clipsToBounds = true;
78+
}
79+
6880
protected updateScrollBarVisibility(value) {
6981
if (!this.nativeViewProtected) {
7082
return;

‎packages/core/ui/scroll-view/scroll-view-common.ts

Lines changed: 11 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { CoreTypes } from '../../core-types';
99

1010
@CSSType('ScrollView')
1111
export abstract class ScrollViewBase extends ContentView implements ScrollViewDefinition {
12-
private _scrollChangeCount = 0;
12+
private _addedScrollEvent = false;
1313
public static scrollEvent = 'scroll';
1414

1515
public orientation: CoreTypes.OrientationType;
@@ -19,52 +19,27 @@ export abstract class ScrollViewBase extends ContentView implements ScrollViewDe
1919
public addEventListener(arg: string, callback: any, thisArg?: any) {
2020
super.addEventListener(arg, callback, thisArg);
2121

22-
if (arg === ScrollViewBase.scrollEvent) {
23-
this._scrollChangeCount++;
24-
this.attach();
22+
if (arg === ScrollViewBase.scrollEvent && !this._addedScrollEvent) {
23+
this._addedScrollEvent = true;
24+
this.addNativeListener();
2525
}
2626
}
2727

2828
public removeEventListener(arg: string, callback: any, thisArg?: any) {
2929
super.removeEventListener(arg, callback, thisArg);
3030

31-
if (arg === ScrollViewBase.scrollEvent) {
32-
this._scrollChangeCount--;
33-
this.dettach();
31+
if (arg === ScrollViewBase.scrollEvent && this._addedScrollEvent) {
32+
this._addedScrollEvent = false;
33+
this.removeNativeListener();
3434
}
3535
}
3636

37-
@profile
38-
public onLoaded() {
39-
super.onLoaded();
40-
41-
this.attach();
42-
}
43-
44-
public onUnloaded() {
45-
super.onUnloaded();
46-
47-
this.dettach();
48-
}
49-
50-
private attach() {
51-
if (this._scrollChangeCount > 0 && this.isLoaded) {
52-
this.attachNative();
53-
}
54-
}
55-
56-
private dettach() {
57-
if (this._scrollChangeCount === 0 && this.isLoaded) {
58-
this.dettachNative();
59-
}
60-
}
61-
62-
protected attachNative() {
63-
//
37+
protected addNativeListener() {
38+
// implemented per platform
6439
}
6540

66-
protected dettachNative() {
67-
//
41+
protected removeNativeListener() {
42+
// implemented per platform
6843
}
6944

7045
get horizontalOffset(): number {

‎packages/core/ui/search-bar/index.ios.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,23 +82,14 @@ export class SearchBar extends SearchBarBase {
8282
initNativeView() {
8383
super.initNativeView();
8484
this._delegate = UISearchBarDelegateImpl.initWithOwner(new WeakRef(this));
85+
this.nativeViewProtected.delegate = this._delegate;
8586
}
8687

8788
disposeNativeView() {
8889
this._delegate = null;
8990
super.disposeNativeView();
9091
}
9192

92-
public onLoaded() {
93-
super.onLoaded();
94-
this.ios.delegate = this._delegate;
95-
}
96-
97-
public onUnloaded() {
98-
this.ios.delegate = null;
99-
super.onUnloaded();
100-
}
101-
10293
public dismissSoftInput() {
10394
(<UIResponder>this.ios).resignFirstResponder();
10495
}

0 commit comments

Comments
 (0)