From f7d4efc59e69576ca29d397f910cc9d96d8a63cc Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 22 Dec 2025 17:04:43 +0100 Subject: [PATCH] Rerender visible lines when scrolling by page This fixes a bug in ListContextTrait.FocusLine whereby the view would go blank when scrolling by page (using ',' or '.') in views that have renderOnlyVisibleLines set to true but refreshViewportOnChange set to false. Currently we don't have any such views; the only ones who use renderOnlyVisibleLines are commits and subcommits, and they also use refreshViewportOnChange. However, we are going to add one in the next commit, and eventually it might be a good idea to convert all our list views to that by default, and get rid of the renderOnlyVisibleLines flag. --- pkg/gui/context/list_context_trait.go | 12 ++++++++++-- pkg/gui/controllers/list_controller.go | 7 +++++++ pkg/gui/types/context.go | 1 + 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/pkg/gui/context/list_context_trait.go b/pkg/gui/context/list_context_trait.go index 374c570c3..f04c24ffe 100644 --- a/pkg/gui/context/list_context_trait.go +++ b/pkg/gui/context/list_context_trait.go @@ -21,6 +21,9 @@ type ListContextTrait struct { // If this is true, we only render the visible lines of the list. Useful for lists that can // get very long, because it can save a lot of memory renderOnlyVisibleLines bool + // If renderOnlyVisibleLines is true, needRerenderVisibleLines indicates whether we need to + // rerender the visible lines e.g. because the scroll position changed + needRerenderVisibleLines bool } func (self *ListContextTrait) IsListContext() {} @@ -50,8 +53,8 @@ func (self *ListContextTrait) FocusLine(scrollIntoView bool) { self.refreshViewport() } else if self.renderOnlyVisibleLines { newOrigin, _ := self.GetViewTrait().ViewPortYBounds() - if oldOrigin != newOrigin { - self.HandleRender() + if oldOrigin != newOrigin || self.needRerenderVisibleLines { + self.refreshViewport() } } return nil @@ -105,6 +108,7 @@ func (self *ListContextTrait) HandleRender() { startIdx, length := self.GetViewTrait().ViewPortYBounds() content := self.renderLines(startIdx, startIdx+length) self.GetViewTrait().SetViewPortContentAndClearEverythingElse(totalLength, content) + self.needRerenderVisibleLines = false } else { content := self.renderLines(-1, -1) self.GetViewTrait().SetContent(content) @@ -141,6 +145,10 @@ func (self *ListContextTrait) RenderOnlyVisibleLines() bool { return self.renderOnlyVisibleLines } +func (self *ListContextTrait) SetNeedRerenderVisibleLines() { + self.needRerenderVisibleLines = true +} + func (self *ListContextTrait) TotalContentHeight() int { result := self.list.Len() if self.getNonModelItems != nil { diff --git a/pkg/gui/controllers/list_controller.go b/pkg/gui/controllers/list_controller.go index a91074392..cebc0f62e 100644 --- a/pkg/gui/controllers/list_controller.go +++ b/pkg/gui/controllers/list_controller.go @@ -178,6 +178,13 @@ func (self *ListController) handlePageChange(delta int) error { } } + // Since we already scrolled the view above, the normal mechanism that + // ListContextTrait.FocusLine uses for deciding whether rerendering is needed won't work. It is + // based on checking whether the origin was changed by the call to FocusPoint in that function, + // but since we scrolled the view directly above, the origin has already been updated. So we + // 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{}) diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index 91d4f314b..790f4c84b 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -182,6 +182,7 @@ type IListContext interface { IsListContext() // used for type switch RangeSelectEnabled() bool RenderOnlyVisibleLines() bool + SetNeedRerenderVisibleLines() IndexForGotoBottom() int }