Refresh dialogs correctly, fixes #1866 (#1931)

* fix modalPopUpRenderer refresh and refresh dialog when it is shown to fix #1866

* update themedBackgroundRenderer functions to respect fyne style guidelines

dialog/base.go

@@ -40,7 +40,7 @@ type dialog struct {
 	desiredSize fyne.Size
 
 	win            *widget.PopUp
-	bg             *canvas.Rectangle
+	bg             *themedBackground
 	content, label fyne.CanvasObject
 	dismiss        *widget.Button
 	parent         fyne.Window
@@ -120,7 +120,6 @@ func (d *dialog) Show() {
 }
 
 func (d *dialog) Refresh() {
-	d.applyTheme()
 	d.win.Refresh()
 }
 
@@ -164,12 +163,6 @@ func (d *dialog) SetOnClosed(closed func()) {
 	}
 }
 
-func (d *dialog) applyTheme() {
-	r, g, b, _ := theme.BackgroundColor().RGBA()
-	bg := &color.NRGBA{R: uint8(r), G: uint8(g), B: uint8(b), A: 230}
-	d.bg.FillColor = bg
-}
-
 func (d *dialog) hideWithResponse(resp bool) {
 	d.win.Hide()
 	if d.callback != nil {
@@ -178,8 +171,8 @@ func (d *dialog) hideWithResponse(resp bool) {
 }
 
 func (d *dialog) setButtons(buttons fyne.CanvasObject) {
-	d.bg = canvas.NewRectangle(theme.BackgroundColor())
-	d.label = newDialogTitle(d.title, d)
+	d.bg = newThemedBackground()
+	d.label = widget.NewLabelWithStyle(d.title, fyne.TextAlignLeading, fyne.TextStyle{Bold: true})
 
 	var content fyne.CanvasObject
 	if d.icon == nil {
@@ -227,31 +220,56 @@ func newButtonList(buttons ...*widget.Button) fyne.CanvasObject {
 	return list
 }
 
-// dialogTitle is really just a normal title but we use the Refresh() hook to update the background rectangle.
-type dialogTitle struct {
-	widget.Label
+// ===============================================================
+// ThemedBackground
+// ===============================================================
 
-	d *dialog
+type themedBackground struct {
+	widget.BaseWidget
+}
+
+func newThemedBackground() *themedBackground {
+	t := &themedBackground{}
+	t.ExtendBaseWidget(t)
+	return t
+}
+
+func (t *themedBackground) CreateRenderer() fyne.WidgetRenderer {
+	t.ExtendBaseWidget(t)
+	rect := canvas.NewRectangle(theme.BackgroundColor())
+	return &themedBackgroundRenderer{rect, []fyne.CanvasObject{rect}}
 }
 
-// Refresh applies the current theme to the whole dialog before refreshing the underlying label.
-func (t *dialogTitle) Refresh() {
-	t.d.Refresh()
+type themedBackgroundRenderer struct {
+	rect    *canvas.Rectangle
+	objects []fyne.CanvasObject
+}
 
-	t.BaseWidget.Refresh()
+func (renderer *themedBackgroundRenderer) Destroy() {
 }
 
-func newDialogTitle(title string, d *dialog) *dialogTitle {
-	l := &dialogTitle{}
-	l.Text = title
-	l.Alignment = fyne.TextAlignLeading
-	l.TextStyle.Bold = true
+func (renderer *themedBackgroundRenderer) Layout(size fyne.Size) {
+	renderer.rect.Resize(size)
+}
 
-	l.d = d
-	l.ExtendBaseWidget(l)
-	return l
+func (renderer *themedBackgroundRenderer) MinSize() fyne.Size {
+	return renderer.rect.MinSize()
 }
 
+func (renderer *themedBackgroundRenderer) Objects() []fyne.CanvasObject {
+	return renderer.objects
+}
+
+func (renderer *themedBackgroundRenderer) Refresh() {
+	r, g, b, _ := theme.BackgroundColor().RGBA()
+	bg := &color.NRGBA{R: uint8(r), G: uint8(g), B: uint8(b), A: 230}
+	renderer.rect.FillColor = bg
+}
+
+// ===============================================================
+// DialogLayout
+// ===============================================================
+
 type dialogLayout struct {
 	d *dialog
 }
@@ -260,7 +278,7 @@ func (l *dialogLayout) Layout(obj []fyne.CanvasObject, size fyne.Size) {
 	l.d.bg.Move(fyne.NewPos(0, 0))
 	l.d.bg.Resize(size)
 
-	btnMin := obj[3].MinSize().Max(obj[3].Size())
+	btnMin := obj[3].MinSize()
 
 	// icon
 	iconHeight := padHeight*2 + l.d.label.MinSize().Height*2 - theme.Padding()
@@ -280,7 +298,7 @@ func (l *dialogLayout) Layout(obj []fyne.CanvasObject, size fyne.Size) {
 
 func (l *dialogLayout) MinSize(obj []fyne.CanvasObject) fyne.Size {
 	contentMin := obj[2].MinSize()
-	btnMin := obj[3].MinSize().Max(obj[3].Size())
+	btnMin := obj[3].MinSize()
 
 	width := fyne.Max(fyne.Max(contentMin.Width, btnMin.Width), obj[4].MinSize().Width) + padWidth
 	height := contentMin.Height + btnMin.Height + l.d.label.MinSize().Height + theme.Padding() + padHeight*2

dialog/base_test.go

@@ -48,3 +48,51 @@ func TestShowCustom_Resize(t *testing.T) {
 	d.Show()
 	assert.Equal(t, size, d.(*dialog).win.Content.Size().Add(fyne.NewSize(theme.Padding()*2, theme.Padding()*2)))
 }
+
+func TestCustom_ApplyThemeOnShow(t *testing.T) {
+	test.NewApp()
+	defer test.NewApp()
+	w := test.NewWindow(canvas.NewRectangle(color.Transparent))
+	w.Resize(fyne.NewSize(200, 300))
+
+	label := widget.NewLabel("Content")
+	label.Alignment = fyne.TextAlignCenter
+	d := NewCustom("Title", "OK", label, w)
+
+	test.ApplyTheme(t, test.Theme())
+	d.Show()
+	test.AssertImageMatches(t, "dialog-onshow-theme-default.png", w.Canvas().Capture())
+	d.Hide()
+
+	test.ApplyTheme(t, test.NewTheme())
+	d.Show()
+	test.AssertImageMatches(t, "dialog-onshow-theme-changed.png", w.Canvas().Capture())
+	d.Hide()
+
+	test.ApplyTheme(t, test.Theme())
+	d.Show()
+	test.AssertImageMatches(t, "dialog-onshow-theme-default.png", w.Canvas().Capture())
+	d.Hide()
+}
+
+func TestCustom_ResizeOnShow(t *testing.T) {
+	test.NewApp()
+	defer test.NewApp()
+	w := test.NewWindow(canvas.NewRectangle(color.Transparent))
+	size := fyne.NewSize(200, 300)
+	w.Resize(size)
+
+	label := widget.NewLabel("Content")
+	label.Alignment = fyne.TextAlignCenter
+	d := NewCustom("Title", "OK", label, w).(*dialog)
+
+	d.Show()
+	assert.Equal(t, size, d.win.Size())
+	d.Hide()
+
+	size = fyne.NewSize(500, 500)
+	w.Resize(size)
+	d.Show()
+	assert.Equal(t, size, d.win.Size())
+	d.Hide()
+}

dialog/file_test.go

@@ -387,7 +387,7 @@ func TestFileFilters(t *testing.T) {
 			count++
 		}
 	}
-	assert.Equal(t, 3, count)
+	assert.Equal(t, 5, count)
 
 	f.SetFilter(storage.NewMimeTypeFileFilter([]string{"image/jpeg"}))
 
@@ -412,7 +412,7 @@ func TestFileFilters(t *testing.T) {
 			count++
 		}
 	}
-	assert.Equal(t, 4, count)
+	assert.Equal(t, 6, count)
 }
 
 func TestFileFavorites(t *testing.T) {

dialog/progress.go

@@ -1,7 +1,11 @@
 package dialog
 
 import (
+	"image/color"
+
 	"fyne.io/fyne/v2"
+	"fyne.io/fyne/v2/canvas"
+	"fyne.io/fyne/v2/container"
 	"fyne.io/fyne/v2/theme"
 	"fyne.io/fyne/v2/widget"
 )
@@ -27,8 +31,9 @@ func (p *ProgressDialog) SetValue(v float64) {
 func NewProgress(title, message string, parent fyne.Window) *ProgressDialog {
 	d := newDialog(title, message, theme.InfoIcon(), nil /*cancel?*/, parent)
 	bar := widget.NewProgressBar()
-	bar.Resize(fyne.NewSize(200, bar.MinSize().Height))
+	rect := canvas.NewRectangle(color.Transparent)
+	rect.SetMinSize(fyne.NewSize(200, 0))
 
-	d.setButtons(bar)
+	d.setButtons(container.NewMax(rect, bar))
 	return &ProgressDialog{d, bar}
 }

dialog/progressinfinite.go

@@ -1,7 +1,11 @@
 package dialog
 
 import (
+	"image/color"
+
 	"fyne.io/fyne/v2"
+	"fyne.io/fyne/v2/canvas"
+	"fyne.io/fyne/v2/container"
 	"fyne.io/fyne/v2/theme"
 	"fyne.io/fyne/v2/widget"
 )
@@ -22,9 +26,10 @@ type ProgressInfiniteDialog struct {
 func NewProgressInfinite(title, message string, parent fyne.Window) *ProgressInfiniteDialog {
 	d := newDialog(title, message, theme.InfoIcon(), nil /*cancel?*/, parent)
 	bar := widget.NewProgressBarInfinite()
-	bar.Resize(fyne.NewSize(200, bar.MinSize().Height))
+	rect := canvas.NewRectangle(color.Transparent)
+	rect.SetMinSize(fyne.NewSize(200, 0))
 
-	d.setButtons(bar)
+	d.setButtons(container.NewMax(rect, bar))
 	return &ProgressInfiniteDialog{d, bar}
 }
 

internal/driver/glfw/canvas_test.go

@@ -451,6 +451,7 @@ func TestGlCanvas_ResizeWithPopUpOverlay(t *testing.T) {
 
 	content := widget.NewLabel("Content")
 	over := widget.NewPopUp(widget.NewLabel("Over"), w.Canvas())
+	over.Show() // to ensure popup content size is not zero
 	w.SetContent(content)
 	w.Canvas().Overlays().Add(over)
 

widget/popup.go

@@ -63,6 +63,7 @@ func (p *PopUp) Show() {
 		p.Canvas.Overlays().Add(p)
 		p.overlayShown = true
 	}
+	p.Refresh()
 	p.BaseWidget.Show()
 }
 
@@ -172,26 +173,27 @@ type popUpRenderer struct {
 }
 
 func (r *popUpRenderer) Layout(_ fyne.Size) {
-	r.popUp.Content.Resize(r.popUp.innerSize.Subtract(r.padding()))
+	innerSize := r.popUp.innerSize.Max(r.popUp.MinSize())
+	r.popUp.Content.Resize(innerSize.Subtract(r.padding()))
 
 	innerPos := r.popUp.innerPos
-	if innerPos.X+r.popUp.innerSize.Width > r.popUp.Canvas.Size().Width {
-		innerPos.X = r.popUp.Canvas.Size().Width - r.popUp.innerSize.Width
+	if innerPos.X+innerSize.Width > r.popUp.Canvas.Size().Width {
+		innerPos.X = r.popUp.Canvas.Size().Width - innerSize.Width
 		if innerPos.X < 0 {
 			innerPos.X = 0 // TODO here we may need a scroller as it's wider than our canvas
 		}
 	}
-	if innerPos.Y+r.popUp.innerSize.Height > r.popUp.Canvas.Size().Height {
-		innerPos.Y = r.popUp.Canvas.Size().Height - r.popUp.innerSize.Height
+	if innerPos.Y+innerSize.Height > r.popUp.Canvas.Size().Height {
+		innerPos.Y = r.popUp.Canvas.Size().Height - innerSize.Height
 		if innerPos.Y < 0 {
 			innerPos.Y = 0 // TODO here we may need a scroller as it's longer than our canvas
 		}
 	}
 	r.popUp.Content.Move(innerPos.Add(r.offset()))
 
-	r.background.Resize(r.popUp.innerSize)
+	r.background.Resize(innerSize)
 	r.background.Move(innerPos)
-	r.LayoutShadow(r.popUp.innerSize, innerPos)
+	r.LayoutShadow(innerSize, innerPos)
 }
 
 func (r *popUpRenderer) MinSize() fyne.Size {
@@ -200,9 +202,16 @@ func (r *popUpRenderer) MinSize() fyne.Size {
 
 func (r *popUpRenderer) Refresh() {
 	r.background.FillColor = theme.BackgroundColor()
-	if r.background.Size() != r.popUp.innerSize || r.background.Position() != r.popUp.innerPos {
+	expectedContentSize := r.popUp.innerSize.Max(r.popUp.MinSize()).Subtract(r.padding())
+	shouldRelayout := !r.popUp.Content.Size().Subtract(expectedContentSize).IsZero()
+
+	if r.background.Size() != r.popUp.innerSize || r.background.Position() != r.popUp.innerPos || shouldRelayout {
 		r.Layout(r.popUp.Size())
 	}
+	if !r.popUp.Canvas.Size().Subtract(r.popUp.BaseWidget.Size()).IsZero() {
+		r.popUp.BaseWidget.Resize(r.popUp.Canvas.Size())
+	}
+	r.popUp.Content.Refresh()
 	r.background.Refresh()
 }
 
@@ -238,8 +247,15 @@ func (r *modalPopUpRenderer) MinSize() fyne.Size {
 func (r *modalPopUpRenderer) Refresh() {
 	r.underlay.FillColor = theme.ShadowColor()
 	r.background.FillColor = theme.BackgroundColor()
-	if r.background.Size() != r.popUp.innerSize {
+	expectedContentSize := r.popUp.innerSize.Max(r.popUp.MinSize()).Subtract(r.padding())
+	shouldRelayout := !r.popUp.Content.Size().Subtract(expectedContentSize).IsZero()
+
+	if r.background.Size() != r.popUp.innerSize || shouldRelayout {
 		r.Layout(r.popUp.Size())
 	}
+	if !r.popUp.Canvas.Size().Subtract(r.popUp.BaseWidget.Size()).IsZero() {
+		r.popUp.BaseWidget.Resize(r.popUp.Canvas.Size())
+	}
+	r.popUp.Content.Refresh()
 	r.background.Refresh()
 }