From 2b4bdda32c31e9a01a3284bfba962f39808583ef Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Mon, 15 Nov 2021 21:43:13 +0000 Subject: [PATCH 1/5] Don't call OnScrolled if offset did not change Fixes #1868 --- container/doctabs.go | 4 ---- internal/widget/scroller.go | 8 ++++++-- internal/widget/scroller_test.go | 20 +++++++++++++++++++- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/container/doctabs.go b/container/doctabs.go index 25dcbca16b..1a1e9dbb10 100644 --- a/container/doctabs.go +++ b/container/doctabs.go @@ -69,10 +69,6 @@ func (t *DocTabs) CreateRenderer() fyne.WidgetRenderer { r.box = NewHBox(r.create, r.action) var lastX, lastY float32 r.scroller.OnScrolled = func(offset fyne.Position) { - // FIXME OnScrolled can be called when the offset hasn't changed (#1868) - if offset.X == lastX && offset.Y == lastY { - return - } lastX = offset.X lastY = offset.Y r.updateIndicator(false) diff --git a/internal/widget/scroller.go b/internal/widget/scroller.go index ecc377f850..4117244620 100644 --- a/internal/widget/scroller.go +++ b/internal/widget/scroller.go @@ -209,13 +209,15 @@ func (a *scrollBarArea) MouseOut() { } func (a *scrollBarArea) moveBar(offset float32, barSize fyne.Size) { + oldX := a.scroll.Offset.X + oldY := a.scroll.Offset.Y switch a.orientation { case scrollBarOrientationHorizontal: a.scroll.Offset.X = a.computeScrollOffset(barSize.Width, offset, a.scroll.Size().Width, a.scroll.Content.Size().Width) default: a.scroll.Offset.Y = a.computeScrollOffset(barSize.Height, offset, a.scroll.Size().Height, a.scroll.Content.Size().Height) } - if f := a.scroll.OnScrolled; f != nil { + if f := a.scroll.OnScrolled; f != nil && (a.scroll.Offset.X != oldX || a.scroll.Offset.Y != oldY){ f(a.scroll.Offset) } a.scroll.refreshWithoutOffsetUpdate() @@ -476,9 +478,11 @@ func (s *Scroll) updateOffset(deltaX, deltaY float32) bool { } return false } + oldX := s.Offset.X + oldY := s.Offset.Y s.Offset.X = computeOffset(s.Offset.X, -deltaX, s.Size().Width, s.Content.MinSize().Width) s.Offset.Y = computeOffset(s.Offset.Y, -deltaY, s.Size().Height, s.Content.MinSize().Height) - if f := s.OnScrolled; f != nil { + if f := s.OnScrolled; f != nil && (s.Offset.X != oldX || s.Offset.Y != oldY) { f(s.Offset) } return true diff --git a/internal/widget/scroller_test.go b/internal/widget/scroller_test.go index f374b59a57..6fae239e26 100644 --- a/internal/widget/scroller_test.go +++ b/internal/widget/scroller_test.go @@ -90,6 +90,25 @@ func TestScrollContainer_MinSize_Direction(t *testing.T) { }) } +func TestScrollContainer_OnScrolled(t *testing.T) { + rect := canvas.NewRectangle(color.Black) + rect.SetMinSize(fyne.NewSize(1000, 1000)) + scroll := NewScroll(rect) + scroll.Resize(fyne.NewSize(100, 100)) + + scrolled := false + scroll.OnScrolled = func(fyne.Position) { + scrolled = true + } + + scroll.Scrolled(&fyne.ScrollEvent{Scrolled: fyne.NewDelta(-10, -10)}) + assert.True(t, scrolled) + scrolled = false + + scroll.Scrolled(&fyne.ScrollEvent{Scrolled: fyne.NewDelta(0, 0)}) + assert.False(t, scrolled) // don't repeat for no-change +} + func TestScrollContainer_SetMinSize_Direction(t *testing.T) { t.Run("Both", func(t *testing.T) { rect := canvas.NewRectangle(color.Black) @@ -181,7 +200,6 @@ func TestScrollContainer_Scrolled(t *testing.T) { scroll.Scrolled(&fyne.ScrollEvent{Scrolled: fyne.NewDelta(-10, -10)}) assert.Equal(t, float32(10), scroll.Offset.X) assert.Equal(t, float32(10), scroll.Offset.Y) - } func TestScrollContainer_Scrolled_Limit(t *testing.T) { From 9f657068d71586599dcd9ef5fd394d1f76176380 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Mon, 15 Nov 2021 21:53:51 +0000 Subject: [PATCH 2/5] Fix warning for unused vars --- container/doctabs.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/container/doctabs.go b/container/doctabs.go index 1a1e9dbb10..90f8958317 100644 --- a/container/doctabs.go +++ b/container/doctabs.go @@ -67,10 +67,7 @@ func (t *DocTabs) CreateRenderer() fyne.WidgetRenderer { r.action = r.buildAllTabsButton() r.create = r.buildCreateTabsButton() r.box = NewHBox(r.create, r.action) - var lastX, lastY float32 r.scroller.OnScrolled = func(offset fyne.Position) { - lastX = offset.X - lastY = offset.Y r.updateIndicator(false) } r.updateAllTabs() From 3a76e92715dd737e4fde5afe3b4449fb915f0497 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Thu, 16 Dec 2021 17:53:25 -0800 Subject: [PATCH 3/5] Fix lint mistake --- internal/widget/scroller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/widget/scroller.go b/internal/widget/scroller.go index 4117244620..7a3608db3b 100644 --- a/internal/widget/scroller.go +++ b/internal/widget/scroller.go @@ -217,7 +217,7 @@ func (a *scrollBarArea) moveBar(offset float32, barSize fyne.Size) { default: a.scroll.Offset.Y = a.computeScrollOffset(barSize.Height, offset, a.scroll.Size().Height, a.scroll.Content.Size().Height) } - if f := a.scroll.OnScrolled; f != nil && (a.scroll.Offset.X != oldX || a.scroll.Offset.Y != oldY){ + if f := a.scroll.OnScrolled; f != nil && (a.scroll.Offset.X != oldX || a.scroll.Offset.Y != oldY) { f(a.scroll.Offset) } a.scroll.refreshWithoutOffsetUpdate() From f06981706b2b862bdef1a42b3b4b722cfb76c6e9 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Thu, 16 Dec 2021 19:02:00 -0800 Subject: [PATCH 4/5] Fix various glitches caused by removing excess scroll events --- container/doctabs.go | 1 + internal/widget/scroller.go | 10 ++++++---- widget/table.go | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/container/doctabs.go b/container/doctabs.go index 90f8958317..c43088de1f 100644 --- a/container/doctabs.go +++ b/container/doctabs.go @@ -352,6 +352,7 @@ func (r *docTabsRenderer) scrollToSelected() { } } r.scroller.Offset = offset + r.updateIndicator(false) } func (r *docTabsRenderer) updateIndicator(animate bool) { diff --git a/internal/widget/scroller.go b/internal/widget/scroller.go index 7a3608db3b..410fd33dfe 100644 --- a/internal/widget/scroller.go +++ b/internal/widget/scroller.go @@ -392,14 +392,13 @@ func (s *Scroll) CreateRenderer() fyne.WidgetRenderer { //ScrollToBottom will scroll content to container bottom - to show latest info which end user just added func (s *Scroll) ScrollToBottom() { - s.Offset.Y = s.Content.MinSize().Height - s.Size().Height + s.scrollBy(0, -1*(s.Content.MinSize().Height - s.Size().Height - s.Offset.Y)) s.Refresh() } //ScrollToTop will scroll content to container top func (s *Scroll) ScrollToTop() { - s.Offset.Y = 0 - s.Refresh() + s.scrollBy(0, -s.Offset.Y) } // DragEnd will stop scrolling on mobile has stopped @@ -460,7 +459,10 @@ func (s *Scroll) refreshWithoutOffsetUpdate() { // Scrolled is called when an input device triggers a scroll event func (s *Scroll) Scrolled(ev *fyne.ScrollEvent) { - dx, dy := ev.Scrolled.DX, ev.Scrolled.DY + s.scrollBy(ev.Scrolled.DX, ev.Scrolled.DY) +} + +func (s *Scroll) scrollBy(dx, dy float32) { if s.Size().Width < s.Content.MinSize().Width && s.Size().Height >= s.Content.MinSize().Height && dx == 0 { dx, dy = dy, dx } diff --git a/widget/table.go b/widget/table.go index 4d30347f14..970f29bbd5 100644 --- a/widget/table.go +++ b/widget/table.go @@ -116,6 +116,7 @@ func (t *Table) SetColumnWidth(id int, width float32) { } t.columnWidths[id] = width t.Refresh() + t.scroll.Refresh() } // Unselect will mark the cell provided by id as unselected. From 6a0ba96a5f125fccf0952ab3035099e8a5a6567c Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Thu, 16 Dec 2021 19:07:35 -0800 Subject: [PATCH 5/5] Fix format --- internal/widget/scroller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/widget/scroller.go b/internal/widget/scroller.go index 410fd33dfe..d6f62f7371 100644 --- a/internal/widget/scroller.go +++ b/internal/widget/scroller.go @@ -392,7 +392,7 @@ func (s *Scroll) CreateRenderer() fyne.WidgetRenderer { //ScrollToBottom will scroll content to container bottom - to show latest info which end user just added func (s *Scroll) ScrollToBottom() { - s.scrollBy(0, -1*(s.Content.MinSize().Height - s.Size().Height - s.Offset.Y)) + s.scrollBy(0, -1*(s.Content.MinSize().Height-s.Size().Height-s.Offset.Y)) s.Refresh() }