From 31adea8ce347b54233d2ebefba10432eeae707ce Mon Sep 17 00:00:00 2001 From: Pablo Fuentes Date: Fri, 26 Feb 2021 05:43:30 -0500 Subject: [PATCH] Ensure Popups are resized in newly created window and when window is resized, fixes #1692 (#1932) * fix file dialog resize method, ensure popup overlays are refreshed when windows is created and resized * remove logic in baseDialog.Resize that is already delegated to widget.PopUp * remove unnecessary logic in popup.Resize * changes applied to mobileDriver and testCanvas fixes #1692 --- dialog/base.go | 16 +------- dialog/file.go | 16 +------- internal/driver/glfw/canvas.go | 2 +- internal/driver/glfw/canvas_test.go | 34 ++++++++++++---- internal/driver/gomobile/canvas.go | 3 +- internal/driver/gomobile/canvas_test.go | 20 +++++++++ test/testcanvas.go | 13 +++++- widget/popup.go | 4 -- widget/popup_test.go | 54 +++++++++++++++++++++++++ 9 files changed, 116 insertions(+), 46 deletions(-) diff --git a/dialog/base.go b/dialog/base.go index 7d89e47299..3c7c74266c 100644 --- a/dialog/base.go +++ b/dialog/base.go @@ -145,21 +145,7 @@ func (d *dialog) Refresh() { // Resize dialog, call this function after dialog show func (d *dialog) Resize(size fyne.Size) { d.desiredSize = size - maxSize := d.win.Size() - minSize := d.win.MinSize() - newWidth := size.Width - if size.Width > maxSize.Width { - newWidth = maxSize.Width - } else if size.Width < minSize.Width { - newWidth = minSize.Width - } - newHeight := size.Height - if size.Height > maxSize.Height { - newHeight = maxSize.Height - } else if size.Height < minSize.Height { - newHeight = minSize.Height - } - d.win.Resize(fyne.NewSize(newWidth, newHeight)) + d.win.Resize(size) } // SetDismissText allows custom text to be set in the confirmation button diff --git a/dialog/file.go b/dialog/file.go index 5ac471ab2d..247e57bb66 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -476,21 +476,7 @@ func (f *FileDialog) Resize(size fyne.Size) { if f.dialog == nil { return } - maxSize := f.dialog.win.Size() - minSize := f.dialog.win.MinSize() - newWidth := size.Width - if size.Width > maxSize.Width { - newWidth = maxSize.Width - } else if size.Width < minSize.Width { - newWidth = minSize.Width - } - newHeight := size.Height - if size.Height > maxSize.Height { - newHeight = maxSize.Height - } else if size.Height < minSize.Height { - newHeight = minSize.Height - } - f.dialog.win.Resize(fyne.NewSize(newWidth, newHeight)) + f.dialog.win.Resize(size) } // Hide hides the file dialog. diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index aa36d2ea7d..e2662b6637 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -182,7 +182,7 @@ func (c *glCanvas) Resize(size fyne.Size) { if p, ok := overlay.(*widget.PopUp); ok { // TODO: remove this when #707 is being addressed. // “Notifies” the PopUp of the canvas size change. - p.Resize(p.Content.Size().Add(fyne.NewSize(theme.Padding()*2, theme.Padding()*2))) + p.Refresh() } else { overlay.Resize(size) } diff --git a/internal/driver/glfw/canvas_test.go b/internal/driver/glfw/canvas_test.go index 06f5f7710c..4b9dc6d013 100644 --- a/internal/driver/glfw/canvas_test.go +++ b/internal/driver/glfw/canvas_test.go @@ -412,14 +412,8 @@ func TestGlCanvas_ResizeWithOverlays(t *testing.T) { o3 := widget.NewLabel("o3") w.SetContent(content) w.Canvas().Overlays().Add(o1) - // TODO: address #707; overlays should always be canvas size - o1.Resize(w.Canvas().Size()) w.Canvas().Overlays().Add(o2) - // TODO: address #707; overlays should always be canvas size - o2.Resize(w.Canvas().Size()) w.Canvas().Overlays().Add(o3) - // TODO: address #707; overlays should always be canvas size - o3.Resize(w.Canvas().Size()) size := fyne.NewSize(200, 100) assert.NotEqual(t, size, content.Size()) @@ -441,12 +435,12 @@ 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) + over.Show() size := fyne.NewSize(200, 100) overContentSize := over.Content.Size() + assert.NotZero(t, overContentSize) assert.NotEqual(t, size, content.Size()) assert.NotEqual(t, size, over.Size()) assert.NotEqual(t, size, overContentSize) @@ -457,6 +451,30 @@ func TestGlCanvas_ResizeWithPopUpOverlay(t *testing.T) { assert.Equal(t, overContentSize, over.Content.Size(), "canvas overlay content is _not_ resized") } +func TestGlCanvas_ResizeWithModalPopUpOverlay(t *testing.T) { + w := createWindow("Test") + w.SetPadded(false) + + content := widget.NewLabel("Content") + w.SetContent(content) + + popup := widget.NewModalPopUp(widget.NewLabel("PopUp"), w.Canvas()) + popupBgSize := fyne.NewSize(975, 575) + popup.Show() + popup.Resize(popupBgSize) + + winSize := fyne.NewSize(1000, 600) + w.Resize(winSize) + w.Show() + defer w.Close() + + // get popup content padding dynamically + popupContentPadding := popup.MinSize().Subtract(popup.Content.MinSize()) + + assert.Equal(t, popupBgSize.Subtract(popupContentPadding), popup.Content.Size()) + assert.Equal(t, winSize, popup.Size()) +} + func TestGlCanvas_Scale(t *testing.T) { w := createWindow("Test").(*window) c := w.Canvas().(*glCanvas) diff --git a/internal/driver/gomobile/canvas.go b/internal/driver/gomobile/canvas.go index eeab19cfad..b62fe7e045 100644 --- a/internal/driver/gomobile/canvas.go +++ b/internal/driver/gomobile/canvas.go @@ -340,8 +340,7 @@ func (c *mobileCanvas) sizeContent(size fyne.Size) { if p, ok := overlay.(*widget.PopUp); ok { // TODO: remove this when #707 is being addressed. // “Notifies” the PopUp of the canvas size change. - size := p.Content.Size().Add(fyne.NewSize(theme.Padding()*2, theme.Padding()*2)).Min(areaSize) - p.Resize(size) + p.Refresh() } else { overlay.Resize(areaSize) overlay.Move(topLeft) diff --git a/internal/driver/gomobile/canvas_test.go b/internal/driver/gomobile/canvas_test.go index 8d4cc37bcb..7929cfbe37 100644 --- a/internal/driver/gomobile/canvas_test.go +++ b/internal/driver/gomobile/canvas_test.go @@ -227,6 +227,26 @@ func TestWindow_TappedAndDoubleTapped(t *testing.T) { assert.Equal(t, 2, tapped) } +func TestGlCanvas_ResizeWithModalPopUpOverlay(t *testing.T) { + c := NewCanvas().(*mobileCanvas) + + c.SetContent(widget.NewLabel("Content")) + + popup := widget.NewModalPopUp(widget.NewLabel("PopUp"), c) + popupBgSize := fyne.NewSize(200, 200) + popup.Show() + popup.Resize(popupBgSize) + + canvasSize := fyne.NewSize(600, 700) + c.resize(canvasSize) + + // get popup content padding dynamically + popupContentPadding := popup.MinSize().Subtract(popup.Content.MinSize()) + + assert.Equal(t, popupBgSize.Subtract(popupContentPadding), popup.Content.Size()) + assert.Equal(t, canvasSize, popup.Size()) +} + func TestCanvas_Focusable(t *testing.T) { content := newFocusableEntry() c := NewCanvas().(*mobileCanvas) diff --git a/test/testcanvas.go b/test/testcanvas.go index 24ac67b32c..1bb95e3a1b 100644 --- a/test/testcanvas.go +++ b/test/testcanvas.go @@ -160,8 +160,19 @@ func (c *testCanvas) Resize(size fyne.Size) { return } + // Ensure testcanvas mimics real canvas.Resize behavior for _, overlay := range overlays.List() { - overlay.Resize(size) + type popupWidget interface { + fyne.CanvasObject + ShowAtPosition(fyne.Position) + } + if p, ok := overlay.(popupWidget); ok { + // TODO: remove this when #707 is being addressed. + // “Notifies” the PopUp of the canvas size change. + p.Refresh() + } else { + overlay.Resize(size) + } } if padded { diff --git a/widget/popup.go b/widget/popup.go index 955607160b..652af47dca 100644 --- a/widget/popup.go +++ b/widget/popup.go @@ -48,7 +48,6 @@ func (p *PopUp) Move(pos fyne.Position) { // Implements: fyne.Widget func (p *PopUp) Resize(size fyne.Size) { p.innerSize = size - p.BaseWidget.Resize(p.Canvas.Size()) // The canvas size might not have changed and therefore the Resize won't trigger a layout. // Until we have a widget.Relayout() or similar, the renderer's refresh will do the re-layout. p.Refresh() @@ -57,9 +56,6 @@ func (p *PopUp) Resize(size fyne.Size) { // Show this pop-up as overlay if not already shown. func (p *PopUp) Show() { if !p.overlayShown { - if p.Size().IsZero() { - p.Resize(p.MinSize()) - } p.Canvas.Overlays().Add(p) p.overlayShown = true } diff --git a/widget/popup_test.go b/widget/popup_test.go index 5356c31042..701ba37ae4 100644 --- a/widget/popup_test.go +++ b/widget/popup_test.go @@ -350,6 +350,33 @@ func TestPopUp_ResizeOnShow(t *testing.T) { pop.Hide() } +func TestPopUp_ResizeBeforeShow_CanvasSizeZero(t *testing.T) { + test.NewApp() + defer test.NewApp() + + // Simulate canvas size {0,0} + rect := canvas.NewRectangle(color.Black) + rect.SetMinSize(fyne.NewSize(0, 0)) + w := test.NewWindow(rect) + w.SetPadded(false) + w.Resize(fyne.NewSize(0, 0)) + assert.Zero(t, w.Canvas().Size()) + + pop := NewPopUp(NewLabel("Label"), w.Canvas()) + popBgSize := fyne.NewSize(200, 200) + pop.Resize(popBgSize) + pop.Show() + + winSize := fyne.NewSize(300, 300) + w.Resize(winSize) + + // get content padding dynamically + popContentPadding := pop.MinSize().Subtract(pop.Content.MinSize()) + + assert.Equal(t, popBgSize.Subtract(popContentPadding), pop.Content.Size()) + assert.Equal(t, winSize, pop.Size()) +} + func TestModalPopUp_Tapped(t *testing.T) { label := NewLabel("Hi") pop := NewModalPopUp(label, test.Canvas()) @@ -451,3 +478,30 @@ func TestModalPopUp_ResizeOnShow(t *testing.T) { assert.Equal(t, size, pop.Size()) pop.Hide() } + +func TestModelPopUp_ResizeBeforeShow_CanvasSizeZero(t *testing.T) { + test.NewApp() + defer test.NewApp() + + // Simulate canvas size {0,0} + rect := canvas.NewRectangle(color.Black) + rect.SetMinSize(fyne.NewSize(0, 0)) + w := test.NewWindow(rect) + w.SetPadded(false) + w.Resize(fyne.NewSize(0, 0)) + assert.Zero(t, w.Canvas().Size()) + + pop := NewModalPopUp(NewLabel("Label"), w.Canvas()) + popBgSize := fyne.NewSize(200, 200) + pop.Resize(popBgSize) + pop.Show() + + winSize := fyne.NewSize(300, 300) + w.Resize(winSize) + + // get content padding dynamically + popContentPadding := pop.MinSize().Subtract(pop.Content.MinSize()) + + assert.Equal(t, popBgSize.Subtract(popContentPadding), pop.Content.Size()) + assert.Equal(t, winSize, pop.Size()) +}