RadioGroup improvements

widget/check_group.go

@@ -265,3 +265,17 @@ func (r *checkGroupRenderer) updateItems() {
 		item.Refresh()
 	}
 }
+
+func removeDuplicates(options []string) []string {
+	var result []string
+	found := make(map[string]bool)
+
+	for _, option := range options {
+		if _, ok := found[option]; !ok {
+			found[option] = true
+			result = append(result, option)
+		}
+	}
+
+	return result
+}

widget/radio_group.go

@@ -18,7 +18,10 @@ type RadioGroup struct {
 	Options    []string
 	Selected   string
 
-	items []*radioItem
+	// this index is ONE-BASED so the default zero-value is unselected
+	// use r.selectedIndex(), r.setSelectedIndex(int) to maniupulate this field
+	// as if it were a zero-based index (with -1 == nothing selected)
+	_selIdx int
 }
 
 var _ fyne.Widget = (*RadioGroup)(nil)
@@ -32,7 +35,6 @@ func NewRadioGroup(options []string, changed func(string)) *RadioGroup {
 		OnChanged: changed,
 	}
 	r.ExtendBaseWidget(r)
-	r.update()
 	return r
 }
 
@@ -46,16 +48,19 @@ func (r *RadioGroup) Append(option string) {
 // CreateRenderer is a private method to Fyne which links this widget to its renderer
 func (r *RadioGroup) CreateRenderer() fyne.WidgetRenderer {
 	r.ExtendBaseWidget(r)
-	r.propertyLock.Lock()
-	defer r.propertyLock.Unlock()
 
-	r.update()
-	objects := make([]fyne.CanvasObject, len(r.items))
-	for i, item := range r.items {
-		objects[i] = item
+	items := make([]fyne.CanvasObject, len(r.Options))
+	for i, option := range r.Options {
+		idx := i
+		items[idx] = newRadioItem(option, func(item *radioItem) {
+			r.itemTapped(item, idx)
+		})
 	}
 
-	return &radioGroupRenderer{widget.NewBaseRenderer(objects), r.items, r}
+	render := &radioGroupRenderer{widget.NewBaseRenderer(items), items, r}
+	r.updateSelectedIndex()
+	render.updateItems(false)
+	return render
 }
 
 // MinSize returns the size that this widget should not shrink below
@@ -64,23 +69,14 @@ func (r *RadioGroup) MinSize() fyne.Size {
 	return r.BaseWidget.MinSize()
 }
 
-// Refresh causes this widget to be redrawn in it's current state.
-//
-// Implements: fyne.CanvasObject
-func (r *RadioGroup) Refresh() {
-	r.propertyLock.Lock()
-	r.update()
-	r.propertyLock.Unlock()
-	r.BaseWidget.Refresh()
-}
-
 // SetSelected sets the radio option, it can be used to set a default option.
 func (r *RadioGroup) SetSelected(option string) {
 	if r.Selected == option {
 		return
 	}
 
 	r.Selected = option
+	// selectedIndex will be updated on refresh to the first matching item
 
 	if r.OnChanged != nil {
 		r.OnChanged(option)
@@ -89,19 +85,21 @@ func (r *RadioGroup) SetSelected(option string) {
 	r.Refresh()
 }
 
-func (r *RadioGroup) itemTapped(item *radioItem) {
+func (r *RadioGroup) itemTapped(item *radioItem, idx int) {
 	if r.Disabled() {
 		return
 	}
 
-	if r.Selected == item.Label {
+	if r.selectedIndex() == idx {
 		if r.Required {
 			return
 		}
 		r.Selected = ""
+		r.setSelectedIndex(-1)
 		item.SetSelected(false)
 	} else {
 		r.Selected = item.Label
+		r.setSelectedIndex(idx)
 		item.SetSelected(true)
 	}
 
@@ -111,27 +109,47 @@ func (r *RadioGroup) itemTapped(item *radioItem) {
 	r.Refresh()
 }
 
-func (r *RadioGroup) update() {
-	r.Options = removeDuplicates(r.Options)
-	if len(r.items) < len(r.Options) {
-		for i := len(r.items); i < len(r.Options); i++ {
-			item := newRadioItem(r.Options[i], r.itemTapped)
-			r.items = append(r.items, item)
-		}
-	} else if len(r.items) > len(r.Options) {
-		r.items = r.items[:len(r.Options)]
+func (r *RadioGroup) Refresh() {
+	r.updateSelectedIndex()
+	r.BaseWidget.Refresh()
+}
+
+func (r *RadioGroup) selectedIndex() int {
+	return r._selIdx - 1
+}
+
+func (r *RadioGroup) setSelectedIndex(idx int) {
+	r._selIdx = idx + 1
+}
+
+// if selectedIndex does not match the public Selected property,
+// set it to the index of the first radio item whose label matches Selected
+func (r *RadioGroup) updateSelectedIndex() {
+	sel := r.Selected
+	sIdx := r.selectedIndex()
+	if sIdx >= 0 && sIdx < len(r.Options) && r.Options[sIdx] == sel {
+		return // selected index matches Selected
 	}
-	for i, item := range r.items {
-		item.Label = r.Options[i]
-		item.Selected = item.Label == r.Selected
-		item.DisableableWidget.disabled = r.disabled
-		item.Refresh()
+	if sIdx == -1 && sel == "" {
+		return // nothing selected
 	}
+
+	sIdx = -1
+	for i, opt := range r.Options {
+		if sel == opt {
+			sIdx = i
+			break
+		}
+	}
+	r.setSelectedIndex(sIdx)
 }
 
 type radioGroupRenderer struct {
 	widget.BaseRenderer
-	items []*radioItem
+
+	// slice of *radioItem, but using fyne.CanvasObject as the type
+	// so we can directly set it to the BaseRenderer's objects slice
+	items []fyne.CanvasObject
 	radio *RadioGroup
 }
 
@@ -187,41 +205,44 @@ func (r *radioGroupRenderer) MinSize() fyne.Size {
 }
 
 func (r *radioGroupRenderer) Refresh() {
-	r.updateItems()
+	r.updateItems(true)
 	canvas.Refresh(r.radio.super())
 }
 
-func (r *radioGroupRenderer) updateItems() {
+func (r *radioGroupRenderer) updateItems(refresh bool) {
 	if len(r.items) < len(r.radio.Options) {
 		for i := len(r.items); i < len(r.radio.Options); i++ {
-			item := newRadioItem(r.radio.Options[i], r.radio.itemTapped)
-			r.SetObjects(append(r.Objects(), item))
+			idx := i
+			item := newRadioItem(r.radio.Options[idx], func(item *radioItem) {
+				r.radio.itemTapped(item, idx)
+			})
 			r.items = append(r.items, item)
 		}
 		r.Layout(r.radio.Size())
 	} else if len(r.items) > len(r.radio.Options) {
 		total := len(r.radio.Options)
 		r.items = r.items[:total]
-		r.SetObjects(r.Objects()[:total])
-	}
-	for i, item := range r.items {
-		item.Label = r.radio.Options[i]
-		item.Selected = item.Label == r.radio.Selected
-		item.disabled = r.radio.disabled
-		item.Refresh()
 	}
-}
+	r.SetObjects(r.items)
 
-func removeDuplicates(options []string) []string {
-	var result []string
-	found := make(map[string]bool)
+	for i, item := range r.items {
+		item := item.(*radioItem)
+		changed := false
+		if l := r.radio.Options[i]; l != item.Label {
+			item.Label = r.radio.Options[i]
+			changed = true
+		}
+		if sel := i == r.radio.selectedIndex(); sel != item.Selected {
+			item.Selected = sel
+			changed = true
+		}
+		if d := r.radio.disabled; d != item.disabled {
+			item.disabled = d
+			changed = true
+		}
 
-	for _, option := range options {
-		if _, ok := found[option]; !ok {
-			found[option] = true
-			result = append(result, option)
+		if refresh && changed {
+			item.Refresh()
 		}
 	}
-
-	return result
 }

widget/radio_group_extended_test.go

@@ -22,7 +22,6 @@ func newextendedRadioGroup(opts []string, f func(string)) *extendedRadioGroup {
 	ret.Options = opts
 	ret.OnChanged = f
 	ret.ExtendBaseWidget(ret)
-	ret.update() // Not needed for extending Radio but for the tests to be able to access items without creating a renderer first.
 
 	return ret
 }
@@ -32,7 +31,7 @@ func TestRadioGroup_Extended_Selected(t *testing.T) {
 	radio := newextendedRadioGroup([]string{"Hi"}, func(sel string) {
 		selected = sel
 	})
-	radio.items[0].Tapped(&fyne.PointEvent{Position: fyne.NewPos(theme.Padding(), theme.Padding())})
+	extendedRadioGroupTestTapItem(t, radio, 0)
 
 	assert.Equal(t, "Hi", selected)
 }
@@ -43,7 +42,7 @@ func TestRadioGroup_Extended_Unselected(t *testing.T) {
 		selected = sel
 	})
 	radio.Selected = selected
-	radio.items[0].Tapped(&fyne.PointEvent{Position: fyne.NewPos(theme.Padding(), theme.Padding())})
+	extendedRadioGroupTestTapItem(t, radio, 0)
 
 	assert.Equal(t, "", selected)
 }
@@ -89,7 +88,7 @@ func TestRadioGroup_Extended_Disable(t *testing.T) {
 	})
 
 	radio.Disable()
-	radio.items[0].Tapped(&fyne.PointEvent{Position: fyne.NewPos(theme.Padding(), theme.Padding())})
+	extendedRadioGroupTestTapItem(t, radio, 0)
 
 	assert.Equal(t, "", selected, "RadioGroup should have been disabled.")
 }
@@ -101,11 +100,11 @@ func TestRadioGroup_Extended_Enable(t *testing.T) {
 	})
 
 	radio.Disable()
-	radio.items[0].Tapped(&fyne.PointEvent{Position: fyne.NewPos(theme.Padding(), theme.Padding())})
+	extendedRadioGroupTestTapItem(t, radio, 0)
 	assert.Equal(t, "", selected, "Radio should have been disabled.")
 
 	radio.Enable()
-	radio.items[0].Tapped(&fyne.PointEvent{Position: fyne.NewPos(theme.Padding(), theme.Padding())})
+	extendedRadioGroupTestTapItem(t, radio, 0)
 	assert.Equal(t, "Hi", selected, "Radio should have been re-enabled.")
 }
 
@@ -131,9 +130,9 @@ func TestRadioGroup_Extended_Hovered(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			radio := newextendedRadioGroup(tt.options, nil)
 			radio.Horizontal = tt.isHorizontal
-			item1 := radio.items[0]
+			item1 := test.WidgetRenderer(radio).Objects()[0].(*radioItem)
 			render1 := cache.Renderer(item1).(*radioItemRenderer)
-			render2 := cache.Renderer(radio.items[1]).(*radioItemRenderer)
+			render2 := cache.Renderer(test.WidgetRenderer(radio).Objects()[1].(*radioItem)).(*radioItemRenderer)
 
 			assert.False(t, item1.hovered)
 			assert.Equal(t, color.Transparent, render1.focusIndicator.FillColor)
@@ -166,7 +165,7 @@ func TestRadioGroup_Extended_Hovered(t *testing.T) {
 
 func TestRadioGroupRenderer_Extended_ApplyTheme(t *testing.T) {
 	radio := newextendedRadioGroup([]string{"Test"}, func(string) {})
-	render := cache.Renderer(radio.items[0]).(*radioItemRenderer)
+	render := cache.Renderer(test.WidgetRenderer(radio).Objects()[0].(*radioItem)).(*radioItemRenderer)
 
 	textSize := render.label.TextSize
 	customTextSize := textSize
@@ -177,3 +176,10 @@ func TestRadioGroupRenderer_Extended_ApplyTheme(t *testing.T) {
 
 	assert.NotEqual(t, textSize, customTextSize)
 }
+
+func extendedRadioGroupTestTapItem(t *testing.T, radio *extendedRadioGroup, item int) {
+	t.Helper()
+	renderer := test.WidgetRenderer(radio)
+	radioItem := renderer.Objects()[item].(*radioItem)
+	radioItem.Tapped(&fyne.PointEvent{Position: fyne.NewPos(theme.Padding(), theme.Padding())})
+}