Don't call OnScrolled if offset did not change

Fixes fyne-io#1868
andydotxyz · Nov 15, 2021 · 2b4bdda · 2b4bdda
1 parent 640b699
commit 2b4bdda
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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) {