~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
missing
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
missing
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.
Review patch Export thread (mbox)