Skip to content

Commit 7afb508

Browse files
[iOS] ScrollView: Complete element scroll requests inside collapsed branches (#37837)
<!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change Ported the changes from PR #37409 to the inflight/Candidate branch.
1 parent 1edefae commit 7afb508

5 files changed

Lines changed: 622 additions & 55 deletions

File tree

src/Controls/src/Core/ScrollView/ScrollView.cs

Lines changed: 116 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -43,27 +43,75 @@ public Rect LayoutAreaOverride
4343
ScrollToRequestedEventArgs _pendingScrollToRequested;
4444
bool _replayPendingScrollToRequestedEvent;
4545

46+
// A parked element request lives exactly as long as the task the caller is awaiting.
47+
// It leaves the park in one of two ways, and no other: the arrange arrives and the
48+
// request is replayed against real geometry (the task completes with the scroll),
49+
// or there is nothing left that could ever satisfy it and the request is dropped
50+
// (the task completes without a scroll). The latter happens when the view's
51+
// lifecycle ends — its handler goes away or it is removed from the tree — and when
52+
// the target is orphaned from this ScrollView's content (see IsElementTargetOrphaned).
53+
// Nothing releases the task while the request is still parked, so a completed await
54+
// never scrolls later; and nothing but one of those terminal conditions drops the
55+
// request, so a view that is merely hidden (a collapsed branch, an unselected tab)
56+
// still scrolls to the element when it is eventually arranged. A view that stays
57+
// attached and is never arranged keeps the task pending — that is the contract, not
58+
// a leak: the task completes when the scroll happens or nothing can make it happen.
4659
private protected override void OnHandlerChangedCore()
4760
{
4861
base.OnHandlerChangedCore();
4962

5063
if (Handler is null)
5164
{
5265
// The handler went away with a request still queued, so nothing will ever
53-
// dispatch it. Release the caller rather than leaving its task pending
54-
// forever; Core does the same for its own pending request on disconnect.
55-
if (_pendingScrollToRequested is not null)
56-
{
57-
_pendingScrollToRequested = null;
58-
SendScrollFinished();
59-
}
60-
66+
// dispatch it; Core does the same for its own pending request on disconnect
67+
DropPendingScrollToRequest();
6168
return;
6269
}
6370

6471
DispatchPendingScrollToRequest();
6572
}
6673

74+
private protected override void OnParentChangedCore()
75+
{
76+
base.OnParentChangedCore();
77+
78+
// Removed from the tree — the handler is not necessarily disconnected by the
79+
// removal, so this is the other lifecycle end that drops a parked request. This
80+
// includes the transient removal of a reparent: a request made against a tree
81+
// position that no longer exists is cancelled and its task completes at the
82+
// removal, and it is not carried over to the new parent (its arrange does not
83+
// replay it). The drop is deliberately not deferred to "see whether a re-add
84+
// follows": that would put a window between the drop and the completion, in
85+
// which a newer request could be released in the old one's place. A caller that
86+
// moves a ScrollView with a pending element scroll re-requests it in the new
87+
// location.
88+
if (RealParent is null)
89+
{
90+
DropPendingScrollToRequest();
91+
}
92+
}
93+
94+
void DropPendingScrollToRequest()
95+
{
96+
if (_pendingScrollToRequested is null)
97+
{
98+
return;
99+
}
100+
101+
_pendingScrollToRequested = null;
102+
// A stale replay flag from a pre-handler park must not carry over to a later request
103+
_replayPendingScrollToRequestedEvent = false;
104+
105+
// Complete inline, at the moment the request is dropped. Deferring the completion
106+
// would open a window in which a newer ScrollToAsync could swap the completion
107+
// source, so a deferred completion would release the wrong task and orphan this
108+
// one. Completing here binds the release to the request being dropped by
109+
// construction. This is also the convention already in place: the handler-detach
110+
// drop and Core's own DisconnectHandler both complete the task inline from their
111+
// lifecycle hooks.
112+
SendScrollFinished();
113+
}
114+
67115
void DispatchPendingScrollToRequest()
68116
{
69117
if (Handler is null || _pendingScrollToRequested is not { } pending)
@@ -73,33 +121,59 @@ void DispatchPendingScrollToRequest()
73121

74122
if (pending.Mode == ScrollToMode.Element)
75123
{
76-
if (!IsElementTargetGeometryReady())
124+
if (IsElementTargetOrphaned(pending.Element))
125+
{
126+
// The target no longer hangs off this ScrollView's content (the content
127+
// was replaced or removed under a parked request), so no arrange can ever
128+
// give it a position: nothing is left to wait for. Terminal, like a
129+
// lifecycle end.
130+
DropPendingScrollToRequest();
131+
return;
132+
}
133+
134+
if (!IsElementTargetGeometryReady(pending.Element))
77135
{
78136
// The request has to wait; OnSizeAllocated and ContentSizeChanged retry it.
79137
return;
80138
}
81139

82-
// Those callbacks run while the pass that produced the sizes is still arranging
83-
// children, so resolve on the next tick, once positions are final. Posting on
84-
// every retry is deliberate: SendPendingScrollToRequest is a no-op once the
85-
// request has been sent or superseded, so a dropped callback cannot wedge the
86-
// request the way an "already queued" flag would.
140+
// Those callbacks run while the pass that produced the sizes is still
141+
// arranging children, so resolve on the next tick, once positions are final.
142+
// Posting on every retry is deliberate: SendPendingScrollToRequest is a no-op
143+
// once the request has been sent or superseded, so a dropped callback cannot
144+
// wedge the request the way an "already queued" flag would.
87145
Dispatcher.Dispatch(SendPendingScrollToRequest);
88146
return;
89147
}
90148

91149
SendPendingScrollToRequest();
92150
}
93151

94-
// An element target is resolved against this ScrollView's geometry and the element's
95-
// position inside the arranged content. Before the first layout pass Width/Height are
96-
// still -1 (the never-arranged sentinel, for the content too), so a target computed
97-
// then is garbage. The content check must be "not yet arranged" rather than "arranged
98-
// to nothing": content can legitimately arrange to a zero size (a collapsed container),
99-
// and that raises no further callbacks — gating on the size would hang the caller's
100-
// task forever, while dispatching just clamps the target to the origin.
101-
bool IsElementTargetGeometryReady() =>
102-
Width >= 0 && Height >= 0 && Content is not ({ Width: < 0 } or { Height: < 0 });
152+
// An element target is resolved against the geometry it actually depends on. Before
153+
// the first layout pass Width/Height are still -1 (the never-arranged sentinel), so a
154+
// target computed then is garbage. The ScrollView itself is a valid target and needs
155+
// only its own geometry; any other target sits inside the content, so its position
156+
// is meaningful only once the content has been arranged too. That content check must
157+
// be "not yet arranged" rather than "arranged to nothing": content can legitimately
158+
// arrange to a zero size (a collapsed container), and that raises no further
159+
// callbacks — gating on the size would hang the caller's task forever, while
160+
// dispatching just clamps the target to the origin.
161+
bool IsElementTargetGeometryReady(Element target)
162+
{
163+
if (Width < 0 || Height < 0)
164+
{
165+
return false;
166+
}
167+
168+
return target == this || Content is { Width: >= 0, Height: >= 0 };
169+
}
170+
171+
// A target validated at request time as belonging to this ScrollView can stop
172+
// belonging to it while parked: the content it hung off was replaced or removed. Its
173+
// coordinates then no longer relate to this ScrollView and no arrange of this
174+
// ScrollView can change that, so waiting would be waiting for nothing.
175+
bool IsElementTargetOrphaned(Element target) =>
176+
target != this && !CheckElementBelongsToScrollViewer(target);
103177

104178
void SendPendingScrollToRequest()
105179
{
@@ -120,7 +194,18 @@ void SendPendingScrollToRequest()
120194
if (_replayPendingScrollToRequestedEvent)
121195
{
122196
_replayPendingScrollToRequestedEvent = false;
197+
198+
// A subscriber may issue a new ScrollToAsync from inside the event (a
199+
// compatibility renderer scrolling, say). That newer request wins: it has
200+
// already been sent, and sending the stale replay after it would land the
201+
// scroll on the old target. Every request creates a fresh completion source,
202+
// so a changed source is the exact signal that one was made.
203+
var replayed = _scrollCompletionSource;
123204
ScrollToRequested?.Invoke(this, pending);
205+
if (!ReferenceEquals(_scrollCompletionSource, replayed))
206+
{
207+
return;
208+
}
124209
}
125210

126211
Handler.Invoke(nameof(IScrollView.RequestScrollTo), ConvertRequestMode(pending).ToRequest());
@@ -279,6 +364,11 @@ public View Content
279364

280365
OnPropertyChanged();
281366
Handler?.UpdateValue(nameof(Content));
367+
368+
// A parked element request may have just been orphaned (its target hung off
369+
// the old content) — resolve that now rather than at the next arrange, which
370+
// may not come if this ScrollView is already laid out
371+
DispatchPendingScrollToRequest();
282372
}
283373
}
284374

@@ -527,12 +617,14 @@ void OnScrollToRequested(ScrollToRequestedEventArgs e)
527617
_pendingScrollToRequested = e;
528618
_replayPendingScrollToRequestedEvent = true;
529619
}
530-
else if (e.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady())
620+
else if (e.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady(e.Element))
531621
{
532622
// The handler exists but layout has not run yet (e.g. ScrollToAsync from
533623
// OnAppearing): resolving the element target now would compute against the -1
534624
// never-arranged sentinels. Park it for the layout callbacks instead — the
535625
// subscribers were already notified above, so the replay must not re-raise.
626+
// It stays parked until the arrange arrives or the view's lifecycle ends
627+
// (see OnHandlerChangedCore); a hidden view scrolls once it is shown.
536628
_pendingScrollToRequested = e;
537629
_replayPendingScrollToRequestedEvent = false;
538630
}

0 commit comments

Comments
 (0)