diff --git a/pkg/gui/context/list_context_trait.go b/pkg/gui/context/list_context_trait.go index 597fc99df..9ab24cf9b 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/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 6672fab26..81eeec4d1 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() @@ -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) }) } diff --git a/pkg/gui/controllers/list_controller.go b/pkg/gui/controllers/list_controller.go index 8136c6aa9..c073e5141 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 642263d12..0a02e7398 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/gui_common.go b/pkg/gui/gui_common.go index e7b14ba04..d92284ea5 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/types/common.go b/pkg/gui/types/common.go index 75cb53018..ff73b91f6 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 416b39b95..35662d86c 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 d139984fa..e9ad48aab 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 {