Fix checkbox tag in form and add support for array field

gobuffalo · Oct 8, 2019 · 746cfa8 · 746cfa8
1 parent ca0da11
commit 746cfa8
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 33 deletions.
diff --git a/form/bootstrap/form_for_test.go b/form/bootstrap/form_for_test.go
@@ -157,7 +157,7 @@ func Test_CheckBox(t *testing.T) {
 	r := require.New(t)
 	f := bootstrap.NewFormFor(struct{ Name string }{}, tags.Options{})
 	l := f.CheckboxTag("Name", tags.Options{"label": "Custom"})
-	r.Equal(`<div class="form-group"><label><input class="" id="-Name" name="Name" type="checkbox" value="" /><input name="Name" type="hidden" value="" /> Custom</label></div>`, l.String())
+	r.Equal(`<div class="form-group"><label><input class="" id="-Name" name="Name" type="checkbox" value="" /> Custom</label></div>`, l.String())
 }
 
 func Test_InputError(t *testing.T) {
@@ -229,7 +229,7 @@ func Test_CheckBoxError(t *testing.T) {
 
 	f := bootstrap.NewFormFor(struct{ Name string }{}, tags.Options{"errors": errors})
 	l := f.CheckboxTag("Name", tags.Options{"label": "Custom"})
-	r.Equal(`<div class="form-group has-error"><label><input class=" is-invalid" id="-Name" name="Name" type="checkbox" value="" /><input name="Name" type="hidden" value="" /> Custom</label><div class="invalid-feedback help-block">Name shoud be AJ.</div></div>`, l.String())
+	r.Equal(`<div class="form-group has-error"><label><input class=" is-invalid" id="-Name" name="Name" type="checkbox" value="" /> Custom</label><div class="invalid-feedback help-block">Name shoud be AJ.</div></div>`, l.String())
 }
 
 type Person struct {

diff --git a/form/checkbox_tag.go b/form/checkbox_tag.go
@@ -13,35 +13,22 @@ func (f Form) CheckboxTag(opts tags.Options) *tags.Tag {
 
 	value := opts["value"]
 	delete(opts, "value")
-	if value != nil {
-		opts["value"] = value
-	}
 
 	checked := opts["checked"]
 	delete(opts, "checked")
 	if checked == nil {
 		checked = "true"
 	}
 
-	if value == nil {
-		opts["value"] = checked
+	isChecked := template.HTMLEscaper(value) == template.HTMLEscaper(checked)
+	if isChecked || value == nil {
+		value = checked
 	}
 
-	isChecked := template.HTMLEscaper(value) == template.HTMLEscaper(checked)
+	opts["value"] = value
 
 	unchecked := opts["unchecked"]
 	delete(opts, "unchecked")
-	if unchecked != nil {
-		isUnchecked := template.HTMLEscaper(value) == template.HTMLEscaper(unchecked)
-		if isUnchecked {
-			isChecked = false
-
-			if value != "" {
-				delete(opts, "value")
-
-			}
-		}
-	}
 
 	hl := opts["hide_label"]
 	delete(opts, "hide_label")

diff --git a/form/form_for.go b/form/form_for.go
@@ -74,21 +74,34 @@ func loadErrors(opts tags.Options) *validate.Errors {
 
 // CheckboxTag creates a checkbox for a field on the form Struct
 func (f FormFor) CheckboxTag(field string, opts tags.Options) *tags.Tag {
+	f.buildCheckboxOptions(field, opts)
 	f.buildOptions(field, opts)
-	f.validateCheck(field, opts)
 
 	return f.Form.CheckboxTag(opts)
 }
 
-func (f FormFor) validateCheck(field string, opts tags.Options) {
-	fv := f.value(field)
-	if fv != nil && fv != "" {
-		opts["checked"] = fv
-		return
+func (f FormFor) buildCheckboxOptions(field string, opts tags.Options) {
+	fn := f.reflection.FieldByName(field)
+	ov := opts["value"]
+	if fn.Kind() == reflect.Slice || fn.Kind() == reflect.Array && ov != nil {
+		s := reflect.ValueOf(fn.Interface())
+		for i := 0; i < s.Len(); i++ {
+			if reflect.DeepEqual(s.Index(i).Interface(), ov) {
+				opts["checked"] = s.Index(i)
+				break
+			}
+		}
 	}
 
-	opts["unchecked"] = fv
-	delete(opts, "checked")
+	if fn.Kind() == reflect.Bool {
+		if ov == nil {
+			opts["value"] = true
+
+			if opts["checked"] == nil && !fn.Bool() {
+				opts["checked"] = fn.Bool()
+			}
+		}
+	}
 }
 
 // InputTag creates an input for a field on the form Struct

diff --git a/form/form_for_test.go b/form/form_for_test.go
@@ -342,7 +342,7 @@ func Test_FormFor_CheckboxTag_With_Value(t *testing.T) {
 
 	f := form.NewFormFor(p, tags.Options{})
 	i := f.CheckboxTag("UserType", tags.Options{})
-	r.Equal(`<label><input id="-UserType" name="UserType" type="checkbox" value="ADMIN" checked /></label>`, i.String())
+	r.Equal(`<label><input id="-UserType" name="UserType" type="checkbox" value="ADMIN" /></label>`, i.String())
 }
 
 type testObject struct {
@@ -360,29 +360,51 @@ func Test_FormFor_CheckboxTag_Cases(t *testing.T) {
 		{
 			Object:   testObject{1},
 			Options:  tags.Options{},
-			Expected: `<label><input id="test-object-Field" name="Field" type="checkbox" value="1" checked /></label>`,
+			Expected: `<label><input id="test-object-Field" name="Field" type="checkbox" value="1" /></label>`,
 		},
 		{
 			Object:   testObject{"Text"},
 			Options:  tags.Options{},
-			Expected: `<label><input id="test-object-Field" name="Field" type="checkbox" value="Text" checked /></label>`,
+			Expected: `<label><input id="test-object-Field" name="Field" type="checkbox" value="Text" /></label>`,
 		},
 		{
 			Object:   testObject{true},
 			Options:  tags.Options{},
 			Expected: `<label><input id="test-object-Field" name="Field" type="checkbox" value="true" checked /></label>`,
 		},
 		{
-			Object:   testObject{""},
+			Object:   testObject{},
 			Options:  tags.Options{},
-			Expected: `<label><input id="test-object-Field" name="Field" type="checkbox" value="" /><input name="Field" type="hidden" value="" /></label>`,
+			Expected: `<label><input id="test-object-Field" name="Field" type="checkbox" value="true" /></label>`,
+		},
+		{
+			Object: testObject{},
+			Options: tags.Options{
+				"tag_only": true,
+			},
+			Expected: `<input id="test-object-Field" name="Field" type="checkbox" value="true" />`,
+		},
+		{
+			Object: testObject{},
+			Options: tags.Options{
+				"tag_only": true,
+			},
+			Expected: `<input id="test-object-Field" name="Field" type="checkbox" value="true" />`,
+		},
+		{
+			Object: testObject{},
+			Options: tags.Options{
+				"checked":   false,
+				"unchecked": true,
+			},
+			Expected: `<label><input id="test-object-Field" name="Field" type="checkbox" value="false" /><input name="Field" type="hidden" value="true" /></label>`,
 		},
 		{
 			Object: testObject{1},
 			Options: tags.Options{
 				"unchecked": 1,
 			},
-			Expected: `<label><input id="test-object-Field" name="Field" type="checkbox" /><input name="Field" type="hidden" value="1" /></label>`,
+			Expected: `<label><input id="test-object-Field" name="Field" type="checkbox" value="1" /><input name="Field" type="hidden" value="1" /></label>`,
 		},
 	}