diff --git a/pkg/gui/context/list_context_trait.go b/pkg/gui/context/list_context_trait.go index 597fc99df29..9ab24cf9b5e 100644 --- a/pkg/gui/context/list_context_trait.go +++ b/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) diff --git a/pkg/gui/controllers/helpers/fixup_helper.go b/pkg/gui/controllers/helpers/fixup_helper.go index e8fa43f2dcd..e998c2ad10e 100644 --- a/pkg/gui/controllers/helpers/fixup_helper.go +++ b/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 }, diff --git a/pkg/gui/controllers/helpers/mode_helper.go b/pkg/gui/controllers/helpers/mode_helper.go index 6e5139744b6..68f7ea14913 100644 --- a/pkg/gui/controllers/helpers/mode_helper.go +++ b/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 }, }) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 6672fab2656..d076ced0a5d 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/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) }) } diff --git a/pkg/gui/controllers/helpers/refresh_helper_test.go b/pkg/gui/controllers/helpers/refresh_helper_test.go index 3a5f6ea829c..d3829127a4c 100644 --- a/pkg/gui/controllers/helpers/refresh_helper_test.go +++ b/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, } diff --git a/pkg/gui/controllers/helpers/search_helper.go b/pkg/gui/controllers/helpers/search_helper.go index bc0c938f93a..51f51079263 100644 --- a/pkg/gui/controllers/helpers/search_helper.go +++ b/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()) diff --git a/pkg/gui/controllers/helpers/sub_commits_helper.go b/pkg/gui/controllers/helpers/sub_commits_helper.go index 7bd92882693..09f32d1a99c 100644 --- a/pkg/gui/controllers/helpers/sub_commits_helper.go +++ b/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 diff --git a/pkg/gui/controllers/list_controller.go b/pkg/gui/controllers/list_controller.go index 8136c6aa9ae..c073e51413d 100644 --- a/pkg/gui/controllers/list_controller.go +++ b/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 { diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 642263d129c..0a02e73985d 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/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() diff --git a/pkg/gui/controllers/stash_controller.go b/pkg/gui/controllers/stash_controller.go index 03011e421c2..7730587f488 100644 --- a/pkg/gui/controllers/stash_controller.go +++ b/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. diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index e7b14ba048c..d92284ea563 100644 --- a/pkg/gui/gui_common.go +++ b/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 { diff --git a/pkg/gui/gui_driver.go b/pkg/gui/gui_driver.go index 673d9d726c7..096534e9664 100644 --- a/pkg/gui/gui_driver.go +++ b/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) diff --git a/pkg/gui/menu_panel.go b/pkg/gui/menu_panel.go index 23016b9a504..0ddefdbeefc 100644 --- a/pkg/gui/menu_panel.go +++ b/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 diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 75cb530186e..ff73b91f643 100644 --- a/pkg/gui/types/common.go +++ b/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) diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index 416b39b957f..35662d86c5d 100644 --- a/pkg/gui/types/context.go +++ b/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 { diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index d139984fa01..e9ad48aabf5 100644 --- a/pkg/gui/view_helpers.go +++ b/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 { diff --git a/pkg/integration/components/test_driver.go b/pkg/integration/components/test_driver.go index d65caee5d6b..afd55a845a6 100644 --- a/pkg/integration/components/test_driver.go +++ b/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 diff --git a/pkg/integration/components/test_test.go b/pkg/integration/components/test_test.go index cf0338decb1..bda63e3fb51 100644 --- a/pkg/integration/components/test_test.go +++ b/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() } diff --git a/pkg/integration/components/view_driver.go b/pkg/integration/components/view_driver.go index 937c19b7bbe..23e3502a189 100644 --- a/pkg/integration/components/view_driver.go +++ b/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 diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 15ef6f8c75d..a945e324d54 100644 --- a/pkg/integration/tests/test_list.go +++ b/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, diff --git a/pkg/integration/tests/ui/background_refresh_keeps_scroll_position.go b/pkg/integration/tests/ui/background_refresh_keeps_scroll_position.go new file mode 100644 index 00000000000..e2cf6ee1eda --- /dev/null +++ b/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) + }, +}) diff --git a/pkg/integration/tests/ui/drag_beyond_viewport.go b/pkg/integration/tests/ui/drag_beyond_viewport.go new file mode 100644 index 00000000000..f756a783c79 --- /dev/null +++ b/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) + }, +}) diff --git a/pkg/integration/tests/ui/filtering_scrolls_selection_into_view.go b/pkg/integration/tests/ui/filtering_scrolls_selection_into_view.go new file mode 100644 index 00000000000..3f1ab4d8497 --- /dev/null +++ b/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() + }, +}) diff --git a/pkg/integration/tests/ui/find_base_commit_for_fixup_scrolls_into_view.go b/pkg/integration/tests/ui/find_base_commit_for_fixup_scrolls_into_view.go new file mode 100644 index 00000000000..b0c63dcf5a9 --- /dev/null +++ b/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() + }, +}) diff --git a/pkg/integration/tests/ui/menu_scroll_position_is_reset.go b/pkg/integration/tests/ui/menu_scroll_position_is_reset.go new file mode 100644 index 00000000000..448e0b99542 --- /dev/null +++ b/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) + }, +}) diff --git a/pkg/integration/tests/ui/move_commit_scrolls_selection_into_view.go b/pkg/integration/tests/ui/move_commit_scrolls_selection_into_view.go new file mode 100644 index 00000000000..a665b548d36 --- /dev/null +++ b/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() + }, +}) diff --git a/pkg/integration/tests/ui/page_up_and_down.go b/pkg/integration/tests/ui/page_up_and_down.go new file mode 100644 index 00000000000..603edfd923d --- /dev/null +++ b/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) + }, +}) diff --git a/pkg/integration/tests/ui/sub_commits_scroll_position_is_reset.go b/pkg/integration/tests/ui/sub_commits_scroll_position_is_reset.go new file mode 100644 index 00000000000..1de8acb7043 --- /dev/null +++ b/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) + }, +}) diff --git a/pkg/integration/types/types.go b/pkg/integration/types/types.go index 4d2da760245..db9068ad920 100644 --- a/pkg/integration/types/types.go +++ b/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())