Merge pull request #4803 from andydotxyz/fix/4767

Focus selects when tapped
fyne-io · May 6, 2024 · 748c20b · 748c20b
2 parents d247b0e + 375d165
commit 748c20b
Show file tree

Hide file tree

Showing 65 changed files with 911 additions and 31 deletions.
diff --git a/widget/check.go b/widget/check.go
@@ -138,12 +138,8 @@ func (c *Check) Tapped(pe *fyne.PointEvent) {
 		return
 	}
 
-	if !c.focused && !fyne.CurrentDevice().IsMobile() {
-		impl := c.super()
-
-		if c := fyne.CurrentApp().Driver().CanvasForObject(impl); c != nil {
-			c.Focus(impl.(fyne.Focusable))
-		}
+	if !c.focused {
+		focusIfNotMobile(c.super())
 	}
 	c.SetChecked(!c.Checked)
 }
@@ -382,3 +378,12 @@ func (c *checkRenderer) updateFocusIndicator(th fyne.Theme, v fyne.ThemeVariant)
 		c.focusIndicator.FillColor = color.Transparent
 	}
 }
+
+func focusIfNotMobile(w fyne.Widget) {
+	if !fyne.CurrentDevice().IsMobile() {
+		if c := fyne.CurrentApp().Driver().CanvasForObject(w); c != nil {
+			c.Focus(w.(fyne.Focusable))
+		}
+	}
+
+}
diff --git a/widget/radio_item.go b/widget/radio_item.go
@@ -105,12 +105,8 @@ func (i *radioItem) SetSelected(selected bool) {
 //
 // Implements: fyne.Tappable
 func (i *radioItem) Tapped(_ *fyne.PointEvent) {
-	if !i.focused && !fyne.CurrentDevice().IsMobile() {
-		impl := i.super()
-
-		if c := fyne.CurrentApp().Driver().CanvasForObject(impl); c != nil {
-			c.Focus(impl.(fyne.Focusable))
-		}
+	if !i.focused {
+		focusIfNotMobile(i.super())
 	}
 	i.toggle()
 }

diff --git a/widget/select.go b/widget/select.go
@@ -200,6 +200,10 @@ func (s *Select) Tapped(*fyne.PointEvent) {
 		return
 	}
 
+	if !s.focused {
+		focusIfNotMobile(s.super())
+	}
+
 	s.tapAnimation()
 	s.Refresh()
 

diff --git a/widget/select_internal_test.go b/widget/select_internal_test.go
@@ -47,11 +47,16 @@ func TestSelectRenderer_TapAnimation(t *testing.T) {
 	sel.Resize(sel.MinSize())
 	sel.Refresh()
 
+	path := "select/desktop/tap_animation.png"
+	if fyne.CurrentDevice().IsMobile() {
+		path = "select/mobile/tap_animation.png"
+	}
+
 	render1 := test.WidgetRenderer(sel).(*selectRenderer)
 	test.Tap(sel)
 	sel.popUp.Hide()
 	sel.tapAnim.Tick(0.5)
-	test.AssertImageMatches(t, "select/tap_animation.png", w.Canvas().Capture())
+	test.AssertImageMatches(t, path, w.Canvas().Capture())
 
 	cache.DestroyRenderer(sel)
 	sel.Refresh()
@@ -63,5 +68,5 @@ func TestSelectRenderer_TapAnimation(t *testing.T) {
 	test.Tap(sel)
 	sel.popUp.Hide()
 	sel.tapAnim.Tick(0.5)
-	test.AssertImageMatches(t, "select/tap_animation.png", w.Canvas().Capture())
+	test.AssertImageMatches(t, path, w.Canvas().Capture())
 }
diff --git a/widget/select_test.go b/widget/select_test.go
@@ -1,6 +1,8 @@
 package widget_test
 
 import (
+	"fmt"
+	"image"
 	"testing"
 	"time"
 
@@ -35,11 +37,11 @@ func TestSelect_Align(t *testing.T) {
 	c := fyne.CurrentApp().Driver().CanvasForObject(sel)
 
 	test.Tap(sel)
-	test.AssertRendersToMarkup(t, "select/center.xml", c)
+	assertRendersToPlatformMarkup(t, "select/%s/center.xml", c)
 
 	sel.Alignment = fyne.TextAlignTrailing
 	sel.Refresh()
-	test.AssertRendersToMarkup(t, "select/trailing.xml", c)
+	assertRendersToPlatformMarkup(t, "select/%s/trailing.xml", c)
 }
 
 func TestSelect_ChangeTheme(t *testing.T) {
@@ -54,13 +56,13 @@ func TestSelect_ChangeTheme(t *testing.T) {
 	combo.Move(fyne.NewPos(10, 10))
 	test.Tap(combo)
 
-	test.AssertImageMatches(t, "select/theme_initial.png", w.Canvas().Capture())
+	assertImageMatchesPlatform(t, "select/%s/theme_initial.png", w.Canvas().Capture())
 
 	test.WithTestTheme(t, func() {
 		combo.Resize(combo.MinSize())
 		combo.Refresh()
 		time.Sleep(100 * time.Millisecond)
-		test.AssertImageMatches(t, "select/theme_changed.png", w.Canvas().Capture())
+		assertImageMatchesPlatform(t, "select/%s/theme_changed.png", w.Canvas().Capture())
 	})
 }
 
@@ -295,10 +297,10 @@ func TestSelect_Move(t *testing.T) {
 	test.AssertRendersToMarkup(t, "select/move_initial.xml", w.Canvas())
 
 	combo.Tapped(&fyne.PointEvent{})
-	test.AssertRendersToMarkup(t, "select/move_tapped.xml", w.Canvas())
+	assertRendersToPlatformMarkup(t, "select/%s/move_tapped.xml", w.Canvas())
 
 	combo.Move(fyne.NewPos(20, 20))
-	test.AssertRendersToMarkup(t, "select/move_moved.xml", w.Canvas())
+	assertRendersToPlatformMarkup(t, "select/%s/move_moved.xml", w.Canvas())
 }
 
 func TestSelect_PlaceHolder(t *testing.T) {
@@ -427,7 +429,7 @@ func TestSelect_Tapped(t *testing.T) {
 	test.Tap(combo)
 	canvas := fyne.CurrentApp().Driver().CanvasForObject(combo)
 	assert.Equal(t, 1, len(canvas.Overlays().List()))
-	test.AssertRendersToMarkup(t, "select/tapped.xml", w.Canvas())
+	assertRendersToPlatformMarkup(t, "select/%s/tapped.xml", w.Canvas())
 }
 
 func TestSelect_Tapped_Constrained(t *testing.T) {
@@ -444,7 +446,7 @@ func TestSelect_Tapped_Constrained(t *testing.T) {
 	combo.Move(fyne.NewPos(canvas.Size().Width-10, canvas.Size().Height-10))
 	test.Tap(combo)
 	assert.Equal(t, 1, len(canvas.Overlays().List()))
-	test.AssertRendersToMarkup(t, "select/tapped_constrained.xml", w.Canvas())
+	assertRendersToPlatformMarkup(t, "select/%s/tapped_constrained.xml", w.Canvas())
 }
 
 func TestSelect_Layout(t *testing.T) {
@@ -554,9 +556,29 @@ func TestSelect_Layout(t *testing.T) {
 			}
 			window.Resize(combo.MinSize().Max(fyne.NewSize(150, 200)))
 
-			test.AssertRendersToMarkup(t, "select/layout_"+name+".xml", window.Canvas())
+			assertRendersToPlatformMarkup(t, "select/%s/layout_"+name+".xml", window.Canvas())
 
 			window.Close()
 		})
 	}
 }
+
+func assertRendersToPlatformMarkup(t *testing.T, file string, c fyne.Canvas) {
+	platform := "desktop"
+	if fyne.CurrentDevice().IsMobile() {
+		platform = "mobile"
+	}
+
+	path := fmt.Sprintf(file, platform)
+	test.AssertRendersToMarkup(t, path, c)
+}
+
+func assertImageMatchesPlatform(t *testing.T, file string, i image.Image) {
+	platform := "desktop"
+	if fyne.CurrentDevice().IsMobile() {
+		platform = "mobile"
+	}
+
+	path := fmt.Sprintf(file, platform)
+	test.AssertImageMatches(t, path, i)
+}
diff --git a/widget/slider.go b/widget/slider.go
@@ -114,13 +114,8 @@ func (s *Slider) Tapped(e *fyne.PointEvent) {
 		return
 	}
 
-	driver := fyne.CurrentApp().Driver()
-	if !s.focused && !driver.Device().IsMobile() {
-		impl := s.super()
-
-		if c := driver.CanvasForObject(impl); c != nil {
-			c.Focus(impl.(fyne.Focusable))
-		}
+	if !s.focused {
+		focusIfNotMobile(s.super())
 	}
 
 	ratio := s.getRatio(e)

diff --git a/widget/slider_test.go b/widget/slider_test.go
@@ -290,9 +290,20 @@ func TestSlider_SetValue(t *testing.T) {
 	assert.Equal(t, 2.0, slider.Value)
 }
 
+func TestSlider_FocusDesktop(t *testing.T) {
+	if fyne.CurrentDevice().IsMobile() {
+		return
+	}
+	slider := NewSlider(0, 10)
+	win := test.NewWindow(slider)
+	test.Tap(slider)
+
+	assert.Equal(t, win.Canvas().Focused(), slider)
+	assert.True(t, slider.focused)
+}
+
 func TestSlider_Focus(t *testing.T) {
 	slider := NewSlider(0, 5)
-
 	slider.FocusGained()
 	assert.True(t, slider.focused)
 

diff --git a/widget/testdata/select/desktop/center.xml b/widget/testdata/select/desktop/center.xml
@@ -0,0 +1,42 @@
+<canvas padded size="200x150">
+	<content>
+		<container pos="4,4" size="192x142">
+			<widget pos="32,53" size="127x35" type="*widget.Select">
+				<rectangle fillColor="focus" radius="4" size="127x35"/>
+				<rectangle size="127x35"/>
+				<widget pos="4,4" size="95x27" type="*widget.RichText">
+					<text alignment="center" pos="10,4" size="75x19">(Select one)</text>
+				</widget>
+				<widget pos="99,7" size="20x20" type="*widget.Icon">
+					<image fillMode="contain" rsc="menuDropDownIcon" size="iconInlineSize" themed="foreground"/>
+				</widget>
+			</widget>
+		</container>
+	</content>
+	<overlay>
+		<widget size="200x150" type="*widget.OverlayContainer">
+			<widget pos="36,90" size="127x35" type="*widget.PopUpMenu">
+				<widget size="127x35" type="*widget.Shadow">
+					<radialGradient centerOffset="0.5,0.5" pos="-4,-4" size="4x4" startColor="shadow"/>
+					<linearGradient endColor="shadow" pos="0,-4" size="127x4"/>
+					<radialGradient centerOffset="-0.5,0.5" pos="127,-4" size="4x4" startColor="shadow"/>
+					<linearGradient angle="270" pos="127,0" size="4x35" startColor="shadow"/>
+					<radialGradient centerOffset="-0.5,-0.5" pos="127,35" size="4x4" startColor="shadow"/>
+					<linearGradient pos="0,35" size="127x4" startColor="shadow"/>
+					<radialGradient centerOffset="0.5,-0.5" pos="-4,35" size="4x4" startColor="shadow"/>
+					<linearGradient angle="270" endColor="shadow" pos="-4,0" size="4x35"/>
+				</widget>
+				<widget size="127x35" type="*widget.Scroll">
+					<widget size="127x35" type="*widget.menuBox">
+						<rectangle fillColor="menuBackground" size="127x35"/>
+						<container size="127x35">
+							<widget size="127x35" type="*widget.menuItem">
+								<text alignment="center" pos="8,8" size="111x19">Hi</text>
+							</widget>
+						</container>
+					</widget>
+				</widget>
+			</widget>
+		</widget>
+	</overlay>
+</canvas>
diff --git a/widget/testdata/select/layout_empty.xml → .../testdata/select/desktop/layout_empty.xml b/widget/testdata/select/layout_empty.xml → .../testdata/select/desktop/layout_empty.xml
diff --git a/widget/testdata/select/desktop/layout_empty_expanded.xml b/widget/testdata/select/desktop/layout_empty_expanded.xml
@@ -0,0 +1,39 @@
+<canvas padded size="150x200">
+	<content>
+		<container pos="4,4" size="142x192">
+			<widget pos="7,78" size="127x35" type="*widget.Select">
+				<rectangle fillColor="focus" radius="4" size="127x35"/>
+				<rectangle size="127x35"/>
+				<widget pos="4,4" size="95x27" type="*widget.RichText">
+					<text pos="4,4" size="75x19">(Select one)</text>
+				</widget>
+				<widget pos="99,7" size="20x20" type="*widget.Icon">
+					<image fillMode="contain" rsc="menuDropDownIcon" size="iconInlineSize" themed="foreground"/>
+				</widget>
+			</widget>
+		</container>
+	</content>
+	<overlay>
+		<widget size="150x200" type="*widget.OverlayContainer">
+			<widget pos="11,115" size="127x0" type="*widget.PopUpMenu">
+				<widget size="127x0" type="*widget.Shadow">
+					<radialGradient centerOffset="0.5,0.5" pos="-4,-4" size="4x4" startColor="shadow"/>
+					<linearGradient endColor="shadow" pos="0,-4" size="127x4"/>
+					<radialGradient centerOffset="-0.5,0.5" pos="127,-4" size="4x4" startColor="shadow"/>
+					<linearGradient angle="270" pos="127,0" size="4x0" startColor="shadow"/>
+					<radialGradient centerOffset="-0.5,-0.5" pos="127,0" size="4x4" startColor="shadow"/>
+					<linearGradient size="127x4" startColor="shadow"/>
+					<radialGradient centerOffset="0.5,-0.5" pos="-4,0" size="4x4" startColor="shadow"/>
+					<linearGradient angle="270" endColor="shadow" pos="-4,0" size="4x0"/>
+				</widget>
+				<widget size="127x0" type="*widget.Scroll">
+					<widget size="127x0" type="*widget.menuBox">
+						<rectangle fillColor="menuBackground" size="127x0"/>
+						<container size="127x0">
+						</container>
+					</widget>
+				</widget>
+			</widget>
+		</widget>
+	</overlay>
+</canvas>
diff --git a/widget/testdata/select/desktop/layout_empty_expanded_placeholder.xml b/widget/testdata/select/desktop/layout_empty_expanded_placeholder.xml
@@ -0,0 +1,39 @@
+<canvas padded size="150x200">
+	<content>
+		<container pos="4,4" size="142x192">
+			<widget pos="21,78" size="98x35" type="*widget.Select">
+				<rectangle fillColor="focus" radius="4" size="98x35"/>
+				<rectangle size="98x35"/>
+				<widget pos="4,4" size="66x27" type="*widget.RichText">
+					<text pos="4,4" size="46x19">(Pick 1)</text>
+				</widget>
+				<widget pos="70,7" size="20x20" type="*widget.Icon">
+					<image fillMode="contain" rsc="menuDropDownIcon" size="iconInlineSize" themed="foreground"/>
+				</widget>
+			</widget>
+		</container>
+	</content>
+	<overlay>
+		<widget size="150x200" type="*widget.OverlayContainer">
+			<widget pos="25,115" size="98x0" type="*widget.PopUpMenu">
+				<widget size="98x0" type="*widget.Shadow">
+					<radialGradient centerOffset="0.5,0.5" pos="-4,-4" size="4x4" startColor="shadow"/>
+					<linearGradient endColor="shadow" pos="0,-4" size="98x4"/>
+					<radialGradient centerOffset="-0.5,0.5" pos="98,-4" size="4x4" startColor="shadow"/>
+					<linearGradient angle="270" pos="98,0" size="4x0" startColor="shadow"/>
+					<radialGradient centerOffset="-0.5,-0.5" pos="98,0" size="4x4" startColor="shadow"/>
+					<linearGradient size="98x4" startColor="shadow"/>
+					<radialGradient centerOffset="0.5,-0.5" pos="-4,0" size="4x4" startColor="shadow"/>
+					<linearGradient angle="270" endColor="shadow" pos="-4,0" size="4x0"/>
+				</widget>
+				<widget size="98x0" type="*widget.Scroll">
+					<widget size="98x0" type="*widget.menuBox">
+						<rectangle fillColor="menuBackground" size="98x0"/>
+						<container size="98x0">
+						</container>
+					</widget>
+				</widget>
+			</widget>
+		</widget>
+	</overlay>
+</canvas>
diff --git a/...tdata/select/layout_empty_placeholder.xml → ...lect/desktop/layout_empty_placeholder.xml b/...tdata/select/layout_empty_placeholder.xml → ...lect/desktop/layout_empty_placeholder.xml
diff --git a/widget/testdata/select/layout_multiple.xml → ...stdata/select/desktop/layout_multiple.xml b/widget/testdata/select/layout_multiple.xml → ...stdata/select/desktop/layout_multiple.xml
diff --git a/widget/testdata/select/desktop/layout_multiple_expanded.xml b/widget/testdata/select/desktop/layout_multiple_expanded.xml
@@ -0,0 +1,45 @@
+<canvas padded size="150x200">
+	<content>
+		<container pos="4,4" size="142x192">
+			<widget pos="7,78" size="127x35" type="*widget.Select">
+				<rectangle fillColor="focus" radius="4" size="127x35"/>
+				<rectangle size="127x35"/>
+				<widget pos="4,4" size="95x27" type="*widget.RichText">
+					<text pos="4,4" size="75x19">(Select one)</text>
+				</widget>
+				<widget pos="99,7" size="20x20" type="*widget.Icon">
+					<image fillMode="contain" rsc="menuDropDownIcon" size="iconInlineSize" themed="foreground"/>
+				</widget>
+			</widget>
+		</container>
+	</content>
+	<overlay>
+		<widget size="150x200" type="*widget.OverlayContainer">
+			<widget pos="11,115" size="127x74" type="*widget.PopUpMenu">
+				<widget size="127x74" type="*widget.Shadow">
+					<radialGradient centerOffset="0.5,0.5" pos="-4,-4" size="4x4" startColor="shadow"/>
+					<linearGradient endColor="shadow" pos="0,-4" size="127x4"/>
+					<radialGradient centerOffset="-0.5,0.5" pos="127,-4" size="4x4" startColor="shadow"/>
+					<linearGradient angle="270" pos="127,0" size="4x74" startColor="shadow"/>
+					<radialGradient centerOffset="-0.5,-0.5" pos="127,74" size="4x4" startColor="shadow"/>
+					<linearGradient pos="0,74" size="127x4" startColor="shadow"/>
+					<radialGradient centerOffset="0.5,-0.5" pos="-4,74" size="4x4" startColor="shadow"/>
+					<linearGradient angle="270" endColor="shadow" pos="-4,0" size="4x74"/>
+				</widget>
+				<widget size="127x74" type="*widget.Scroll">
+					<widget size="127x74" type="*widget.menuBox">
+						<rectangle fillColor="menuBackground" size="127x74"/>
+						<container size="127x74">
+							<widget size="127x35" type="*widget.menuItem">
+								<text pos="8,8" size="111x19">Foo</text>
+							</widget>
+							<widget pos="0,39" size="127x35" type="*widget.menuItem">
+								<text pos="8,8" size="111x19">Bar</text>
+							</widget>
+						</container>
+					</widget>
+				</widget>
+			</widget>
+		</widget>
+	</overlay>
+</canvas>