Scroll the selection into view by default again (#5928)

In v0.58 (see #5134) we changed the refresh behavior to no longer scroll
a selection into view by default if it had been scrolled out of view
using the mouse wheel; the primary reason for that was to avoid a
background fetch or files refresh to yank the selection back into view
while you are looking at something else, which was pretty annoying.
However, that meant we had to fix lots of cases where the selection
didn't become visible after a normal, foreground user action, and add
code to manually scroll it into view again in those cases. This is
error-prone and easy to forget for new features, and to this day we were
still missing some.

So turn it around: by default, the selection is scrolled into view
again, and the few cases where we don't want it (background routines and
the focus-in refresh) opt out.

This does mean that some user actions now scroll into view that didn't
before, and there might be cases where this is unwanted (I can't think
of one, but who knows). If we come across one we can easily fix it by
opting out; this is probably still much better than not scrolling into
view where it's wanted.

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/fixup_helper.go

@@ -141,7 +141,6 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error {
 			}
 
 			self.c.Contexts().LocalCommits.SetSelection(index)
-			self.c.Contexts().LocalCommits.FocusLine(true)
 			self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{})
 			return nil
 		},

pkg/gui/controllers/helpers/mode_helper.go

@@ -270,11 +270,6 @@ func (self *ModeHelper) changeFiltering(setFilter func(), selectCommit func()) e
 
 				selectCommit()
 				self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits)
-				// The list we just selected in has nothing to do with the one
-				// that was showing, so wherever it was scrolled to says nothing
-				// about where the selection now is. PostRefreshUpdate leaves the
-				// scroll position alone, so ask for it separately.
-				self.c.Contexts().LocalCommits.FocusLine(true)
 				return nil
 			},
 		})

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()
@@ -842,33 +851,21 @@ func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState,
 			self.c.Model().CheckedOutBranch = ""
 		}
 
-		scrollSelectionIntoView := false
 		switch commitSelection {
 		case types.SelectHeadCommit:
 			if headCommitIdx := models.HeadCommitIdx(commits); headCommitIdx >= 0 {
 				self.c.Contexts().LocalCommits.SetSelection(headCommitIdx)
-				scrollSelectionIntoView = true
 			}
 		case types.KeepCommitSelectionByHash:
 			if selectionRange != nil {
-				selectedIdx, rangeStartIdx, didMove, found := findLocalCommitSelectionRange(commits, selectionRange)
+				selectedIdx, rangeStartIdx, found := findLocalCommitSelectionRange(commits, selectionRange)
 				if found {
 					self.c.Contexts().LocalCommits.SetSelectionRangeAndMode(selectedIdx, rangeStartIdx, selectionRange.mode)
-					scrollSelectionIntoView = didMove
 				}
 			}
 		case types.KeepCommitSelectionIndex:
 			// The caller set the selection index deliberately; leave it untouched.
 		}
-
-		if scrollSelectionIntoView {
-			// Enqueued from within this bounce so it runs after refreshView's
-			// render below (which was enqueued first), matching the previous
-			// ordering where FocusLine ran after the view was re-rendered.
-			self.onUIThreadUnlessRepoChanged(env, func() {
-				self.c.Contexts().LocalCommits.FocusLine(true)
-			})
-		}
 	})
 
 	self.refreshView(self.c.Contexts().LocalCommits, env)
@@ -880,8 +877,6 @@ type localCommitSelectionRange struct {
 	selectedIsTODO   bool
 	rangeStartHash   string
 	rangeStartIsTODO bool
-	selectedIdx      int
-	rangeStartIdx    int
 	mode             traits.RangeSelectMode
 }
 
@@ -900,8 +895,6 @@ func captureLocalCommitSelectionRange(
 		selectedIsTODO:   commits[selectedIdx].IsTODO(),
 		rangeStartHash:   commits[rangeStartIdx].Hash(),
 		rangeStartIsTODO: commits[rangeStartIdx].IsTODO(),
-		selectedIdx:      selectedIdx,
-		rangeStartIdx:    rangeStartIdx,
 		mode:             mode,
 	}
 }
@@ -909,17 +902,16 @@ func captureLocalCommitSelectionRange(
 func findLocalCommitSelectionRange(
 	commits []*models.Commit,
 	selectionRange *localCommitSelectionRange,
-) (int, int, bool, bool) {
+) (int, int, bool) {
 	selectedIdx, foundSelected := findCommitByHashPreferringTODOStatus(
 		commits, selectionRange.selectedHash, selectionRange.selectedIsTODO)
 	rangeStartIdx, foundRangeStart := findCommitByHashPreferringTODOStatus(
 		commits, selectionRange.rangeStartHash, selectionRange.rangeStartIsTODO)
 	if !foundSelected || !foundRangeStart {
-		return 0, 0, false, false
+		return 0, 0, false
 	}
 
-	didMove := selectedIdx != selectionRange.selectedIdx || rangeStartIdx != selectionRange.rangeStartIdx
-	return selectedIdx, rangeStartIdx, didMove, true
+	return selectedIdx, rangeStartIdx, true
 }
 
 // findCommitByHashPreferringTODOStatus finds the commit with the given hash.
@@ -1170,10 +1162,8 @@ func (self *RefreshHelper) refreshBranches(captured capturedBranchState, refresh
 				}
 			}
 		case types.SelectCheckedOutBranch:
-			// The checked-out branch is always at the top of the list. Setting
-			// the selection doesn't scroll the view, so also reset the origin.
+			// The checked-out branch is always at the top of the list.
 			self.c.Contexts().Branches.SetSelectedLineIdx(0)
-			self.c.Contexts().Branches.GetView().SetOriginY(0)
 		}
 
 		// Need to re-render the commits view because the visualization of local
@@ -1464,11 +1454,9 @@ func (self *RefreshHelper) refreshReflogCommits(captured capturedReflogState, en
 		self.c.Model().ReflogCommits = reflogCommits
 		self.c.Model().FilteredReflogCommits = filteredReflogCommits
 		// Setting the selection here, in the same bounce that writes the list,
-		// keeps it on the UI thread and atomic with the list update. Setting the
-		// selection doesn't scroll the view, so also reset the origin.
+		// keeps it on the UI thread and atomic with the list update.
 		if selectTopEntry {
 			self.c.Contexts().ReflogCommits.SetSelectedLineIdx(0)
-			self.c.Contexts().ReflogCommits.GetView().SetOriginY(0)
 		}
 	})
 
@@ -1595,7 +1583,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 +1763,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/helpers/refresh_helper_test.go

@@ -28,8 +28,6 @@ func TestCaptureLocalCommitSelectionRange(t *testing.T) {
 			expected: &localCommitSelectionRange{
 				selectedHash:   "b",
 				rangeStartHash: "a",
-				selectedIdx:    1,
-				rangeStartIdx:  0,
 				mode:           traits.RangeSelectModeSticky,
 			},
 		},
@@ -74,15 +72,12 @@ func TestFindLocalCommitSelectionRange(t *testing.T) {
 	type expectation struct {
 		selectedIdx   int
 		rangeStartIdx int
-		moved         bool
 		found         bool
 	}
 
 	selectionRange := localCommitSelectionRange{
 		selectedHash:   "b",
 		rangeStartHash: "c",
-		selectedIdx:    1,
-		rangeStartIdx:  2,
 		mode:           traits.RangeSelectModeSticky,
 	}
 
@@ -97,7 +92,6 @@ func TestFindLocalCommitSelectionRange(t *testing.T) {
 			expected: expectation{
 				selectedIdx:   2,
 				rangeStartIdx: 3,
-				moved:         true,
 				found:         true,
 			},
 		},
@@ -126,7 +120,6 @@ func TestFindLocalCommitSelectionRange(t *testing.T) {
 			expected: expectation{
 				selectedIdx:   2,
 				rangeStartIdx: 3,
-				moved:         true,
 				found:         true,
 			},
 		},
@@ -139,7 +132,6 @@ func TestFindLocalCommitSelectionRange(t *testing.T) {
 			expected: expectation{
 				selectedIdx:   0,
 				rangeStartIdx: 1,
-				moved:         true,
 				found:         true,
 			},
 		},
@@ -147,11 +139,10 @@ func TestFindLocalCommitSelectionRange(t *testing.T) {
 
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			selectedIdx, rangeStartIdx, moved, found := findLocalCommitSelectionRange(testCase.commits, &selectionRange)
+			selectedIdx, rangeStartIdx, found := findLocalCommitSelectionRange(testCase.commits, &selectionRange)
 			actual := expectation{
 				selectedIdx:   selectedIdx,
 				rangeStartIdx: rangeStartIdx,
-				moved:         moved,
 				found:         found,
 			}
 

pkg/gui/controllers/helpers/search_helper.go

@@ -225,7 +225,6 @@ func (self *SearchHelper) OnPromptContentChanged(searchString string) {
 	switch context := state.Context.(type) {
 	case types.IFilterableContext:
 		context.SetSelection(0)
-		context.GetView().SetOriginY(0)
 		context.SetFilter(searchString, self.c.UserConfig().Gui.UseFuzzySearch())
 		self.c.PostRefreshUpdate(context)
 	case types.ISearchableContext:
@@ -241,6 +240,9 @@ func (self *SearchHelper) ReApplyFilter(context types.Context) {
 		state := self.searchState()
 		if context == state.Context && self.c.Context().Current().GetKey() == self.c.Contexts().Search.GetKey() {
 			filterableContext.SetSelection(0)
+			// This runs as part of a refresh, and a refresh that no user action
+			// is behind keeps the scroll position, which would leave the view
+			// scrolled somewhere the filtered list no longer has anything at.
 			filterableContext.GetView().SetOriginY(0)
 		}
 		filterableContext.ReApplyFilter(self.c.UserConfig().Gui.UseFuzzySearch())

pkg/gui/controllers/helpers/sub_commits_helper.go

@@ -66,7 +66,6 @@ func (self *SubCommitsHelper) ViewSubCommits(opts ViewSubCommitsOpts) error {
 	subCommitsContext.GetView().TitlePrefix = opts.Context.GetView().TitlePrefix
 
 	self.c.PostRefreshUpdate(self.c.Contexts().SubCommits)
-	subCommitsContext.FocusLine(true)
 
 	self.c.Context().Push(self.c.Contexts().SubCommits, types.OnFocusOpts{})
 	return nil

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/controllers/stash_controller.go

@@ -251,7 +251,6 @@ func (self *StashController) handleRenameStashEntry(stashEntry *models.StashEntr
 				return err
 			}
 			self.context().SetSelection(0) // Select the renamed stash
-			self.context().FocusLine(true)
 			// Renaming re-creates the stash at the top, shifting the other
 			// entries' indices; block input so that a quick next action sees
 			// the refreshed list rather than the stale indices.

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/gui_driver.go

@@ -69,6 +69,10 @@ func (self *GuiDriver) MouseMove(x, y int) {
 	self.replayMouseEvent(x, y, tcell.ButtonPrimary)
 }
 
+func (self *GuiDriver) ScrollWheelDown(x, y int) {
+	self.replayMouseEvent(x, y, tcell.WheelDown)
+}
+
 func (self *GuiDriver) MouseRelease(x, y int) {
 	self.replayMouseEvent(x, y, tcell.ButtonNone)
 }
@@ -128,6 +132,16 @@ func (self *GuiDriver) FocusInAndClick(x, y int) {
 	self.waitTillIdle()
 }
 
+// RefreshInBackground performs the refresh that the background routines perform
+// on a timer (see BackgroundRoutineMgr). Tests drive it directly rather than
+// turning those routines on, so that they neither wait for a timer nor depend on
+// one firing at a particular moment.
+func (self *GuiDriver) RefreshInBackground() {
+	self.gui.c.RefreshFromWorker(types.RefreshOptions{Background: true})
+
+	self.waitTillIdle()
+}
+
 func (self *GuiDriver) PretendMergeOrRebaseStartedInLazygit() {
 	self.gui.onUIThread(func() error {
 		self.gui.State.SetMergeOrRebaseStartedInLazygit(true)

pkg/gui/menu_panel.go

@@ -72,8 +72,6 @@ func (gui *Gui) createMenu(opts types.CreateMenuOptions) error {
 	gui.State.Contexts.Menu.SetOnCancel(opts.OnCancel)
 	gui.State.Contexts.Menu.SetSelection(0)
 
-	gui.Views.Menu.SetOriginY(0)
-
 	gui.Views.Menu.Title = opts.Title
 	gui.Views.Menu.FgColor = theme.GocuiDefaultTextColor
 

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 {

pkg/integration/components/test_driver.go

@@ -78,6 +78,12 @@ func (self *TestDriver) repeatMouseMove() {
 	self.mouseMove(self.mouseX, self.mouseY)
 }
 
+func (self *TestDriver) scrollWheelDown(x, y int) {
+	self.SetCaption(fmt.Sprintf("Scrolling down at %d, %d", x, y))
+	self.gui.ScrollWheelDown(x, y)
+	self.Wait(self.inputDelay)
+}
+
 func (self *TestDriver) mouseRelease() {
 	self.SetCaption(fmt.Sprintf("Releasing mouse at %d, %d", self.mouseX, self.mouseY))
 	self.gui.MouseRelease(self.mouseX, self.mouseY)
@@ -136,6 +142,15 @@ func (self *TestDriver) Log(message string) {
 	self.gui.LogUI(message)
 }
 
+// RefreshInBackground performs the refresh that lazygit's background routines
+// perform on a timer, e.g. to pick up changes made by RunCommand. Tests use this
+// rather than turning those routines on and waiting for them.
+func (self *TestDriver) RefreshInBackground() {
+	self.SetCaption("Refreshing in the background")
+	self.gui.RefreshInBackground()
+	self.Wait(self.inputDelay)
+}
+
 // allows the user to run shell commands during the test to emulate background activity
 func (self *TestDriver) Shell() *Shell {
 	return self.shell

pkg/integration/components/test_test.go

@@ -56,6 +56,12 @@ func (self *fakeGuiDriver) MouseRelease(x, y int) {
 	self.releasedCoordinates = append(self.releasedCoordinates, coordinate{x: x, y: y})
 }
 
+func (self *fakeGuiDriver) ScrollWheelDown(x, y int) {
+}
+
+func (self *fakeGuiDriver) RefreshInBackground() {
+}
+
 func (self *fakeGuiDriver) OnUIThreadAndWait(f func()) {
 	f()
 }

pkg/integration/components/view_driver.go

@@ -355,6 +355,31 @@ func (self *ViewDriver) SelectedLineIdxAtLeast(expected int) *ViewDriver {
 	return self
 }
 
+// asserts on the scroll position of the view, i.e. the index of the line that
+// is shown at the top of the view.
+func (self *ViewDriver) OriginY(expected int) *ViewDriver {
+	self.t.assertWithRetries(func() (bool, string) {
+		actual := self.getView().OriginY()
+		return expected == actual, fmt.Sprintf("%s: Expected origin Y to be %d, got %d", self.context, expected, actual)
+	})
+
+	return self
+}
+
+// asserts that the selected line is inside the visible area of the view
+func (self *ViewDriver) SelectedLineIsVisible() *ViewDriver {
+	self.t.assertWithRetries(func() (bool, string) {
+		view := self.getView()
+		firstVisible, lastVisible := view.OriginY(), view.OriginY()+view.InnerHeight()-1
+		actual := view.SelectedLineIdx()
+		return actual >= firstVisible && actual <= lastVisible,
+			fmt.Sprintf("%s: Expected the selected line (%d) to be visible, but only lines %d to %d are",
+				self.context, actual, firstVisible, lastVisible)
+	})
+
+	return self
+}
+
 func (self *ViewDriver) OriginYAtLeast(expected int) *ViewDriver {
 	self.t.assertEventually(func() (bool, string) {
 		var actual int
@@ -533,6 +558,15 @@ func (self *ViewDriver) MouseMoveToBottom(x int) *ViewDriver {
 	return self.MouseMove(x, self.getView().InnerHeight()-1)
 }
 
+// scrolls the view down by one notch of the mouse wheel, i.e. by
+// gui.scrollHeight lines. This moves the scroll position without moving the
+// selection.
+func (self *ViewDriver) ScrollWheelDown() *ViewDriver {
+	offsetX, offsetY, _, _ := self.getView().Dimensions()
+	self.t.scrollWheelDown(offsetX+1, offsetY+1)
+	return self
+}
+
 func (self *ViewDriver) RepeatMouseMove() *ViewDriver {
 	self.t.repeatMouseMove()
 	return self

pkg/integration/tests/test_list.go

@@ -496,20 +496,28 @@ var tests = []*components.IntegrationTest{
 	tag.Reset,
 	tag.ResetToDuplicateNamedBranch,
 	ui.Accordion,
+	ui.BackgroundRefreshKeepsScrollPosition,
 	ui.BranchesNotFirstTab,
 	ui.CommitsNotFirstTab,
 	ui.DisableSwitchTabWithPanelJumpKeys,
+	ui.DragBeyondViewport,
 	ui.EmptyMenu,
+	ui.FilteringScrollsSelectionIntoView,
+	ui.FindBaseCommitForFixupScrollsIntoView,
 	ui.HideSidePanel,
 	ui.KeybindingSuggestionsDontCrashOnDisabledBindings,
 	ui.KeybindingSuggestionsWhenSwitchingRepos,
+	ui.MenuScrollPositionIsReset,
 	ui.ModeSpecificKeybindingSuggestions,
+	ui.MoveCommitScrollsSelectionIntoView,
 	ui.OpenLinkFailure,
+	ui.PageUpAndDown,
 	ui.PromoteTabToSidePanel,
 	ui.RangeSelect,
 	ui.RangeSelectWithAutoscroll,
 	ui.ReloadSidePanels,
 	ui.ReorderSidePanels,
+	ui.SubCommitsScrollPositionIsReset,
 	ui.SwitchTabFromMenu,
 	ui.SwitchTabWithPanelJumpKeys,
 	undo.UndoCheckoutAndDrop,

pkg/integration/tests/ui/background_refresh_keeps_scroll_position.go

@@ -0,0 +1,41 @@
+package ui
+
+import (
+	"fmt"
+
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var BackgroundRefreshKeepsScrollPosition = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "A background refresh doesn't scroll the selection back into view",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	Width:        120,
+	Height:       30,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.EmptyCommit("initial commit")
+		for i := range 20 {
+			shell.CreateFile(fmt.Sprintf("file%02d", i), "")
+		}
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Files().
+			Focus().
+			SelectNextItem().
+			SelectedLine(Contains("file00")).
+			// Scroll the selection out of view with the mouse wheel
+			ScrollWheelDown().
+			ScrollWheelDown().
+			OriginY(4).
+			Tap(func() {
+				t.Shell().CreateFile("aaa", "")
+				t.RefreshInBackground()
+			}).
+			// The new file sorts before the selected one, so the selection has
+			// moved down a line; the view must stay where the user left it though
+			SelectedLineIdx(2).
+			OriginY(4)
+	},
+})

pkg/integration/tests/ui/drag_beyond_viewport.go

@@ -0,0 +1,37 @@
+package ui
+
+import (
+	"fmt"
+
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var DragBeyondViewport = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Dragging a range selection beyond the bottom of the panel doesn't scroll the view",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	Width:        120,
+	Height:       30,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.EmptyCommit("initial commit")
+		for i := range 20 {
+			shell.CreateFile(fmt.Sprintf("file%02d", i), "")
+		}
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Files().
+			Focus().
+			OriginY(0).
+			// The pointer ends up below the panel, so the range extends to a line
+			// that isn't visible. Scrolling there is the drag autoscroller's job,
+			// which scrolls line by line for as long as the pointer stays there;
+			// the drag itself must leave the scroll position alone.
+			ClickAndHold(1, 1).
+			MouseMove(1, 8).
+			MouseRelease().
+			SelectedLineIdx(8).
+			OriginY(0)
+	},
+})

pkg/integration/tests/ui/filtering_scrolls_selection_into_view.go

@@ -0,0 +1,62 @@
+package ui
+
+import (
+	"fmt"
+
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var FilteringScrollsSelectionIntoView = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Entering and leaving filtering mode scrolls the selected commit into view",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	Width:        120,
+	Height:       30,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		for i := range 40 {
+			file := "otherFile"
+			if i%2 == 0 {
+				file = "filterFile"
+			}
+			shell.UpdateFileAndAdd(file, fmt.Sprintf("content %02d", i))
+			shell.Commit(fmt.Sprintf("commit %02d", i))
+		}
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Press(keys.Universal.GotoBottom).
+			SelectedLine(Contains("commit 00")).
+			OriginYAtLeast(1).
+			Press(keys.Universal.FilteringMenu)
+
+		t.ExpectPopup().Menu().
+			Title(Equals("Filtering")).
+			Select(Contains("Enter path to filter by")).
+			Confirm()
+		t.ExpectPopup().Prompt().
+			Title(Equals("Enter path:")).
+			Type("filterFile").
+			Confirm()
+
+		// The filtered list has nothing to do with the one that was showing, so
+		// its scroll position doesn't either: we start at the top again
+		t.Views().Commits().
+			IsFocused().
+			SelectedLine(Contains("commit 38")).
+			SelectedLineIdx(0).
+			OriginY(0).
+			Press(keys.Universal.GotoBottom).
+			SelectedLine(Contains("commit 00")).
+			PressEscape()
+
+		// Leaving filtering mode keeps the commit selected, at its position in
+		// the full list, which needs scrolling to again
+		t.Views().Commits().
+			IsFocused().
+			SelectedLine(Contains("commit 00")).
+			SelectedLineIsVisible()
+	},
+})

pkg/integration/tests/ui/find_base_commit_for_fixup_scrolls_into_view.go

@@ -0,0 +1,35 @@
+package ui
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var FindBaseCommitForFixupScrollsIntoView = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Finding the base commit for a fixup scrolls it into view",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	Width:        120,
+	Height:       30,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.NewBranch("mybranch").
+			EmptyCommit("1st commit").
+			CreateFileAndAdd("file1", "line 1\nline 2\nline 3\n").
+			Commit("base commit").
+			CreateNCommits(40).
+			UpdateFile("file1", "line 1\nline 2 changed\nline 3\n")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Files().
+			Focus().
+			Press(keys.Files.FindBaseCommitForFixup)
+
+		// The base commit is at the very bottom of the list, far below the
+		// visible area
+		t.Views().Commits().
+			IsFocused().
+			SelectedLine(Contains("base commit")).
+			SelectedLineIsVisible()
+	},
+})

pkg/integration/tests/ui/menu_scroll_position_is_reset.go

@@ -0,0 +1,39 @@
+package ui
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var MenuScrollPositionIsReset = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "A menu that is opened after a scrolled down one starts at the top again",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateFile("myfile", "myfile")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Files().
+			IsFocused().
+			Press(keys.Universal.OptionMenu)
+
+		t.Views().Menu().
+			IsFocused().
+			// The first line is a section header, so the first item is at index 1
+			SelectedLineIdx(1).
+			OriginY(0).
+			Press(keys.Universal.GotoBottom).
+			OriginYAtLeast(1).
+			PressEscape()
+
+		t.Views().Files().
+			IsFocused().
+			Press(keys.Universal.OptionMenu)
+
+		t.Views().Menu().
+			IsFocused().
+			SelectedLineIdx(1).
+			OriginY(0)
+	},
+})

pkg/integration/tests/ui/move_commit_scrolls_selection_into_view.go

@@ -0,0 +1,31 @@
+package ui
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var MoveCommitScrollsSelectionIntoView = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Moving a commit down scrolls it into view if it isn't visible",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	Width:        120,
+	Height:       30,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateNCommits(40)
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			SelectedLine(Contains("commit-40")).
+			// Scroll the selected commit out of view with the mouse wheel
+			ScrollWheelDown().
+			ScrollWheelDown().
+			OriginY(4).
+			Press(keys.Commits.MoveDownCommit).
+			SelectedLine(Contains("commit-40")).
+			SelectedLineIdx(1).
+			SelectedLineIsVisible()
+	},
+})

pkg/integration/tests/ui/page_up_and_down.go

@@ -0,0 +1,47 @@
+package ui
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+const (
+	// The height of the commits panel in this test's window, in lines.
+	commitsPanelHeight = 5
+	// Paging keeps one line of overlap between the old and the new page.
+	pageDelta = commitsPanelHeight - 1
+)
+
+var PageUpAndDown = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Paging down and up keeps the selection at the edge of the viewport",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	Width:        120,
+	Height:       30,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateNCommits(40)
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			SelectedLineIdx(0).
+			OriginY(0).
+			Press(keys.Universal.NextPage).
+			// The selection moves to the bottom of the viewport; nothing scrolls yet
+			SelectedLineIdx(commitsPanelHeight - 1).
+			OriginY(0).
+			Press(keys.Universal.NextPage).
+			// Now the view scrolls by a page, and the selection stays at the bottom
+			SelectedLineIdx(commitsPanelHeight - 1 + pageDelta).
+			OriginY(pageDelta).
+			Press(keys.Universal.PrevPage).
+			// The selection moves to the top of the viewport; nothing scrolls
+			SelectedLineIdx(pageDelta).
+			OriginY(pageDelta).
+			Press(keys.Universal.PrevPage).
+			// And back a page, with the selection staying at the top
+			SelectedLineIdx(0).
+			OriginY(0)
+	},
+})

pkg/integration/tests/ui/sub_commits_scroll_position_is_reset.go

@@ -0,0 +1,39 @@
+package ui
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var SubCommitsScrollPositionIsReset = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Viewing the commits of a branch again after scrolling down starts at the top again",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	Width:        120,
+	Height:       30,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateNCommits(40)
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Branches().
+			Focus().
+			PressEnter()
+
+		t.Views().SubCommits().
+			IsFocused().
+			OriginY(0).
+			Press(keys.Universal.GotoBottom).
+			OriginYAtLeast(1).
+			PressEscape()
+
+		t.Views().Branches().
+			IsFocused().
+			PressEnter()
+
+		t.Views().SubCommits().
+			IsFocused().
+			SelectedLineIdx(0).
+			OriginY(0)
+	},
+})

pkg/integration/types/types.go

@@ -31,6 +31,9 @@ type GuiDriver interface {
 	ClickAndHold(int, int)
 	MouseMove(int, int)
 	MouseRelease(int, int)
+	ScrollWheelDown(int, int)
+	// Perform the refresh that a background routine would perform on a timer
+	RefreshInBackground()
 	// Can be used to avoid data races with the UI thread in the uncommon cases that
 	// the test driver needs to assert state while the gui is not idle.
 	OnUIThreadAndWait(func())