widget: rebuild form item render slot when its Widget is replaced (#6459)

* widget: rebuild form item render slot when its Widget is replaced

ensureRenderItems() only ever appended render objects for newly added
Items, so replacing an existing FormItem.Widget after the form had
already been rendered was silently ignored by Refresh(), unlike Text
changes which are applied in place by updateLabels().

Fixes #1966

* widget: reset stale per-widget FormItem state on widget swap

Address review feedback on the previous commit: when an item's Widget
is swapped, also detach the old widget's validation callbacks and
reset validationError, invalid, wasFocused and helperOutput, so none
of it leaks from the widget that used to occupy the slot.

Author: LunatikDG

widget/form.go

@@ -220,6 +220,31 @@ func (f *Form) createInput(item *FormItem) fyne.CanvasObject {
 	return &fyne.Container{Layout: formItemLayout{form: f}, Objects: []fyne.CanvasObject{item.Widget, textContainer}}
 }
 
+// unwrapItemWidget returns the widget actually shown by rendered, stripping the
+// hint/validation container createInput sometimes wraps it in.
+func (*Form) unwrapItemWidget(rendered fyne.CanvasObject) fyne.CanvasObject {
+	if c, ok := rendered.(*fyne.Container); ok && len(c.Objects) > 0 {
+		return c.Objects[0]
+	}
+	return rendered
+}
+
+func (f *Form) itemRendersWidget(rendered, widget fyne.CanvasObject) bool {
+	return f.unwrapItemWidget(rendered) == widget
+}
+
+// detachValidation unhooks the callbacks setUpValidation registered on widget, so a
+// caller that keeps interacting with a widget no longer shown by the form can't have
+// it write stale state into the FormItem slot it used to occupy.
+func (*Form) detachValidation(widget fyne.CanvasObject) {
+	if v, ok := widget.(fyne.Validatable); ok {
+		v.SetOnValidationChanged(nil)
+	}
+	if r, ok := widget.(fyne.Requireable); ok {
+		r.SetOnRequiredChanged(nil)
+	}
+}
+
 func (*Form) itemWidgetHasValidator(w fyne.CanvasObject) bool {
 	value := reflect.ValueOf(w).Elem()
 	validatorField := value.FieldByName("Validator")
@@ -327,6 +352,23 @@ func (f *Form) checkValidation(err error) {
 
 func (f *Form) ensureRenderItems() {
 	done := len(f.itemGrid.Objects) / 2
+	for i := 0; i < done && i < len(f.Items); i++ {
+		item := f.Items[i]
+		old := f.itemGrid.Objects[i*2+1]
+		if f.itemRendersWidget(old, item.Widget) {
+			continue
+		}
+
+		f.detachValidation(f.unwrapItemWidget(old))
+		item.validationError = nil
+		item.invalid = false
+		item.wasFocused = false
+		item.helperOutput = nil
+
+		f.setUpValidation(item.Widget, i)
+		f.itemGrid.Objects[i*2+1] = f.createInput(item)
+	}
+
 	if done >= len(f.Items) {
 		f.itemGrid.Objects = f.itemGrid.Objects[0 : len(f.Items)*2]
 		return

widget/form_test.go

@@ -142,6 +142,95 @@ func TestForm_ChangeText(t *testing.T) {
 	assert.Equal(t, "Changed", c.Objects[0].(*RichText).String())
 }
 
+func TestForm_ChangeItemWidget(t *testing.T) {
+	oldEntry := NewEntry()
+	item := NewFormItem("Test", oldEntry)
+	form := NewForm(item)
+	test.TempWidgetRenderer(t, form)
+
+	assert.Equal(t, oldEntry, form.itemGrid.Objects[1])
+
+	newEntry := NewEntry()
+	item.Widget = newEntry
+	form.Refresh()
+
+	assert.Equal(t, newEntry, form.itemGrid.Objects[1])
+}
+
+func TestForm_ChangeItemWidget_ResetsValidation(t *testing.T) {
+	test.NewTempApp(t)
+
+	invalidEntry := &Entry{Validator: validation.NewRegexp(`^\d+$`, "must be numeric"), Text: "not-a-number"}
+	item := NewFormItem("Test", invalidEntry)
+	form := &Form{Items: []*FormItem{item}, OnSubmit: func() {}}
+	test.NewTempWindow(t, form)
+
+	assert.True(t, item.invalid)
+	assert.Error(t, item.validationError)
+	assert.True(t, form.submitButton.Disabled())
+
+	item.Widget = NewLabel("static") // not fyne.Validatable
+	form.Refresh()
+
+	assert.False(t, item.invalid)
+	assert.NoError(t, item.validationError)
+	assert.False(t, form.submitButton.Disabled())
+}
+
+func TestForm_ChangeItemWidget_ResetsWasFocused(t *testing.T) {
+	test.NewTempApp(t)
+
+	entry := &Entry{}
+	item := &FormItem{Text: "Test", Widget: entry, HintText: "a hint"}
+	form := &Form{Items: []*FormItem{item}}
+	test.NewTempWindow(t, form)
+
+	entry.FocusGained()
+	entry.FocusLost()
+	form.Refresh()
+	assert.True(t, item.wasFocused)
+
+	item.Widget = &Entry{}
+	form.Refresh()
+
+	assert.False(t, item.wasFocused)
+}
+
+func TestForm_ChangeItemWidget_DetachesOldWidget(t *testing.T) {
+	test.NewTempApp(t)
+
+	oldEntry := &Entry{Validator: validation.NewRegexp(`^\d+$`, "must be numeric")}
+	item := NewFormItem("Test", oldEntry)
+	form := &Form{Items: []*FormItem{item}, OnSubmit: func() {}}
+	test.NewTempWindow(t, form)
+
+	item.Widget = &Entry{}
+	form.Refresh()
+
+	// oldEntry is no longer part of the form; validating it must not write into item.
+	oldEntry.Text = "not-a-number"
+	oldEntry.Validate()
+
+	assert.False(t, item.invalid)
+	assert.NoError(t, item.validationError)
+}
+
+func TestForm_ChangeItemWidget_ClearsHelperOutput(t *testing.T) {
+	test.NewTempApp(t)
+
+	entry := &Entry{Validator: validation.NewRegexp(`^\d+$`, "must be numeric"), Text: "not-a-number"}
+	item := NewFormItem("Test", entry)
+	form := &Form{Items: []*FormItem{item}, OnSubmit: func() {}}
+	test.NewTempWindow(t, form)
+
+	assert.NotNil(t, item.helperOutput)
+
+	item.Widget = NewLabel("static")
+	form.Refresh()
+
+	assert.Nil(t, item.helperOutput)
+}
+
 func TestForm_ChangeTheme(t *testing.T) {
 	test.NewTempApp(t)