From 26bd79ae39a19352b5e66323530ee2451bc86dee Mon Sep 17 00:00:00 2001 From: Jacob Date: Sat, 18 May 2024 18:33:43 +0200 Subject: [PATCH] Don't create the renderer in the constructor for infinite progressbar This fixes the infinite progress bar code to not create the renderer inside the constrcutor. The old behaviour was not only a potential performance and memory leak and just seems generally incorrect from a best practices standpoint. The tests had to be modified to actually render the object so this is a change in behaviour but I would not call it breaking given that I consider the old behaviour a incorrect and a bug. --- widget/progressbarinfinite.go | 6 +++--- widget/progressbarinfinite_test.go | 14 +++++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/widget/progressbarinfinite.go b/widget/progressbarinfinite.go index 86fb38d066..93faf699f7 100644 --- a/widget/progressbarinfinite.go +++ b/widget/progressbarinfinite.go @@ -193,7 +193,7 @@ func (p *ProgressBarInfinite) CreateRenderer() fyne.WidgetRenderer { // SetValue() is not defined for infinite progress bar // To stop the looping progress and set the progress bar to 100%, call ProgressBarInfinite.Stop() func NewProgressBarInfinite() *ProgressBarInfinite { - p := &ProgressBarInfinite{} - cache.Renderer(p).Layout(p.MinSize()) - return p + bar := &ProgressBarInfinite{} + bar.ExtendBaseWidget(bar) + return bar } diff --git a/widget/progressbarinfinite_test.go b/widget/progressbarinfinite_test.go index fe0d50797d..cecb4e617b 100644 --- a/widget/progressbarinfinite_test.go +++ b/widget/progressbarinfinite_test.go @@ -27,12 +27,22 @@ func BenchmarkProgressbarInf(b *testing.B) { func TestProgressBarInfinite_Creation(t *testing.T) { bar := NewProgressBarInfinite() - // ticker should start automatically + + // ticker should be disabled until the widget is shown + assert.False(t, bar.Running()) + + win := test.NewWindow(bar) + win.Show() + + // ticker should start automatically once the renderer is created assert.True(t, bar.Running()) } func TestProgressBarInfinite_Destroy(t *testing.T) { bar := NewProgressBarInfinite() + win := test.NewWindow(bar) + win.Show() + assert.True(t, cache.IsRendered(bar)) assert.True(t, bar.Running()) @@ -46,6 +56,8 @@ func TestProgressBarInfinite_Destroy(t *testing.T) { func TestProgressBarInfinite_Reshown(t *testing.T) { bar := NewProgressBarInfinite() + win := test.NewWindow(bar) + win.Show() assert.True(t, bar.Running()) bar.Hide()