From c9e29aa9af8b4b82cd259f8402fa11af4640f1b4 Mon Sep 17 00:00:00 2001 From: Nathan Walker Date: Sun, 15 Jan 2023 19:49:28 -0800 Subject: [PATCH] fix(core): improve loaded/unloaded handling to be stable and consistent (#10170) --- packages/core/data/observable/index.ts | 30 +++++----- packages/core/ui/button/index.ios.ts | 3 +- packages/core/ui/core/view/index.android.ts | 18 +++--- packages/core/ui/core/view/index.ios.ts | 12 ++-- packages/core/ui/list-picker/index.ios.ts | 15 +---- packages/core/ui/scroll-view/index.android.ts | 55 +++++++++++-------- packages/core/ui/scroll-view/index.ios.ts | 28 +++++++--- .../core/ui/scroll-view/scroll-view-common.ts | 47 ++++------------ packages/core/ui/search-bar/index.ios.ts | 11 +--- packages/core/ui/text-field/index.ios.ts | 12 +--- packages/core/ui/text-view/index.ios.ts | 12 +--- packages/core/ui/web-view/index.ios.ts | 44 +++++++-------- 12 files changed, 121 insertions(+), 166 deletions(-) diff --git a/packages/core/data/observable/index.ts b/packages/core/data/observable/index.ts index f27f9d08e..0ad4c63fb 100644 --- a/packages/core/data/observable/index.ts +++ b/packages/core/data/observable/index.ts @@ -291,22 +291,24 @@ export class Observable implements ObservableDefinition { } for (let i = observers.length - 1; i >= 0; i--) { const entry = observers[i]; - if (entry.once) { - observers.splice(i, 1); - } + if (entry) { + if (entry.once) { + observers.splice(i, 1); + } - let returnValue; - if (entry.thisArg) { - returnValue = entry.callback.apply(entry.thisArg, [data]); - } else { - returnValue = entry.callback(data); - } + let returnValue; + if (entry.thisArg) { + returnValue = entry.callback.apply(entry.thisArg, [data]); + } else { + returnValue = entry.callback(data); + } - // This ensures errors thrown inside asynchronous functions do not get swallowed - if (returnValue && returnValue instanceof Promise) { - returnValue.catch((err) => { - console.error(err); - }); + // This ensures errors thrown inside asynchronous functions do not get swallowed + if (returnValue && returnValue instanceof Promise) { + returnValue.catch((err) => { + console.error(err); + }); + } } } } diff --git a/packages/core/ui/button/index.ios.ts b/packages/core/ui/button/index.ios.ts index d27ee2486..c49e81248 100644 --- a/packages/core/ui/button/index.ios.ts +++ b/packages/core/ui/button/index.ios.ts @@ -21,9 +21,8 @@ export class Button extends ButtonBase { public initNativeView(): void { super.initNativeView(); - const nativeView = this.nativeViewProtected; this._tapHandler = TapHandlerImpl.initWithOwner(new WeakRef(this)); - nativeView.addTargetActionForControlEvents(this._tapHandler, 'tap', UIControlEvents.TouchUpInside); + this.nativeViewProtected.addTargetActionForControlEvents(this._tapHandler, 'tap', UIControlEvents.TouchUpInside); } public disposeNativeView(): void { diff --git a/packages/core/ui/core/view/index.android.ts b/packages/core/ui/core/view/index.android.ts index 62304fd51..9b785bac6 100644 --- a/packages/core/ui/core/view/index.android.ts +++ b/packages/core/ui/core/view/index.android.ts @@ -430,14 +430,6 @@ export class View extends ViewCommon { @profile public onUnloaded() { - if (this.touchListenerIsSet) { - this.touchListenerIsSet = false; - if (this.nativeViewProtected) { - this.nativeViewProtected.setOnTouchListener(null); - this.nativeViewProtected.setClickable(this._isClickable); - } - } - this._manager = null; this._rootManager = null; super.onUnloaded(); @@ -474,7 +466,6 @@ export class View extends ViewCommon { public initNativeView(): void { super.initNativeView(); this._isClickable = this.nativeViewProtected.isClickable(); - if (this.needsOnLayoutChangeListener()) { this.setOnLayoutChangeListener(); } @@ -486,7 +477,12 @@ export class View extends ViewCommon { public disposeNativeView(): void { super.disposeNativeView(); - + if (this.touchListenerIsSet) { + this.touchListenerIsSet = false; + if (this.nativeViewProtected) { + this.nativeViewProtected.setOnTouchListener(null); + } + } if (this.layoutChangeListenerIsSet) { this.layoutChangeListenerIsSet = false; this.nativeViewProtected.removeOnLayoutChangeListener(this.layoutChangeListener); @@ -494,7 +490,7 @@ export class View extends ViewCommon { } setOnTouchListener() { - if (!this.nativeViewProtected || !this.hasGestureObservers()) { + if (this.touchListenerIsSet || !this.nativeViewProtected || !this.hasGestureObservers()) { return; } diff --git a/packages/core/ui/core/view/index.ios.ts b/packages/core/ui/core/view/index.ios.ts index 13f312204..83dd35c46 100644 --- a/packages/core/ui/core/view/index.ios.ts +++ b/packages/core/ui/core/view/index.ios.ts @@ -479,12 +479,14 @@ export class View extends ViewCommon implements ViewDefinition { } else { //use CSS & attribute width & height if option is not provided const handler = () => { - const w = (this.width || this.style.width); - const h = (this.height || this.style.height); + if (controller) { + const w = (this.width || this.style.width); + const h = (this.height || this.style.height); - //TODO: only numeric value is supported, percentage value is not supported like Android - if (w > 0 && h > 0) { - controller.preferredContentSize = CGSizeMake(w, h); + //TODO: only numeric value is supported, percentage value is not supported like Android + if (w > 0 && h > 0) { + controller.preferredContentSize = CGSizeMake(w, h); + } } this.off(View.loadedEvent, handler); diff --git a/packages/core/ui/list-picker/index.ios.ts b/packages/core/ui/list-picker/index.ios.ts index f8ff8ea7a..6d6455d24 100644 --- a/packages/core/ui/list-picker/index.ios.ts +++ b/packages/core/ui/list-picker/index.ios.ts @@ -17,9 +17,9 @@ export class ListPicker extends ListPickerBase { initNativeView() { super.initNativeView(); - const nativeView = this.nativeViewProtected; - nativeView.dataSource = this._dataSource = ListPickerDataSource.initWithOwner(new WeakRef(this)); + this.nativeViewProtected.dataSource = this._dataSource = ListPickerDataSource.initWithOwner(new WeakRef(this)); this._delegate = ListPickerDelegateImpl.initWithOwner(new WeakRef(this)); + this.nativeViewProtected.delegate = this._delegate; } public disposeNativeView() { @@ -33,17 +33,6 @@ export class ListPicker extends ListPickerBase { return this.nativeViewProtected; } - @profile - public onLoaded() { - super.onLoaded(); - this.ios.delegate = this._delegate; - } - - public onUnloaded() { - this.ios.delegate = null; - super.onUnloaded(); - } - [selectedIndexProperty.getDefault](): number { return -1; } diff --git a/packages/core/ui/scroll-view/index.android.ts b/packages/core/ui/scroll-view/index.android.ts index 9ec97d965..9d798ed61 100644 --- a/packages/core/ui/scroll-view/index.android.ts +++ b/packages/core/ui/scroll-view/index.android.ts @@ -118,17 +118,10 @@ export class ScrollView extends ScrollViewBase { this.nativeViewProtected.setId(this._androidViewId); } - public _onOrientationChanged() { - if (this.nativeViewProtected) { - const parent = this.parent; - if (parent) { - parent._removeView(this); - parent._addView(this); - } + protected addNativeListener() { + if (!this.nativeViewProtected) { + return; } - } - - protected attachNative() { const that = new WeakRef(this); if (this.orientation === 'vertical') { this.scrollChangeHandler = new androidx.core.widget.NestedScrollView.OnScrollChangeListener({ @@ -144,7 +137,7 @@ export class ScrollView extends ScrollViewBase { } }, }); - this.nativeView.setOnScrollChangeListener(this.scrollChangeHandler); + this.nativeViewProtected.setOnScrollChangeListener(this.scrollChangeHandler); } else { this.handler = new android.view.ViewTreeObserver.OnScrollChangedListener({ onScrollChanged: function () { @@ -158,6 +151,35 @@ export class ScrollView extends ScrollViewBase { } } + protected removeNativeListener() { + if (!this.nativeViewProtected) { + return; + } + if (this.handler) { + this.nativeViewProtected?.getViewTreeObserver().removeOnScrollChangedListener(this.handler); + this.handler = null; + } + if (this.scrollChangeHandler) { + this.nativeView?.setOnScrollChangeListener(null); + this.scrollChangeHandler = null; + } + } + + disposeNativeView() { + super.disposeNativeView(); + this.removeNativeListener(); + } + + public _onOrientationChanged() { + if (this.nativeViewProtected) { + const parent = this.parent; + if (parent) { + parent._removeView(this); + parent._addView(this); + } + } + } + private _lastScrollX = -1; private _lastScrollY = -1; private _onScrollChanged() { @@ -179,17 +201,6 @@ export class ScrollView extends ScrollViewBase { } } } - - protected dettachNative() { - if (this.handler) { - this.nativeViewProtected?.getViewTreeObserver().removeOnScrollChangedListener(this.handler); - this.handler = null; - } - if (this.scrollChangeHandler) { - this.nativeView?.setOnScrollChangeListener(null); - this.scrollChangeHandler = null; - } - } } ScrollView.prototype.recycleNativeView = 'never'; diff --git a/packages/core/ui/scroll-view/index.ios.ts b/packages/core/ui/scroll-view/index.ios.ts index cab5f14c4..9b8c48cd7 100644 --- a/packages/core/ui/scroll-view/index.ios.ts +++ b/packages/core/ui/scroll-view/index.ios.ts @@ -40,9 +40,7 @@ export class ScrollView extends ScrollViewBase { private _delegate: UIScrollViewDelegateImpl; public createNativeView() { - const view = UIScrollView.new(); - - return view; + return UIScrollView.new(); } initNativeView() { @@ -51,20 +49,34 @@ export class ScrollView extends ScrollViewBase { this._setNativeClipToBounds(); } - _setNativeClipToBounds() { - // Always set clipsToBounds for scroll-view - this.nativeViewProtected.clipsToBounds = true; + disposeNativeView() { + this._delegate = null; + super.disposeNativeView(); } - protected attachNative() { + protected addNativeListener() { + if (!this.nativeViewProtected) { + return; + } this._delegate = UIScrollViewDelegateImpl.initWithOwner(new WeakRef(this)); this.nativeViewProtected.delegate = this._delegate; } - protected dettachNative() { + protected removeNativeListener() { + if (!this.nativeViewProtected) { + return; + } this.nativeViewProtected.delegate = null; } + _setNativeClipToBounds() { + if (!this.nativeViewProtected) { + return; + } + // Always set clipsToBounds for scroll-view + this.nativeViewProtected.clipsToBounds = true; + } + protected updateScrollBarVisibility(value) { if (!this.nativeViewProtected) { return; diff --git a/packages/core/ui/scroll-view/scroll-view-common.ts b/packages/core/ui/scroll-view/scroll-view-common.ts index 3e25fb098..bd8e1af32 100644 --- a/packages/core/ui/scroll-view/scroll-view-common.ts +++ b/packages/core/ui/scroll-view/scroll-view-common.ts @@ -9,7 +9,7 @@ import { CoreTypes } from '../../core-types'; @CSSType('ScrollView') export abstract class ScrollViewBase extends ContentView implements ScrollViewDefinition { - private _scrollChangeCount = 0; + private _addedScrollEvent = false; public static scrollEvent = 'scroll'; public orientation: CoreTypes.OrientationType; @@ -19,52 +19,27 @@ export abstract class ScrollViewBase extends ContentView implements ScrollViewDe public addEventListener(arg: string, callback: any, thisArg?: any) { super.addEventListener(arg, callback, thisArg); - if (arg === ScrollViewBase.scrollEvent) { - this._scrollChangeCount++; - this.attach(); + if (arg === ScrollViewBase.scrollEvent && !this._addedScrollEvent) { + this._addedScrollEvent = true; + this.addNativeListener(); } } public removeEventListener(arg: string, callback: any, thisArg?: any) { super.removeEventListener(arg, callback, thisArg); - if (arg === ScrollViewBase.scrollEvent) { - this._scrollChangeCount--; - this.dettach(); + if (arg === ScrollViewBase.scrollEvent && this._addedScrollEvent) { + this._addedScrollEvent = false; + this.removeNativeListener(); } } - @profile - public onLoaded() { - super.onLoaded(); - - this.attach(); + protected addNativeListener() { + // implemented per platform } - public onUnloaded() { - super.onUnloaded(); - - this.dettach(); - } - - private attach() { - if (this._scrollChangeCount > 0 && this.isLoaded) { - this.attachNative(); - } - } - - private dettach() { - if (this._scrollChangeCount === 0 && this.isLoaded) { - this.dettachNative(); - } - } - - protected attachNative() { - // - } - - protected dettachNative() { - // + protected removeNativeListener() { + // implemented per platform } get horizontalOffset(): number { diff --git a/packages/core/ui/search-bar/index.ios.ts b/packages/core/ui/search-bar/index.ios.ts index 429a47d52..051d4d8b7 100644 --- a/packages/core/ui/search-bar/index.ios.ts +++ b/packages/core/ui/search-bar/index.ios.ts @@ -82,6 +82,7 @@ export class SearchBar extends SearchBarBase { initNativeView() { super.initNativeView(); this._delegate = UISearchBarDelegateImpl.initWithOwner(new WeakRef(this)); + this.nativeViewProtected.delegate = this._delegate; } disposeNativeView() { @@ -89,16 +90,6 @@ export class SearchBar extends SearchBarBase { super.disposeNativeView(); } - public onLoaded() { - super.onLoaded(); - this.ios.delegate = this._delegate; - } - - public onUnloaded() { - this.ios.delegate = null; - super.onUnloaded(); - } - public dismissSoftInput() { (this.ios).resignFirstResponder(); } diff --git a/packages/core/ui/text-field/index.ios.ts b/packages/core/ui/text-field/index.ios.ts index 5d08897b2..57ec45248 100644 --- a/packages/core/ui/text-field/index.ios.ts +++ b/packages/core/ui/text-field/index.ios.ts @@ -123,6 +123,7 @@ export class TextField extends TextFieldBase { initNativeView() { super.initNativeView(); this._delegate = UITextFieldDelegateImpl.initWithOwner(new WeakRef(this)); + this.nativeViewProtected.delegate = this._delegate; } disposeNativeView() { @@ -130,17 +131,6 @@ export class TextField extends TextFieldBase { super.disposeNativeView(); } - @profile - public onLoaded() { - super.onLoaded(); - this.ios.delegate = this._delegate; - } - - public onUnloaded() { - this.ios.delegate = null; - super.onUnloaded(); - } - // @ts-ignore get ios(): UITextField { return this.nativeViewProtected; diff --git a/packages/core/ui/text-view/index.ios.ts b/packages/core/ui/text-view/index.ios.ts index ff4fc5f96..7b44166d6 100644 --- a/packages/core/ui/text-view/index.ios.ts +++ b/packages/core/ui/text-view/index.ios.ts @@ -106,6 +106,7 @@ export class TextView extends TextViewBaseCommon { initNativeView() { super.initNativeView(); this._delegate = UITextViewDelegateImpl.initWithOwner(new WeakRef(this)); + this.nativeTextViewProtected.delegate = this._delegate; } disposeNativeView() { @@ -113,17 +114,6 @@ export class TextView extends TextViewBaseCommon { super.disposeNativeView(); } - @profile - public onLoaded() { - super.onLoaded(); - this.nativeTextViewProtected.delegate = this._delegate; - } - - public onUnloaded() { - this.nativeTextViewProtected.delegate = null; - super.onUnloaded(); - } - // @ts-ignore get ios(): UITextView { return this.nativeViewProtected; diff --git a/packages/core/ui/web-view/index.ios.ts b/packages/core/ui/web-view/index.ios.ts index d3033f128..5c7baf75a 100644 --- a/packages/core/ui/web-view/index.ios.ts +++ b/packages/core/ui/web-view/index.ios.ts @@ -202,18 +202,16 @@ export class WebView extends WebViewBase { this._delegate = WKNavigationDelegateImpl.initWithOwner(new WeakRef(this)); this._scrollDelegate = UIScrollViewDelegateImpl.initWithOwner(new WeakRef(this)); this._uiDelegate = WKUIDelegateImpl.initWithOwner(new WeakRef(this)); - this.ios.navigationDelegate = this._delegate; - this.ios.scrollView.delegate = this._scrollDelegate; - this.ios.UIDelegate = this._uiDelegate; + this.nativeViewProtected.navigationDelegate = this._delegate; + this.nativeViewProtected.scrollView.delegate = this._scrollDelegate; + this.nativeViewProtected.UIDelegate = this._uiDelegate; } - @profile - public onLoaded() { - super.onLoaded(); - } - - public onUnloaded() { - super.onUnloaded(); + disposeNativeView() { + super.disposeNativeView(); + this._delegate = null; + this._scrollDelegate = null; + this._uiDelegate = null; } // @ts-ignore @@ -222,48 +220,48 @@ export class WebView extends WebViewBase { } public stopLoading() { - this.ios.stopLoading(); + this.nativeViewProtected.stopLoading(); } public _loadUrl(src: string) { if (src.startsWith('file:///')) { const cachePath = src.substring(0, src.lastIndexOf('/')); - this.ios.loadFileURLAllowingReadAccessToURL(NSURL.URLWithString(src), NSURL.URLWithString(cachePath)); + this.nativeViewProtected.loadFileURLAllowingReadAccessToURL(NSURL.URLWithString(src), NSURL.URLWithString(cachePath)); } else { - this.ios.loadRequest(NSURLRequest.requestWithURL(NSURL.URLWithString(src))); + this.nativeViewProtected.loadRequest(NSURLRequest.requestWithURL(NSURL.URLWithString(src))); } } public _loadData(content: string) { - this.ios.loadHTMLStringBaseURL(content, NSURL.alloc().initWithString(`file:///${knownFolders.currentApp().path}/`)); + this.nativeViewProtected.loadHTMLStringBaseURL(content, NSURL.alloc().initWithString(`file:///${knownFolders.currentApp().path}/`)); } get canGoBack(): boolean { - return this.ios.canGoBack; + return this.nativeViewProtected.canGoBack; } get canGoForward(): boolean { - return this.ios.canGoForward; + return this.nativeViewProtected.canGoForward; } public goBack() { - this.ios.goBack(); + this.nativeViewProtected.goBack(); } public goForward() { - this.ios.goForward(); + this.nativeViewProtected.goForward(); } public reload() { - this.ios.reload(); + this.nativeViewProtected.reload(); } [disableZoomProperty.setNative](value: boolean) { if (!value && typeof this._minimumZoomScale === 'number' && typeof this._maximumZoomScale === 'number' && typeof this._zoomScale === 'number') { - if (this.ios.scrollView) { - this.ios.scrollView.minimumZoomScale = this._minimumZoomScale; - this.ios.scrollView.maximumZoomScale = this._maximumZoomScale; - this.ios.scrollView.zoomScale = this._zoomScale; + if (this.nativeViewProtected?.scrollView) { + this.nativeViewProtected.scrollView.minimumZoomScale = this._minimumZoomScale; + this.nativeViewProtected.scrollView.maximumZoomScale = this._maximumZoomScale; + this.nativeViewProtected.scrollView.zoomScale = this._zoomScale; this._minimumZoomScale = undefined; this._maximumZoomScale = undefined; this._zoomScale = undefined;