Skip to content

Commit

Permalink
Don't create the renderer in the constructor for infinite progressbar
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Jacalz committed May 18, 2024
1 parent 6145be2 commit 26bd79a
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
6 changes: 3 additions & 3 deletions widget/progressbarinfinite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
14 changes: 13 additions & 1 deletion widget/progressbarinfinite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand All @@ -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()
Expand Down

0 comments on commit 26bd79a

Please sign in to comment.