From 3d75849ae2b153063c2f837b9da0bb53f8c66b7c Mon Sep 17 00:00:00 2001 From: ole108 Date: Tue, 5 Nov 2024 16:53:23 +0100 Subject: [PATCH 1/5] Call color picker callback if dismissed --- dialog/color.go | 3 +++ dialog/color_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/dialog/color.go b/dialog/color.go index 50a4dfcef1..35d662f3e0 100644 --- a/dialog/color.go +++ b/dialog/color.go @@ -135,6 +135,9 @@ func (p *ColorPickerDialog) updateUI() { p.selectColor(p.color) }, } + p.dialog.dismiss.OnTapped = func() { + p.callback(nil) + } p.dialog.create(container.NewGridWithColumns(2, p.dialog.dismiss, confirm)) } else { p.dialog.content = container.NewVBox(p.createSimplePickers()...) diff --git a/dialog/color_test.go b/dialog/color_test.go index b60eed07a5..7869be76cc 100644 --- a/dialog/color_test.go +++ b/dialog/color_test.go @@ -93,6 +93,54 @@ func TestColorDialog_SetColor(t *testing.T) { d.Show() } +func TestColorDialog_Buttons(t *testing.T) { + for name, tt := range map[string]struct { + action func(d *ColorPickerDialog, w fyne.Window) + expectedCalled bool + expectedColored bool + }{ + "confirm": { + action: func(d *ColorPickerDialog, w fyne.Window) { + test.FocusNext(w.Canvas()) // advanced accordion + test.FocusNext(w.Canvas()) // dismiss button + test.FocusNext(w.Canvas()) // confirm button + w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) + }, + expectedCalled: true, + expectedColored: true, + }, + "dismiss": { + action: func(d *ColorPickerDialog, w fyne.Window) { + test.FocusNext(w.Canvas()) // advanced accordion + test.FocusNext(w.Canvas()) // dismiss button + w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) + }, + expectedCalled: true, + expectedColored: false, + }, + } { + t.Run(name, func(t *testing.T) { + test.NewTempApp(t) + w := test.NewTempWindow(t, canvas.NewRectangle(color.Transparent)) + w.Resize(fyne.NewSize(800, 600)) + + called := false + colored := false + d := NewColorPicker("Color Picker", "Pick a Color", func(c color.Color) { + called = true + colored = c != nil + }, w) + d.Advanced = true + d.Refresh() + d.Show() + tt.action(d, w) + + assert.Equal(t, tt.expectedCalled, called, "called isn't equal") + assert.Equal(t, tt.expectedColored, colored, "colored isn't equal") + }) + } +} + func TestColorDialogSimple_Theme(t *testing.T) { test.NewTempApp(t) From f26945b95e2d1d21831dd0b1675a8880fb1c97f4 Mon Sep 17 00:00:00 2001 From: ole108 Date: Tue, 5 Nov 2024 17:12:28 +0100 Subject: [PATCH 2/5] Call callback if dismissed in simple case too --- dialog/color.go | 3 +++ dialog/color_test.go | 30 +++++++++++++++++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/dialog/color.go b/dialog/color.go index 35d662f3e0..3cb8369228 100644 --- a/dialog/color.go +++ b/dialog/color.go @@ -141,6 +141,9 @@ func (p *ColorPickerDialog) updateUI() { p.dialog.create(container.NewGridWithColumns(2, p.dialog.dismiss, confirm)) } else { p.dialog.content = container.NewVBox(p.createSimplePickers()...) + p.dialog.dismiss.OnTapped = func() { + p.callback(nil) + } p.dialog.create(container.NewGridWithColumns(1, p.dialog.dismiss)) } } diff --git a/dialog/color_test.go b/dialog/color_test.go index 7869be76cc..0b7d2dde7b 100644 --- a/dialog/color_test.go +++ b/dialog/color_test.go @@ -95,26 +95,50 @@ func TestColorDialog_SetColor(t *testing.T) { func TestColorDialog_Buttons(t *testing.T) { for name, tt := range map[string]struct { - action func(d *ColorPickerDialog, w fyne.Window) + action func(d *ColorPickerDialog, w fyne.Window) + advanced bool expectedCalled bool expectedColored bool }{ - "confirm": { + "confirmAdvanced": { action: func(d *ColorPickerDialog, w fyne.Window) { test.FocusNext(w.Canvas()) // advanced accordion test.FocusNext(w.Canvas()) // dismiss button test.FocusNext(w.Canvas()) // confirm button w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) }, + advanced: true, expectedCalled: true, expectedColored: true, }, - "dismiss": { + "dismissAdvanced": { action: func(d *ColorPickerDialog, w fyne.Window) { test.FocusNext(w.Canvas()) // advanced accordion test.FocusNext(w.Canvas()) // dismiss button w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) }, + advanced: true, + expectedCalled: true, + expectedColored: false, + }, + "confirmSimple": { + action: func(d *ColorPickerDialog, w fyne.Window) { + test.FocusNext(w.Canvas()) // advanced accordion + test.FocusNext(w.Canvas()) // dismiss button + test.FocusNext(w.Canvas()) // confirm button + w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) + }, + advanced: false, + expectedCalled: true, + expectedColored: true, + }, + "dismissSimple": { + action: func(d *ColorPickerDialog, w fyne.Window) { + test.FocusNext(w.Canvas()) // advanced accordion + test.FocusNext(w.Canvas()) // dismiss button + w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) + }, + advanced: false, expectedCalled: true, expectedColored: false, }, From cd71d5b3c809c453c73debe46ea403e18b072bfa Mon Sep 17 00:00:00 2001 From: ole108 Date: Tue, 5 Nov 2024 17:25:18 +0100 Subject: [PATCH 3/5] Also test simple case --- dialog/color_test.go | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/dialog/color_test.go b/dialog/color_test.go index 0b7d2dde7b..c0802c50bc 100644 --- a/dialog/color_test.go +++ b/dialog/color_test.go @@ -121,20 +121,8 @@ func TestColorDialog_Buttons(t *testing.T) { expectedCalled: true, expectedColored: false, }, - "confirmSimple": { - action: func(d *ColorPickerDialog, w fyne.Window) { - test.FocusNext(w.Canvas()) // advanced accordion - test.FocusNext(w.Canvas()) // dismiss button - test.FocusNext(w.Canvas()) // confirm button - w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) - }, - advanced: false, - expectedCalled: true, - expectedColored: true, - }, "dismissSimple": { action: func(d *ColorPickerDialog, w fyne.Window) { - test.FocusNext(w.Canvas()) // advanced accordion test.FocusNext(w.Canvas()) // dismiss button w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) }, @@ -154,7 +142,7 @@ func TestColorDialog_Buttons(t *testing.T) { called = true colored = c != nil }, w) - d.Advanced = true + d.Advanced = tt.advanced d.Refresh() d.Show() tt.action(d, w) From 6661a819d10b5ab0bc74306bb2d4b21c40938a1a Mon Sep 17 00:00:00 2001 From: ole108 Date: Thu, 7 Nov 2024 17:54:37 +0100 Subject: [PATCH 4/5] Fix color picker callback order. --- dialog/color.go | 16 ++++++++-------- dialog/color_test.go | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/dialog/color.go b/dialog/color.go index 3cb8369228..4b30114a0f 100644 --- a/dialog/color.go +++ b/dialog/color.go @@ -91,7 +91,9 @@ func (p *ColorPickerDialog) createSimplePickers() (contents []fyne.CanvasObject) } func (p *ColorPickerDialog) selectColor(c color.Color) { - p.dialog.Hide() + if w := p.win; w != nil { + w.Hide() + } writeRecentColor(colorToString(c)) if p.picker != nil { p.picker.SetColor(c) @@ -99,6 +101,7 @@ func (p *ColorPickerDialog) selectColor(c color.Color) { if f := p.callback; f != nil { f(c) } + p.dialog.Hide() p.updateUI() } @@ -107,7 +110,10 @@ func (p *ColorPickerDialog) updateUI() { w.Hide() } p.dialog.dismiss = &widget.Button{Text: lang.L("Cancel"), Icon: theme.CancelIcon(), - OnTapped: p.dialog.Hide, + OnTapped: func() { + p.callback(nil) + p.dialog.Hide() + }, } if p.Advanced { p.picker = newColorAdvancedPicker(p.color, func(c color.Color) { @@ -135,15 +141,9 @@ func (p *ColorPickerDialog) updateUI() { p.selectColor(p.color) }, } - p.dialog.dismiss.OnTapped = func() { - p.callback(nil) - } p.dialog.create(container.NewGridWithColumns(2, p.dialog.dismiss, confirm)) } else { p.dialog.content = container.NewVBox(p.createSimplePickers()...) - p.dialog.dismiss.OnTapped = func() { - p.callback(nil) - } p.dialog.create(container.NewGridWithColumns(1, p.dialog.dismiss)) } } diff --git a/dialog/color_test.go b/dialog/color_test.go index c0802c50bc..479eb15ca0 100644 --- a/dialog/color_test.go +++ b/dialog/color_test.go @@ -153,6 +153,25 @@ func TestColorDialog_Buttons(t *testing.T) { } } +func TestColorDialog_DoubleCallback(t *testing.T) { + ch := make(chan int) + d := NewColorPicker("Color Picker", "Pick a Color", func(_ color.Color) { + ch <- 42 + }, test.NewTempWindow(t, nil)) + d.SetOnClosed(func() { + ch <- 43 + }) + d.Refresh() + d.Show() + + assert.False(t, d.win.Hidden) + go test.Tap(d.dismiss) + assert.EqualValues(t, <-ch, 42) + assert.EqualValues(t, <-ch, 43) + assert.True(t, d.win.Hidden) +} + + func TestColorDialogSimple_Theme(t *testing.T) { test.NewTempApp(t) From fce2b2f721e6ec47bc98149229477992d72e2ce0 Mon Sep 17 00:00:00 2001 From: ole108 Date: Sun, 10 Nov 2024 15:26:10 +0100 Subject: [PATCH 5/5] Remove call of callback with nil argument --- dialog/color.go | 5 +--- dialog/color_test.go | 63 +++++++++++++++++++++++++++++--------------- 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/dialog/color.go b/dialog/color.go index 4b30114a0f..9e07dad7c9 100644 --- a/dialog/color.go +++ b/dialog/color.go @@ -110,10 +110,7 @@ func (p *ColorPickerDialog) updateUI() { w.Hide() } p.dialog.dismiss = &widget.Button{Text: lang.L("Cancel"), Icon: theme.CancelIcon(), - OnTapped: func() { - p.callback(nil) - p.dialog.Hide() - }, + OnTapped: p.dialog.Hide, } if p.Advanced { p.picker = newColorAdvancedPicker(p.color, func(c color.Color) { diff --git a/dialog/color_test.go b/dialog/color_test.go index 479eb15ca0..5e7e0b7fb4 100644 --- a/dialog/color_test.go +++ b/dialog/color_test.go @@ -95,10 +95,10 @@ func TestColorDialog_SetColor(t *testing.T) { func TestColorDialog_Buttons(t *testing.T) { for name, tt := range map[string]struct { - action func(d *ColorPickerDialog, w fyne.Window) - advanced bool - expectedCalled bool - expectedColored bool + action func(d *ColorPickerDialog, w fyne.Window) + advanced bool + expectedCalled bool + expectedClosed bool }{ "confirmAdvanced": { action: func(d *ColorPickerDialog, w fyne.Window) { @@ -107,9 +107,9 @@ func TestColorDialog_Buttons(t *testing.T) { test.FocusNext(w.Canvas()) // confirm button w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) }, - advanced: true, - expectedCalled: true, - expectedColored: true, + advanced: true, + expectedCalled: true, + expectedClosed: true, }, "dismissAdvanced": { action: func(d *ColorPickerDialog, w fyne.Window) { @@ -117,18 +117,18 @@ func TestColorDialog_Buttons(t *testing.T) { test.FocusNext(w.Canvas()) // dismiss button w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) }, - advanced: true, - expectedCalled: true, - expectedColored: false, + advanced: true, + expectedCalled: false, + expectedClosed: true, }, "dismissSimple": { action: func(d *ColorPickerDialog, w fyne.Window) { test.FocusNext(w.Canvas()) // dismiss button w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) }, - advanced: false, - expectedCalled: true, - expectedColored: false, + advanced: false, + expectedCalled: false, + expectedClosed: true, }, } { t.Run(name, func(t *testing.T) { @@ -137,24 +137,26 @@ func TestColorDialog_Buttons(t *testing.T) { w.Resize(fyne.NewSize(800, 600)) called := false - colored := false - d := NewColorPicker("Color Picker", "Pick a Color", func(c color.Color) { + closed := false + d := NewColorPicker("Color Picker", "Pick a Color", func(_ color.Color) { called = true - colored = c != nil }, w) d.Advanced = tt.advanced + d.SetOnClosed(func() { + closed = true + }) d.Refresh() d.Show() tt.action(d, w) assert.Equal(t, tt.expectedCalled, called, "called isn't equal") - assert.Equal(t, tt.expectedColored, colored, "colored isn't equal") + assert.Equal(t, tt.expectedClosed, closed, "closed isn't equal") }) } } func TestColorDialog_DoubleCallback(t *testing.T) { - ch := make(chan int) + ch := make(chan int, 2) d := NewColorPicker("Color Picker", "Pick a Color", func(_ color.Color) { ch <- 42 }, test.NewTempWindow(t, nil)) @@ -163,14 +165,33 @@ func TestColorDialog_DoubleCallback(t *testing.T) { }) d.Refresh() d.Show() - assert.False(t, d.win.Hidden) go test.Tap(d.dismiss) - assert.EqualValues(t, <-ch, 42) assert.EqualValues(t, <-ch, 43) assert.True(t, d.win.Hidden) -} + close(ch) + ch = make(chan int, 2) + w := test.NewTempWindow(t, canvas.NewRectangle(color.Transparent)) + d = NewColorPicker("Color Picker", "Pick a Color", func(_ color.Color) { + ch <- 52 + }, w) + d.Advanced = true + d.SetOnClosed(func() { + ch <- 53 + }) + d.Refresh() + d.Show() + assert.False(t, d.win.Hidden) + test.FocusNext(w.Canvas()) // advanced accordion + test.FocusNext(w.Canvas()) // dismiss button + test.FocusNext(w.Canvas()) // confirm button + w.Canvas().Focused().TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace}) + assert.EqualValues(t, <-ch, 52) + assert.EqualValues(t, <-ch, 53) + //assert.True(t, d.win.Hidden, "hidden status of window") shouldn't this work??? + close(ch) +} func TestColorDialogSimple_Theme(t *testing.T) { test.NewTempApp(t)