From cd23678775509ddd83e789540d6731e0af54a2a2 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 Oct 2026 20:35:41 +0100 Subject: [PATCH] Re-render the commits view in the same UI-thread hop as its model update The commits refresh updated the model and the selection in one hop to the UI thread, and re-rendered the view in a second one. The scroll behavior of the re-render was fixed when the refresh started. The next commit makes the scroll behavior depend on how the refresh changed the selection. Do both in one hop, so that the re-render can see that. Co-Authored-By: Claude Opus 5.5 --- pkg/gui/controllers/helpers/refresh_helper.go | 52 ++++++++++++------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 4b3d1c429..b919eb84b 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -134,6 +134,14 @@ type refreshEnv struct { batch *refreshBounceBatch } +// focusOpts returns the options for re-rendering the views this refresh updates. +func (env refreshEnv) focusOpts() types.OnFocusOpts { + return types.OnFocusOpts{ + KeepScrollPosition: env.keepScrollPosition, + SkipMainViewUpdate: env.skipMainViewUpdate, + } +} + // refreshBounceBatch collects the UI-thread bounces of a batched refresh so they // can be applied together in one frame rather than one scope at a time. The // scopes run on separate worker goroutines and add concurrently, hence the @@ -897,9 +905,10 @@ func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState, case types.KeepCommitSelectionIndex: // The caller set the selection index deliberately; leave it untouched. } + + self.rerenderView(self.c.Contexts().LocalCommits, env.focusOpts()) }) - self.refreshView(self.c.Contexts().LocalCommits, env) return nil } @@ -1725,27 +1734,30 @@ func (self *RefreshHelper) refreshView(context types.Context, env refreshEnv) { // dropped, so there's nothing fresh to render — and the captured context // belongs to the old repo's now-replaced context tree anyway. self.onUIThreadUnlessRepoChanged(env, func() { - // Re-applying the filter must be done before re-rendering the view, so that - // the filtered list model is up to date for rendering. - self.searchHelper.ReApplyFilter(context) + self.rerenderView(context, env.focusOpts()) + }) +} - self.c.PostRefreshUpdateWithOptions(context, types.OnFocusOpts{ - KeepScrollPosition: env.keepScrollPosition, - SkipMainViewUpdate: env.skipMainViewUpdate, - }) +// rerenderView re-renders a context's view after a refresh has updated the +// model it shows. It must run on the UI thread. +func (self *RefreshHelper) rerenderView(context types.Context, opts types.OnFocusOpts) { + // Re-applying the filter must be done before re-rendering the view, so that + // the filtered list model is up to date for rendering. + self.searchHelper.ReApplyFilter(context) - self.c.AfterLayout(func() error { - // Re-applying the search must be done after re-rendering the view though, - // so that the "x of y" status is shown correctly. - // - // Also, it must be done after layout, because otherwise FocusPoint - // hasn't been called yet (see ListContextTrait.FocusLine), which means - // that the scroll position might be such that the entire visible - // content is outside the viewport. And this would cause problems in - // searchModelCommits. - self.searchHelper.ReApplySearch(context) - return nil - }) + self.c.PostRefreshUpdateWithOptions(context, opts) + + self.c.AfterLayout(func() error { + // Re-applying the search must be done after re-rendering the view though, + // so that the "x of y" status is shown correctly. + // + // Also, it must be done after layout, because otherwise FocusPoint + // hasn't been called yet (see ListContextTrait.FocusLine), which means + // that the scroll position might be such that the entire visible + // content is outside the viewport. And this would cause problems in + // searchModelCommits. + self.searchHelper.ReApplySearch(context) + return nil }) }