Skip to content

Commit 75821ea

Browse files
NathanWalkerNathanWalker
authored
fix(core): ScrollView event wiring sequencing improvement (#10178)
1 parent 7993de5 commit 75821ea

6 files changed

Lines changed: 106 additions & 74 deletions

File tree

‎apps/toolbox/src/main-page.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
<Button text="labels" tap="{{ viewDemo }}" class="btn btn-primary btn-view-demo" />
1717
<Button text="list-page" tap="{{ viewDemo }}" class="btn btn-primary btn-view-demo" />
1818
<Button text="root-layout" tap="{{ viewDemo }}" class="btn btn-primary btn-view-demo" />
19+
<Button text="scroll-view" tap="{{ viewDemo }}" class="btn btn-primary btn-view-demo" />
1920
<Button text="switch" tap="{{ viewDemo }}" class="btn btn-primary btn-view-demo" />
2021
<Button text="touch-animations" tap="{{ viewDemo }}" class="btn btn-primary btn-view-demo" />
2122
<Button text="vector-image" tap="{{ viewDemo }}" class="btn btn-primary btn-view-demo" />

‎apps/toolbox/src/pages/scroll-view.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { Observable, EventData, Page, ScrollEventData } from '@nativescript/core';
2+
3+
let page: Page;
4+
5+
export function navigatingTo(args: EventData) {
6+
page = <Page>args.object;
7+
page.bindingContext = new ScrollViewModel();
8+
}
9+
10+
export class ScrollViewModel extends Observable {
11+
longText = `The standard Lorem Ipsum passage, used since the 1500s
12+
"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum."
13+
14+
Section 1.10.32 of "de Finibus Bonorum et Malorum", written by Cicero in 45 BC
15+
"Sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium, totam rem aperiam, eaque ipsa quae ab illo inventore veritatis et quasi architecto beatae vitae dicta sunt explicabo. Nemo enim ipsam voluptatem quia voluptas sit aspernatur aut odit aut fugit, sed quia consequuntur magni dolores eos qui ratione voluptatem sequi nesciunt. Neque porro quisquam est, qui dolorem ipsum quia dolor sit amet, consectetur, adipisci velit, sed quia non numquam eius modi tempora incidunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim ad minima veniam, quis nostrum exercitationem ullam corporis suscipit laboriosam, nisi ut aliquid ex ea commodi consequatur? Quis autem vel eum iure reprehenderit qui in ea voluptate velit esse quam nihil molestiae consequatur, vel illum qui dolorem eum fugiat quo voluptas nulla pariatur?"
16+
17+
1914 translation by H. Rackham
18+
"But I must explain to you how all this mistaken idea of denouncing pleasure and praising pain was born and I will give you a complete account of the system, and expound the actual teachings of the great explorer of the truth, the master-builder of human happiness. No one rejects, dislikes, or avoids pleasure itself, because it is pleasure, but because those who do not know how to pursue pleasure rationally encounter consequences that are extremely painful. Nor again is there anyone who loves or pursues or desires to obtain pain of itself, because it is pain, but because occasionally circumstances occur in which toil and pain can procure him some great pleasure. To take a trivial example, which of us ever undertakes laborious physical exercise, except to obtain some advantage from it? But who has any right to find fault with a man who chooses to enjoy a pleasure that has no annoying consequences, or one who avoids a pain that produces no resultant pleasure?"
19+
20+
Section 1.10.33 of "de Finibus Bonorum et Malorum", written by Cicero in 45 BC
21+
"At vero eos et accusamus et iusto odio dignissimos ducimus qui blanditiis praesentium voluptatum deleniti atque corrupti quos dolores et quas molestias excepturi sint occaecati cupiditate non provident, similique sunt in culpa qui officia deserunt mollitia animi, id est laborum et dolorum fuga. Et harum quidem rerum facilis est et expedita distinctio. Nam libero tempore, cum soluta nobis est eligendi optio cumque nihil impedit quo minus id quod maxime placeat facere possimus, omnis voluptas assumenda est, omnis dolor repellendus. Temporibus autem quibusdam et aut officiis debitis aut rerum necessitatibus saepe eveniet ut et voluptates repudiandae sint et molestiae non recusandae. Itaque earum rerum hic tenetur a sapiente delectus, ut aut reiciendis voluptatibus maiores alias consequatur aut perferendis doloribus asperiores repellat."`;
22+
23+
scroll(args: ScrollEventData) {
24+
console.log('scroll:', args.scrollY);
25+
}
26+
}

‎apps/toolbox/src/pages/scroll-view.xml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<Page xmlns="http://schemas.nativescript.org/tns.xsd" navigatingTo="navigatingTo" class="page">
2+
<Page.actionBar>
3+
<ActionBar title="ScrollView" icon="" class="action-bar">
4+
</ActionBar>
5+
</Page.actionBar>
6+
7+
<ScrollView scroll="{{ scroll }}">
8+
<StackLayout>
9+
10+
<Label text="{{longText}}" textWrap="true" />
11+
12+
</StackLayout>
13+
</ScrollView>
14+
</Page>

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

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

121-
protected addNativeListener() {
122-
if (!this.nativeViewProtected) {
123-
return;
121+
public _onOrientationChanged() {
122+
if (this.nativeViewProtected) {
123+
const parent = this.parent;
124+
if (parent) {
125+
parent._removeView(this);
126+
parent._addView(this);
127+
}
124128
}
125-
const that = new WeakRef(this);
126-
if (this.orientation === 'vertical') {
127-
this.scrollChangeHandler = new androidx.core.widget.NestedScrollView.OnScrollChangeListener({
128-
onScrollChange(view, scrollX, scrollY) {
129-
const owner: ScrollView = that?.get();
130-
if (owner) {
131-
owner.notify({
132-
object: owner,
133-
eventName: ScrollView.scrollEvent,
134-
scrollX: layout.toDeviceIndependentPixels(scrollX),
135-
scrollY: layout.toDeviceIndependentPixels(scrollY),
136-
});
137-
}
138-
},
139-
});
140-
this.nativeViewProtected.setOnScrollChangeListener(this.scrollChangeHandler);
141-
} else {
129+
}
130+
131+
protected attachNative() {
132+
if (!this.handler) {
133+
const that = new WeakRef(this);
142134
this.handler = new android.view.ViewTreeObserver.OnScrollChangedListener({
143135
onScrollChanged: function () {
144-
const owner: ScrollView = that?.get();
136+
const owner: ScrollView = that.get();
145137
if (owner) {
146138
owner._onScrollChanged();
147139
}
148140
},
149141
});
150-
this.nativeViewProtected.getViewTreeObserver().addOnScrollChangedListener(this.handler);
151-
}
152-
}
153142

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-
this.removeNativeListener();
170-
super.disposeNativeView();
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-
}
143+
this.nativeViewProtected.getViewTreeObserver().addOnScrollChangedListener(this.handler);
180144
}
181145
}
182146

@@ -201,6 +165,11 @@ export class ScrollView extends ScrollViewBase {
201165
}
202166
}
203167
}
168+
169+
protected dettachNative() {
170+
this.nativeViewProtected.getViewTreeObserver().removeOnScrollChangedListener(this.handler);
171+
this.handler = null;
172+
}
204173
}
205174

206175
ScrollView.prototype.recycleNativeView = 'never';

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

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,32 +50,30 @@ export class ScrollView extends ScrollViewBase {
5050
}
5151

5252
disposeNativeView() {
53-
this.removeNativeListener();
53+
this.dettachNative();
5454
this._delegate = null;
5555
super.disposeNativeView();
5656
}
5757

58-
protected addNativeListener() {
58+
_setNativeClipToBounds() {
5959
if (!this.nativeViewProtected) {
6060
return;
6161
}
62-
this._delegate = UIScrollViewDelegateImpl.initWithOwner(new WeakRef(this));
63-
this.nativeViewProtected.delegate = this._delegate;
62+
// Always set clipsToBounds for scroll-view
63+
this.nativeViewProtected.clipsToBounds = true;
6464
}
6565

66-
protected removeNativeListener() {
67-
if (!this.nativeViewProtected) {
68-
return;
66+
protected attachNative() {
67+
if (!this._delegate) {
68+
this._delegate = UIScrollViewDelegateImpl.initWithOwner(new WeakRef(this));
69+
this.nativeViewProtected.delegate = this._delegate;
6970
}
70-
this.nativeViewProtected.delegate = null;
7171
}
7272

73-
_setNativeClipToBounds() {
74-
if (!this.nativeViewProtected) {
75-
return;
73+
protected dettachNative() {
74+
if (this.nativeViewProtected) {
75+
this.nativeViewProtected.delegate = null;
7676
}
77-
// Always set clipsToBounds for scroll-view
78-
this.nativeViewProtected.clipsToBounds = true;
7977
}
8078

8179
protected updateScrollBarVisibility(value) {

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

Lines changed: 35 additions & 11 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 _addedScrollEvent = false;
12+
private _scrollChangeCount = 0;
1313
public static scrollEvent = 'scroll';
1414

1515
public orientation: CoreTypes.OrientationType;
@@ -19,27 +19,51 @@ 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 && !this._addedScrollEvent) {
23-
this._addedScrollEvent = true;
24-
this.addNativeListener();
22+
if (arg === ScrollViewBase.scrollEvent) {
23+
this._scrollChangeCount++;
24+
this.attach();
2525
}
2626
}
2727

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

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

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

41-
protected removeNativeListener() {
42-
// implemented per platform
65+
protected dettachNative() {
66+
//
4367
}
4468

4569
get horizontalOffset(): number {

0 commit comments

Comments
 (0)