From f683edcae3b4405316149980b78a28ea5f49291f Mon Sep 17 00:00:00 2001 From: Morten Nielsen Date: Fri, 18 Sep 2026 17:00:16 -0700 Subject: [PATCH] Don't wait for layout updated before scrolling to bottom --- .../Chat/ReactorItemsViewScrollController.cs | 85 +++++++------------ .../ChatTimelinePresentationTests.cs | 41 ++++++++- 2 files changed, 72 insertions(+), 54 deletions(-) diff --git a/src/OpenClaw.Tray.WinUI/Chat/ReactorItemsViewScrollController.cs b/src/OpenClaw.Tray.WinUI/Chat/ReactorItemsViewScrollController.cs index c861cb14a..1c5107763 100644 --- a/src/OpenClaw.Tray.WinUI/Chat/ReactorItemsViewScrollController.cs +++ b/src/OpenClaw.Tray.WinUI/Chat/ReactorItemsViewScrollController.cs @@ -94,7 +94,6 @@ public V1UnmountDisposition Unmount(UnmountContext context, ItemsViewVerticalScr private int _itemCount; private int _version; private bool _valid; - private bool _awaitingLayout; private WinUIScrollView? _awaitingScrollView; private WinUIScrollView? _scrollView; private bool _following; @@ -115,7 +114,7 @@ public void Request(int tailIndex, int itemCount, string requestKey, string? dis _requestKey = requestKey; _version++; - DetachLayout(); + StopWaitingForScrollView(); _tailIndex = tailIndex; _itemCount = itemCount; _displayedTailKey = displayedTailKey; @@ -124,8 +123,7 @@ public void Request(int tailIndex, int itemCount, string requestKey, string? dis if (!_valid) return; - if (itemsView.IsLoaded) - AwaitLayout(); + TryPositionInitialTail(); } public void UpdateTail(int tailIndex, int itemCount, string? displayedTailKey) @@ -149,64 +147,54 @@ public void UpdateTail(int tailIndex, int itemCount, string? displayedTailKey) private void OnLoaded(object sender, RoutedEventArgs args) { - if (_valid) - AwaitLayout(); + TryPositionInitialTail(); } - private void AwaitLayout() + private void TryPositionInitialTail() { - if (_disposed || !_valid || !itemsView.IsLoaded || _awaitingLayout) + if (_disposed || !_valid || !itemsView.IsLoaded + || itemsView.ScrollView is not { } scrollView) return; - if (itemsView.ScrollView is { IsLoaded: false } scrollView) + if (!scrollView.IsLoaded) { - _awaitingScrollView = scrollView; - scrollView.Loaded += OnScrollViewLoaded; - return; - } - - _awaitingLayout = true; - itemsView.LayoutUpdated += OnLayoutUpdated; - } - - private void OnScrollViewLoaded(object sender, RoutedEventArgs args) - { - if (sender is WinUIScrollView scrollView) - scrollView.Loaded -= OnScrollViewLoaded; - - _awaitingScrollView = null; - AwaitLayout(); - } - - private void OnLayoutUpdated(object? sender, object args) - { - DetachLayout(); - if (itemsView.ScrollView is not { IsLoaded: true }) - { - AwaitLayout(); + if (!ReferenceEquals(_awaitingScrollView, scrollView)) + { + StopWaitingForScrollView(); + _awaitingScrollView = scrollView; + scrollView.Loaded += OnScrollViewLoaded; + } return; } + StopWaitingForScrollView(); var version = _version; if (!TailNavigationPolicy.TryCapture(_tailIndex, _displayedTailKey, _itemCount, out var request)) return; + // Leave Reactor reconciliation before native navigation. The bring request handles layout itself. itemsView.DispatcherQueue.TryEnqueue(() => { - if (_disposed || !_valid || !itemsView.IsLoaded || version != _version - || itemsView.ScrollView is not { IsLoaded: true }) + if (_disposed || !_valid || version != _version) + return; + + if (!itemsView.IsLoaded || itemsView.ScrollView is not { IsLoaded: true }) { - if (!_disposed && _valid) - AwaitLayout(); + TryPositionInitialTail(); return; } AttachScrollView(); - if (!StartTailRequest(request) && !_disposed && _valid) - AwaitLayout(); + StartTailRequest(request); }); } + private void OnScrollViewLoaded(object sender, RoutedEventArgs args) + { + StopWaitingForScrollView(); + TryPositionInitialTail(); + } + private void AttachScrollView() { var nextScrollView = itemsView.ScrollView; @@ -251,10 +239,10 @@ private void QueueTailRequest(int version, TailNavigationRequest request) } } - private bool StartTailRequest(TailNavigationRequest request) + private void StartTailRequest(TailNavigationRequest request) { if (itemsView.ScrollView is not { IsLoaded: true }) - return false; + return; if (!TailNavigationPolicy.CanExecute( request, @@ -262,7 +250,7 @@ private bool StartTailRequest(TailNavigationRequest request) _displayedTailKey, _itemCount)) { - return false; + return; } _following = true; @@ -271,7 +259,6 @@ private bool StartTailRequest(TailNavigationRequest request) AnimationDesired = false, VerticalAlignmentRatio = 1.0, }); - return true; } private static bool IsNearBottom(WinUIScrollView scrollView) => @@ -281,23 +268,17 @@ private void OnUnloaded(object sender, RoutedEventArgs args) { _version++; _tailNavigationQueue.Clear(); - DetachLayout(); + StopWaitingForScrollView(); DetachScrollView(); } - private void DetachLayout() + private void StopWaitingForScrollView() { if (_awaitingScrollView is { } scrollView) { scrollView.Loaded -= OnScrollViewLoaded; _awaitingScrollView = null; } - - if (_awaitingLayout) - { - itemsView.LayoutUpdated -= OnLayoutUpdated; - _awaitingLayout = false; - } } private void DetachScrollView() @@ -319,7 +300,7 @@ public void Dispose() _disposed = true; _version++; _tailNavigationQueue.Clear(); - DetachLayout(); + StopWaitingForScrollView(); DetachScrollView(); itemsView.Loaded -= OnLoaded; itemsView.Unloaded -= OnUnloaded; diff --git a/tests/OpenClaw.Tray.Tests/ChatTimelinePresentationTests.cs b/tests/OpenClaw.Tray.Tests/ChatTimelinePresentationTests.cs index e258ba47a..f70650c0d 100644 --- a/tests/OpenClaw.Tray.Tests/ChatTimelinePresentationTests.cs +++ b/tests/OpenClaw.Tray.Tests/ChatTimelinePresentationTests.cs @@ -64,7 +64,7 @@ public void ReactorTimeline_UsesStableBottomAnchoringAndDiscreteTailRequests() "ReactorItemsViewScrollController.cs")); Assert.Contains("itemsView.Loaded += OnLoaded", binding); - Assert.Contains("itemsView.LayoutUpdated += OnLayoutUpdated", binding); + Assert.DoesNotContain("LayoutUpdated", binding); Assert.Contains("itemsView.DispatcherQueue.TryEnqueue", binding); Assert.Contains("itemsView.StartBringItemIntoView(", binding); Assert.Contains("VerticalAlignmentRatio = 1.0", binding); @@ -79,7 +79,7 @@ public void ReactorTimeline_UsesStableBottomAnchoringAndDiscreteTailRequests() Assert.Contains("TailNavigationPolicy.CanExecute(", binding); Assert.Contains("itemsView.Unloaded += OnUnloaded", binding); Assert.Contains("itemsView.Loaded -= OnLoaded", binding); - Assert.Contains("itemsView.LayoutUpdated -= OnLayoutUpdated", binding); + Assert.Contains("scrollView.Loaded -= OnScrollViewLoaded", binding); Assert.DoesNotContain("ChangeView", binding); Assert.DoesNotContain("UpdateLayout", binding); Assert.DoesNotContain("TailSettle", binding); @@ -98,6 +98,43 @@ public void ReactorTimeline_UsesStableBottomAnchoringAndDiscreteTailRequests() Assert.DoesNotContain("StartBringItemIntoView", viewChanged); } + [Fact] + public void ReactorTimeline_InitialTailWaitsForLoadWithoutLayoutSubscription() + { + var binding = File.ReadAllText(Path.Combine( + TestRepositoryPaths.GetRepositoryRoot(), + "src", + "OpenClaw.Tray.WinUI", + "Chat", + "ReactorItemsViewScrollController.cs")); + + var requestStart = binding.IndexOf("public void Request(", StringComparison.Ordinal); + var updateStart = binding.IndexOf("public void UpdateTail(", requestStart, StringComparison.Ordinal); + Assert.Contains("TryPositionInitialTail();", binding[requestStart..updateStart]); + + var loadStart = binding.IndexOf("private void OnLoaded(", StringComparison.Ordinal); + var attachStart = binding.IndexOf("private void AttachScrollView(", loadStart, StringComparison.Ordinal); + var initialPositioning = binding[loadStart..attachStart]; + Assert.Contains("_disposed || !_valid || !itemsView.IsLoaded", initialPositioning); + Assert.Contains("if (!scrollView.IsLoaded)", initialPositioning); + Assert.Contains("!ReferenceEquals(_awaitingScrollView, scrollView)", initialPositioning); + Assert.Contains("scrollView.Loaded += OnScrollViewLoaded", initialPositioning); + Assert.Contains("StopWaitingForScrollView();", initialPositioning); + Assert.Contains("AttachScrollView();", initialPositioning); + Assert.Contains("TailNavigationPolicy.TryCapture(", initialPositioning); + Assert.Contains("StartTailRequest(request);", initialPositioning); + Assert.Contains("itemsView.DispatcherQueue.TryEnqueue", initialPositioning); + Assert.Contains("var version = _version;", initialPositioning); + Assert.Contains("_disposed || !_valid || version != _version", initialPositioning); + Assert.DoesNotContain("AwaitLayout", binding); + + var unloadStart = binding.IndexOf("private void OnUnloaded(", StringComparison.Ordinal); + var stopWaitingStart = binding.IndexOf("private void StopWaitingForScrollView(", unloadStart, StringComparison.Ordinal); + Assert.Contains("StopWaitingForScrollView();", binding[unloadStart..stopWaitingStart]); + var disposeStart = binding.IndexOf("public void Dispose()", StringComparison.Ordinal); + Assert.Contains("StopWaitingForScrollView();", binding[disposeStart..]); + } + [Fact] public void TailNavigationPolicy_RejectsQueuedRequestAfterValidTailBecomesEmpty() {