From a7abe93f32b113fc642de6c2dfa1bc8dfc687e8f Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Tue, 9 Feb 2021 17:38:13 -0500 Subject: [PATCH 1/8] fix file dialog resize method, ensure popup overlays are refreshed when windows is created and resized, fixes #1692 when #1931 is landed --- dialog/file.go | 30 ++++++++++++++++-------------- internal/driver/glfw/canvas.go | 3 ++- internal/driver/glfw/window.go | 8 ++++++++ widget/popup.go | 4 +++- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index 5ac471ab2d..98ac051f24 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -476,21 +476,23 @@ 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(minSize.Max(size)) + // 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)) } // Hide hides the file dialog. diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index 265763cc3f..9b3a2e605e 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -178,7 +178,8 @@ 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))) + // fmt.Printf("%#v ,canvas resize: %v\n", p, p.Content.Size()) + p.Refresh() } else { overlay.Resize(size) } diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index baf63bffbd..f734c220b3 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -402,6 +402,14 @@ func (w *window) doShow() { if w.canvas.Content() != nil { w.canvas.Content().Show() } + + // Refresh popup overlays + for _, overlay := range w.canvas.Overlays().List() { + if _, ok := overlay.(*widget.PopUp); !ok { + continue + } + overlay.Refresh() + } } func (w *window) Hide() { diff --git a/widget/popup.go b/widget/popup.go index 848f33e321..799e3bb32f 100644 --- a/widget/popup.go +++ b/widget/popup.go @@ -47,8 +47,10 @@ func (p *PopUp) Move(pos fyne.Position) { // // Implements: fyne.Widget func (p *PopUp) Resize(size fyne.Size) { + canvasSize := p.Canvas.Size() p.innerSize = size - p.BaseWidget.Resize(p.Canvas.Size()) + p.Content.Resize(p.innerSize.Subtract(fyne.NewSize(theme.Padding()*2, theme.Padding()*2))) + p.BaseWidget.Resize(canvasSize) // 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() From 19a2371ad10f8bc9d83c6befe23b0fe7066b49c1 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Tue, 23 Feb 2021 09:54:31 -0500 Subject: [PATCH 2/8] remove commented out code --- dialog/file.go | 15 --------------- internal/driver/glfw/canvas.go | 1 - 2 files changed, 16 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index 2cc5e27fb9..d657352125 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -485,21 +485,6 @@ func (f *FileDialog) Resize(size fyne.Size) { } minSize := f.dialog.win.MinSize() f.dialog.win.Resize(minSize.Max(size)) - // 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)) } // Hide hides the file dialog. diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index 8659afb73c..ae6c305fe2 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -182,7 +182,6 @@ 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. - // fmt.Printf("%#v ,canvas resize: %v\n", p, p.Content.Size()) p.Refresh() } else { overlay.Resize(size) From 06fea4125dcf628c3a3488c916e760c996039727 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Tue, 23 Feb 2021 10:43:40 -0500 Subject: [PATCH 3/8] remove unnecessary logic in popup.Resize --- widget/popup.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/widget/popup.go b/widget/popup.go index 5bba85c0e7..8de2f39fd5 100644 --- a/widget/popup.go +++ b/widget/popup.go @@ -47,10 +47,7 @@ func (p *PopUp) Move(pos fyne.Position) { // // Implements: fyne.Widget func (p *PopUp) Resize(size fyne.Size) { - canvasSize := p.Canvas.Size() p.innerSize = size - p.Content.Resize(p.innerSize.Subtract(fyne.NewSize(theme.Padding()*2, theme.Padding()*2))) - p.BaseWidget.Resize(canvasSize) // 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() From 5a57d1057836cb5d7fb237e83540944a03c8fb5d Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Tue, 23 Feb 2021 11:23:03 -0500 Subject: [PATCH 4/8] remove logic in dialog.Resize that is already delegated to widget.PopUp --- dialog/file.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dialog/file.go b/dialog/file.go index d657352125..3162c7722a 100644 --- a/dialog/file.go +++ b/dialog/file.go @@ -483,8 +483,7 @@ func (f *FileDialog) Resize(size fyne.Size) { if f.dialog == nil { return } - minSize := f.dialog.win.MinSize() - f.dialog.win.Resize(minSize.Max(size)) + f.dialog.win.Resize(size) } // Hide hides the file dialog. From e8d6f9272d9e04f7c232107e2316d465b5e47818 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Wed, 24 Feb 2021 00:48:56 -0500 Subject: [PATCH 5/8] remove logic in baseDialog.Resize that is already delegated to widget.PopUp --- dialog/base.go | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/dialog/base.go b/dialog/base.go index ff863f910b..6bd19d0374 100644 --- a/dialog/base.go +++ b/dialog/base.go @@ -126,21 +126,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 From f5bee9006197d22743a19a6f6a23e2acffa63d33 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Wed, 24 Feb 2021 01:24:12 -0500 Subject: [PATCH 6/8] changes applied to mobileDriver too, removed unnecessary popup refresh call (previously added) on window.Show. --- internal/driver/glfw/window.go | 8 -------- internal/driver/gomobile/canvas.go | 3 +-- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index e08e1da871..01e0ac4cca 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -403,14 +403,6 @@ func (w *window) doShow() { if w.canvas.Content() != nil { w.canvas.Content().Show() } - - // Refresh popup overlays - for _, overlay := range w.canvas.Overlays().List() { - if _, ok := overlay.(*widget.PopUp); !ok { - continue - } - overlay.Refresh() - } } func (w *window) Hide() { 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) From 6c71bf586574f61dc5dcb24f391f0a4b3a73e678 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Wed, 24 Feb 2021 11:08:54 -0500 Subject: [PATCH 7/8] added canvas tests for desktop and mobile --- internal/driver/glfw/canvas_test.go | 34 +++++++++++++++++++------ internal/driver/gomobile/canvas_test.go | 20 +++++++++++++++ 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/internal/driver/glfw/canvas_test.go b/internal/driver/glfw/canvas_test.go index 694288f104..64c1402dd8 100644 --- a/internal/driver/glfw/canvas_test.go +++ b/internal/driver/glfw/canvas_test.go @@ -422,14 +422,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()) @@ -451,12 +445,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) @@ -467,6 +461,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_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) From 0cbe4342fa085c81a0618e25fe9c146f91aaae78 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Wed, 24 Feb 2021 11:34:02 -0500 Subject: [PATCH 8/8] ensure testcanvas mimics real canvas.Resize behavior, remove conflicting code in popup.Show, added popup tests --- test/testcanvas.go | 13 ++++++++++- widget/popup.go | 3 --- widget/popup_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 4 deletions(-) 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 8de2f39fd5..652af47dca 100644 --- a/widget/popup.go +++ b/widget/popup.go @@ -56,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()) +}