Scroll the selection into view by default

Ever since scrolling the selection into view became opt-in, we have been
fixing the same class of regression by hand, five times so far: a
controller moves the selection somewhere new, doesn't say that it wants
the view to follow, and the selection ends up off screen. The decision
needs facts from two places — whether the selection went somewhere new is
known to the list, whether the scroll position is the caller's to manage
is known to the caller — and asking every caller for both is what keeps
going wrong. The callers that get it wrong are usually not even the ones
that moved the selection: they are pass-throughs like postRefreshUpdate,
which can't know what a refresh did to the selection.

So default to scrolling, and let the two callers that maintain the scroll
position themselves say so.

The one case where scrolling is always wrong is a refresh that no user
action is behind: a background poll, or a reload of state on window
focus, after a subprocess, or after a repo switch. Those must leave the
viewport wherever the user last scrolled it to — that is what made the
scrolling opt-in in the first place. Both are already marked in
RefreshOptions, so the refresh can decide it once, centrally, instead of
each caller judging it.

A user action that ends in a foreground refresh does now yank the view
back to the selection if the user had scrolled away from it. That's a
behaviour change, and there may be actions where it turns out to be
unwelcome; those we can fix individually, and it beats the ones that
don't scroll today.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

pkg/gui/context/list_context_trait.go

@@ -89,7 +89,7 @@ func formatListFooter(selectedLineIdx int, length int) string {
 }
 
 func (self *ListContextTrait) HandleFocus(opts types.OnFocusOpts) {
-	self.FocusLine(opts.ScrollSelectionIntoView)
+	self.FocusLine(!opts.KeepScrollPosition)
 
 	self.GetViewTrait().SetHighlight(self.list.Len() > 0)
 

pkg/gui/controllers/helpers/refresh_helper.go

@@ -111,6 +111,14 @@ type refreshEnv struct {
 	// persist its refreshed stat cache.
 	backgroundRoutine bool
 
+	// Whether the views this refresh updates must keep the scroll position they
+	// have. Focusing a list scrolls its selection into view, which is what a
+	// user action should do — but a refresh that no user action is behind must
+	// leave the viewport wherever the user last scrolled it to. That's the case
+	// for the unattended background routines, and for the refreshes that merely
+	// reload state (see RefreshOptions.DontBlockRepoSwitch).
+	keepScrollPosition bool
+
 	// the repo generation captured when the refresh started
 	generation int
 
@@ -220,8 +228,9 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr
 	// against the repo it started in, and the generation guard drops its
 	// writes.
 	env := refreshEnv{
-		background:        options.Background || options.DontBlockRepoSwitch,
-		backgroundRoutine: options.Background,
+		background:         options.Background || options.DontBlockRepoSwitch,
+		backgroundRoutine:  options.Background,
+		keepScrollPosition: options.Background || options.DontBlockRepoSwitch,
 	}
 	if !self.captureOnUIThread(calledFromWorker, env.background, func() {
 		env.generation = self.c.State().GetRepoGeneration()
@@ -1595,7 +1604,11 @@ func (self *RefreshHelper) refreshView(context types.Context, env refreshEnv) {
 		// the filtered list model is up to date for rendering.
 		self.searchHelper.ReApplyFilter(context)
 
-		self.c.PostRefreshUpdate(context)
+		if env.keepScrollPosition {
+			self.c.PostRefreshUpdateKeepingScrollPosition(context)
+		} else {
+			self.c.PostRefreshUpdate(context)
+		}
 
 		self.c.AfterLayout(func() error {
 			// Re-applying the search must be done after re-rendering the view though,
@@ -1771,7 +1784,10 @@ func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo, bra
 		// the branches and remotes as they are on the UI thread, after their
 		// own refreshes' bounces have applied.
 		self.rebuildPullRequestsMap()
-		self.c.PostRefreshUpdate(self.c.Contexts().Branches)
+		// This lands whenever the network call happens to return, and only
+		// changes how the branches are rendered, not which one is selected, so
+		// it has no business moving the viewport.
+		self.c.PostRefreshUpdateKeepingScrollPosition(self.c.Contexts().Branches)
 	})
 }
 

pkg/gui/controllers/list_controller.go

@@ -136,7 +136,7 @@ func (self *ListController) handleLineChangeAux(f func(int), change int) error {
 			self.context.SetNeedRerenderVisibleLines()
 		}
 
-		self.context.HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true})
+		self.context.HandleFocus(types.OnFocusOpts{})
 	} else {
 		// If the selection did not change (because, for example, we are at the top of the list and
 		// press up), we still want to ensure that the selection is visible. This is useful after
@@ -205,9 +205,10 @@ func (self *ListController) handlePageChange(delta int) error {
 	// must tell it explicitly to rerender.
 	self.context.SetNeedRerenderVisibleLines()
 
-	// Since we are maintaining the scroll position ourselves above, there's no point in passing
-	// ScrollSelectionIntoView=true here.
-	self.context.HandleFocus(types.OnFocusOpts{})
+	// This function scrolls the view itself, keeping the selection at the edge of
+	// the viewport rather than in its middle, so the scroll position is ours to
+	// maintain, not the focus mechanism's.
+	self.context.HandleFocus(types.OnFocusOpts{KeepScrollPosition: true})
 
 	return nil
 }
@@ -280,7 +281,10 @@ func (self *ListController) selectRangeThroughViewIndex(viewIndex int) {
 	newSelectedLineIdx := self.context.ViewIndexToModelIndex(viewIndex)
 	list.ExpandNonStickyRange(newSelectedLineIdx - list.GetSelectedLineIdx())
 
-	self.context.HandleFocus(types.OnFocusOpts{})
+	// The pointer can be outside the viewport, in which case so is the end of
+	// the range; the drag autoscroller takes care of following it, one line at a
+	// time, for as long as the pointer stays there.
+	self.context.HandleFocus(types.OnFocusOpts{KeepScrollPosition: true})
 }
 
 func (self *ListController) handleDragAutoscroll(viewIndex int) bool {

pkg/gui/controllers/local_commits_controller.go

@@ -1171,7 +1171,7 @@ func (self *LocalCommitsController) move(
 			return err
 		}
 		self.context().MoveSelection(offset)
-		self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true})
+		self.context().HandleFocus(types.OnFocusOpts{})
 
 		// Block input until the refresh has landed: a quick second press must
 		// read the moved todo from the refreshed model, not grab whatever the
@@ -1204,7 +1204,7 @@ func (self *LocalCommitsController) move(
 				Then: func() error {
 					if err == nil {
 						self.context().MoveSelection(offset)
-						self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true})
+						self.context().HandleFocus(types.OnFocusOpts{})
 					}
 					if onComplete != nil {
 						return onComplete()

pkg/gui/gui_common.go

@@ -39,7 +39,11 @@ func (self *guiCommon) RefreshFromWorker(opts types.RefreshOptions) {
 }
 
 func (self *guiCommon) PostRefreshUpdate(context types.Context) {
-	self.gui.postRefreshUpdate(context)
+	self.gui.postRefreshUpdate(context, false)
+}
+
+func (self *guiCommon) PostRefreshUpdateKeepingScrollPosition(context types.Context) {
+	self.gui.postRefreshUpdate(context, true)
 }
 
 func (self *guiCommon) RunSubprocessAndRefresh(cmdObj *oscommands.CmdObj) error {

pkg/gui/types/common.go

@@ -48,8 +48,13 @@ type IGuiCommon interface {
 	RefreshFromWorker(RefreshOptions)
 	// we call this when we've changed something in the view model but not the actual model,
 	// e.g. expanding or collapsing a folder in a file view. Calling 'Refresh' in this
-	// case would be overkill, although refresh will internally call 'PostRefreshUpdate'
+	// case would be overkill, although refresh will internally call 'PostRefreshUpdate'.
+	// It re-focuses the context's selection, which scrolls it into view.
 	PostRefreshUpdate(Context)
+	// Like PostRefreshUpdate, but leaves the view scrolled where it is. For
+	// refreshes that no user action is behind: those must not move the viewport
+	// away from wherever the user last put it.
+	PostRefreshUpdateKeepingScrollPosition(Context)
 
 	// renders string to a view without resetting its origin
 	SetViewContent(view *gocui.View, content string)

pkg/gui/types/context.go

@@ -227,9 +227,13 @@ type IViewTrait interface {
 }
 
 type OnFocusOpts struct {
-	ClickedWindowName       string
-	ClickedViewLineIdx      int
-	ScrollSelectionIntoView bool
+	ClickedWindowName  string
+	ClickedViewLineIdx int
+
+	// Focusing a list context scrolls its selection into view. Set this to leave
+	// the view's scroll position alone instead; only for callers that maintain
+	// it themselves, e.g. by keeping the selection at the edge of the viewport.
+	KeepScrollPosition bool
 }
 
 type OnFocusLostOpts struct {

pkg/gui/view_helpers.go

@@ -132,7 +132,7 @@ func (gui *Gui) renderContentOnly() {
 // postRefreshUpdate is to be called on a context after the state that it depends on has been refreshed
 // if the context's view is set to another context we do nothing.
 // if the context's view is the current view we trigger a focus; re-selecting the current item.
-func (gui *Gui) postRefreshUpdate(c types.Context) {
+func (gui *Gui) postRefreshUpdate(c types.Context, keepScrollPosition bool) {
 	t := time.Now()
 	defer func() {
 		gui.Log.Infof("postRefreshUpdate for %s took %s", c.GetKey(), time.Since(t))
@@ -141,14 +141,14 @@ func (gui *Gui) postRefreshUpdate(c types.Context) {
 	c.HandleRender()
 
 	if gui.currentViewName() == c.GetViewName() {
-		c.HandleFocus(types.OnFocusOpts{})
+		c.HandleFocus(types.OnFocusOpts{KeepScrollPosition: keepScrollPosition})
 	} else {
 		// The FocusLine call is included in the HandleFocus method which we
 		// call for focused views above; but we need to call it here for
 		// non-focused views to ensure that an inactive selection is painted
 		// correctly, and that integration tests see the up to date selection
 		// state.
-		c.FocusLine(false)
+		c.FocusLine(!keepScrollPosition)
 
 		currentCtx := gui.State.ContextMgr.Current()
 		if currentCtx.GetKey() == context.NORMAL_MAIN_CONTEXT_KEY || currentCtx.GetKey() == context.NORMAL_SECONDARY_CONTEXT_KEY {