~eliasnaur/gio

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH] widgets, widgets/material: add RadioButton & RadioButtonsGroup

Alexander Arin
Details
Message ID
<20191105155429.14496-1-fralx@yandex.ru>
DKIM signature
pass
Download raw message
Patch: +165 -9
Signed-off-by: Alexander Arin <fralx@yandex.ru>
---
 example/kitchen/kitchen.go     | 23 +++++++--
 widget/material/material.go    | 14 ++++--
 widget/material/radiobutton.go | 88 ++++++++++++++++++++++++++++++++++
 widget/radiobuttonsgroup.go    | 49 +++++++++++++++++++
 4 files changed, 165 insertions(+), 9 deletions(-)
 create mode 100644 widget/material/radiobutton.go
 create mode 100644 widget/radiobuttonsgroup.go

diff --git a/example/kitchen/kitchen.go b/example/kitchen/kitchen.go
index e390761..b794cd4 100644
--- a/example/kitchen/kitchen.go
+++ b/example/kitchen/kitchen.go
@@ -29,6 +29,7 @@ func main() {
 	}
 	icon = ic
 	gofont.Register()
+	radioButtonsGroup.SetValue("RadioButton1")
 	go func() {
 		w := app.NewWindow()
 		if err := loop(w); err != nil {
@@ -62,10 +63,11 @@ var (
 		SingleLine: true,
 		Submit:     true,
 	}
-	button      = new(widget.Button)
-	greenButton = new(widget.Button)
-	iconButton  = new(widget.Button)
-	list        = &layout.List{
+	button            = new(widget.Button)
+	greenButton       = new(widget.Button)
+	iconButton        = new(widget.Button)
+	radioButtonsGroup = new(widget.RadioButtonsGroup)
+	list              = &layout.List{
 		Axis: layout.Vertical,
 	}
 	green    = true
@@ -125,6 +127,19 @@ func kitchen(gtx *layout.Context, th *material.Theme) {
 		func() {
 			th.CheckBox("Checkbox").Layout(gtx, checkbox)
 		},
+		func() {
+			group := layout.Flex{}
+			r1 := group.Rigid(gtx, func() {
+				th.RadioButton("RadioButton1").Layout(gtx, radioButtonsGroup)
+			})
+			r2 := group.Rigid(gtx, func() {
+				th.RadioButton("RadioButton2").Layout(gtx, radioButtonsGroup)
+			})
+			r3 := group.Rigid(gtx, func() {
+				th.RadioButton("RadioButton3").Layout(gtx, radioButtonsGroup)
+			})
+			group.Layout(gtx, r1, r2, r3)
+		},
 	}
 	list.Layout(gtx, len(widgets), func(i int) {
 		layout.UniformInset(unit.Dp(16)).Layout(gtx, widgets[i])
diff --git a/widget/material/material.go b/widget/material/material.go
index d5b70db..6747224 100644
--- a/widget/material/material.go
+++ b/widget/material/material.go
@@ -25,9 +25,11 @@ type Theme struct {
 		Hint    color.RGBA
 		InvText color.RGBA
 	}
-	TextSize           unit.Value
-	checkedStateIcon   *Icon
-	uncheckedStateIcon *Icon
+	TextSize              unit.Value
+	checkBoxCheckedIcon   *Icon
+	checkBoxUncheckedIcon *Icon
+	radioCheckedIcon      *Icon
+	radioUncheckedIcon    *Icon
 }
 
 func NewTheme() *Theme {
@@ -40,8 +42,10 @@ func NewTheme() *Theme {
 	t.Color.InvText = rgb(0xffffff)
 	t.TextSize = unit.Sp(16)
 
-	t.checkedStateIcon = mustIcon(NewIcon(icons.ToggleCheckBox))
-	t.uncheckedStateIcon = mustIcon(NewIcon(icons.ToggleCheckBoxOutlineBlank))
+	t.checkBoxCheckedIcon = mustIcon(NewIcon(icons.ToggleCheckBox))
+	t.checkBoxUncheckedIcon = mustIcon(NewIcon(icons.ToggleCheckBoxOutlineBlank))
+	t.radioCheckedIcon = mustIcon(NewIcon(icons.ToggleRadioButtonChecked))
+	t.radioUncheckedIcon = mustIcon(NewIcon(icons.ToggleRadioButtonUnchecked))
 
 	return t
 }
diff --git a/widget/material/radiobutton.go b/widget/material/radiobutton.go
new file mode 100644
index 0000000..d410d0c
--- /dev/null
+++ b/widget/material/radiobutton.go
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: Unlicense OR MIT
+
+// Package material implements the Material design.
+package material
+
+import (
+	"gioui.org/io/pointer"
+	"gioui.org/layout"
+	"gioui.org/op/paint"
+	"gioui.org/text"
+	"gioui.org/unit"
+	"gioui.org/widget"
+	"image"
+	"image/color"
+)
+
+type RadioButton struct {
+	Text string
+	// Color is the text color.
+	Color              color.RGBA
+	Font               text.Font
+	IconColor          color.RGBA
+	Size               unit.Value
+	shaper             *text.Shaper
+	checkedStateIcon   *Icon
+	uncheckedStateIcon *Icon
+}
+
+func (t *Theme) RadioButton(txt string) RadioButton {
+	return RadioButton{
+		Text:      txt,
+		Color:     t.Color.Text,
+		IconColor: t.Color.Primary,
+		Font: text.Font{
+			Size: t.TextSize.Scale(14.0 / 16.0),
+		},
+		Size:               unit.Dp(26),
+		shaper:             t.Shaper,
+		checkedStateIcon:   t.radioCheckedIcon,
+		uncheckedStateIcon: t.radioUncheckedIcon,
+	}
+}
+
+func (c RadioButton) Layout(gtx *layout.Context, rg *widget.RadioButtonsGroup) {
+
+	textColor := c.Color
+	iconColor := c.IconColor
+
+	var icon *Icon
+	if rg.Value(gtx) == c.Text {
+		icon = c.checkedStateIcon
+	} else {
+		icon = c.uncheckedStateIcon
+	}
+
+	hmin := gtx.Constraints.Width.Min
+	vmin := gtx.Constraints.Height.Min
+
+	flex := layout.Flex{Alignment: layout.Middle}
+
+	ico := flex.Rigid(gtx, func() {
+		layout.Align(layout.Center).Layout(gtx, func() {
+			layout.UniformInset(unit.Dp(2)).Layout(gtx, func() {
+				size := gtx.Px(c.Size)
+				icon.Color = iconColor
+				icon.Layout(gtx, unit.Px(float32(size)))
+				gtx.Dimensions = layout.Dimensions{
+					Size: image.Point{X: size, Y: size},
+				}
+			})
+		})
+	})
+
+	lbl := flex.Rigid(gtx, func() {
+		gtx.Constraints.Width.Min = hmin
+		gtx.Constraints.Height.Min = vmin
+		layout.Align(layout.Start).Layout(gtx, func() {
+			layout.UniformInset(unit.Dp(2)).Layout(gtx, func() {
+				paint.ColorOp{Color: textColor}.Add(gtx.Ops)
+				widget.Label{}.Layout(gtx, c.shaper, c.Font, c.Text)
+			})
+		})
+	})
+
+	flex.Layout(gtx, ico, lbl)
+	pointer.RectAreaOp{Rect: image.Rectangle{Max: gtx.Dimensions.Size}}.Add(gtx.Ops)
+	rg.Layout(gtx, c.Text)
+}
diff --git a/widget/radiobuttonsgroup.go b/widget/radiobuttonsgroup.go
new file mode 100644
index 0000000..68ef2b2
--- /dev/null
+++ b/widget/radiobuttonsgroup.go
@@ -0,0 +1,49 @@
+package widget
+
+import (
+	"gioui.org/gesture"
+	"gioui.org/layout"
+)
+
+type RadioButtonsGroup struct {
+	clicks []gesture.Click
+	values []string
+	value  string
+}
+
+func Index(vs []string, t string) int {
+	for i, v := range vs {
+		if v == t {
+			return i
+		}
+	}
+	return -1
+}
+
+func (rg *RadioButtonsGroup) Value(gtx *layout.Context) string {
+	for i := range rg.clicks {
+		for _, e := range rg.clicks[i].Events(gtx) {
+			switch e.Type {
+			case gesture.TypeClick:
+				rg.value = rg.values[i]
+				return rg.value
+			}
+		}
+	}
+	return rg.value
+}
+
+func (rg *RadioButtonsGroup) Layout(gtx *layout.Context, value string) {
+	if Index(rg.values, value) == -1 {
+		rg.values = append(rg.values, value)
+		rg.clicks = append(rg.clicks, gesture.Click{})
+		rg.clicks[len(rg.clicks)-1].Add(gtx.Ops)
+	} else {
+		idx := Index(rg.values, value)
+		rg.clicks[idx].Add(gtx.Ops)
+	}
+}
+
+func (rg *RadioButtonsGroup) SetValue(value string) {
+	rg.value = value
+}
-- 
2.18.0.windows.1
Details
Message ID
<BY843TDH4HBX.2JSH8XUZ00BYR@toolbox>
In-Reply-To
<20191105155429.14496-1-fralx@yandex.ru> (view parent)
DKIM signature
pass
Download raw message
Looks good overall. I have a few comments below.

On Tue Nov 5, 2019 at 6:54 PM Alexander Arin wrote:
> Signed-off-by: Alexander Arin <fralx@yandex.ru>
> ---
>  example/kitchen/kitchen.go     | 23 +++++++--
>  widget/material/material.go    | 14 ++++--
>  widget/material/radiobutton.go | 88 ++++++++++++++++++++++++++++++++++
>  widget/radiobuttonsgroup.go    | 49 +++++++++++++++++++
>  4 files changed, 165 insertions(+), 9 deletions(-)
>  create mode 100644 widget/material/radiobutton.go
>  create mode 100644 widget/radiobuttonsgroup.go
> 
> diff --git a/example/kitchen/kitchen.go b/example/kitchen/kitchen.go
> index e390761..b794cd4 100644
> --- a/example/kitchen/kitchen.go
> +++ b/example/kitchen/kitchen.go
> @@ -29,6 +29,7 @@ func main() {
>  	}
>  	icon = ic
>  	gofont.Register()
> +	radioButtonsGroup.SetValue("RadioButton1")

Drop this line. The kitchen example is for showing off widget behaviour, and
SetValue is too subtle to include.

> diff --git a/widget/material/radiobutton.go b/widget/material/radiobutton.go
> new file mode 100644
> index 0000000..d410d0c
> --- /dev/null
> +++ b/widget/material/radiobutton.go
> @@ -0,0 +1,88 @@
> +
> +func (t *Theme) RadioButton(txt string) RadioButton {

Add a separate key to make it easy to change the label without changing the
code that relies on the keys. Example: multi-language support.

Rename "txt" to "label", to make it extra clear what it is.

Finally, document the method, say

// RadioButton returns a RadioButton with a label. The key specifies
// the value for the Enum.
func (t *Theme) RadioButton(key, txt string) RadioButton

> +func (c RadioButton) Layout(gtx *layout.Context, rg *widget.RadioButtonsGroup) {
> +
> +	textColor := c.Color
> +	iconColor := c.IconColor
> +
> +	var icon *Icon
> +	if rg.Value(gtx) == c.Text {
> +		icon = c.checkedStateIcon
> +	} else {
> +		icon = c.uncheckedStateIcon
> +	}
> +
> +	hmin := gtx.Constraints.Width.Min
> +	vmin := gtx.Constraints.Height.Min
> +
> +	flex := layout.Flex{Alignment: layout.Middle}
> +
> +	ico := flex.Rigid(gtx, func() {
> +		layout.Align(layout.Center).Layout(gtx, func() {
> +			layout.UniformInset(unit.Dp(2)).Layout(gtx, func() {
> +				size := gtx.Px(c.Size)
> +				icon.Color = iconColor
> +				icon.Layout(gtx, unit.Px(float32(size)))
> +				gtx.Dimensions = layout.Dimensions{
> +					Size: image.Point{X: size, Y: size},
> +				}
> +			})
> +		})
> +	})
> +
> +	lbl := flex.Rigid(gtx, func() {
> +		gtx.Constraints.Width.Min = hmin
> +		gtx.Constraints.Height.Min = vmin
> +		layout.Align(layout.Start).Layout(gtx, func() {
> +			layout.UniformInset(unit.Dp(2)).Layout(gtx, func() {
> +				paint.ColorOp{Color: textColor}.Add(gtx.Ops)
> +				widget.Label{}.Layout(gtx, c.shaper, c.Font, c.Text)
> +			})
> +		})
> +	})
> +
> +	flex.Layout(gtx, ico, lbl)
> +	pointer.RectAreaOp{Rect: image.Rectangle{Max: gtx.Dimensions.Size}}.Add(gtx.Ops)
> +	rg.Layout(gtx, c.Text)

Layout looks very similar to a CheckBox. Can some of their Layout code be shared in a
unexported function?

> diff --git a/widget/radiobuttonsgroup.go b/widget/radiobuttonsgroup.go
> new file mode 100644
> index 0000000..68ef2b2
> --- /dev/null
> +++ b/widget/radiobuttonsgroup.go
> @@ -0,0 +1,49 @@
> +package widget
> +
> +import (
> +	"gioui.org/gesture"
> +	"gioui.org/layout"
> +)
> +
> +type RadioButtonsGroup struct {

I don't think this a good name. The widget package is for keeping the state and event
handling, not the particular visual representation. A RadioButton(Group) is just one
particular way of displaying a set of mutually exclusive options.

I suggest widget.Enum or widget.Option. You may have an even better name.

> +func Index(vs []string, t string) int {

Unexport Index.

> +	for i, v := range vs {
> +		if v == t {
> +			return i
> +		}
> +	}
> +	return -1
> +}
> +
> +func (rg *RadioButtonsGroup) Value(gtx *layout.Context) string {

Document the method. Perhaps

// Value processes events and returns the last selected value, or
// the empty string.

> +	for i := range rg.clicks {
> +		for _, e := range rg.clicks[i].Events(gtx) {
> +			switch e.Type {
> +			case gesture.TypeClick:
> +				rg.value = rg.values[i]
> +				return rg.value

Drop this return. Correctly selects the last click and is simpler.

> +func (rg *RadioButtonsGroup) Layout(gtx *layout.Context, value string) {

Rename value to key to make it clear that it is a key, not a user visible
label.

Also document. Perhaps

// Layout adds the event handler for key.