FIX Hide and show form fields by toggling the "hide" class instead of jQuery methods

code/model/EditableCustomRule.php

@@ -189,15 +189,21 @@ class EditableCustomRule extends DataObject
             case 'ValueNot':
                 if ($checkboxField) {
                     if ($formFieldWatch->isCheckBoxGroupField()) {
-                        $expression = sprintf("$.inArray('%s', %s.filter(':checked').map(function(){ return $(this).val();}).get()) > -1",
+                        $expression = sprintf(
-                            $fieldValue, $target);
+                            "$.inArray('%s', %s.filter(':checked').map(function(){ return $(this).val();}).get()) > -1",
+                            $fieldValue,
+                            $target
+                        );
                     } else {
                         $expression = "{$target}.prop('checked')";
                     }
                 } elseif ($radioField) {
                     // We cannot simply get the value of the radio group, we need to find the checked option first.
-                    $expression = sprintf('%s.closest(".field, .control-group").find("input:checked").val() == "%s"',
+                    $expression = sprintf(
-                        $target, $fieldValue);
+                        '%s.closest(".field, .control-group").find("input:checked").val() == "%s"',
+                        $target,
+                        $fieldValue
+                    );
                 } else {
                     $expression = sprintf('%s.val() == "%s"', $target, $fieldValue);
                 }
@@ -211,8 +217,12 @@ class EditableCustomRule extends DataObject
             case 'ValueLessThanEqual':
             case 'ValueGreaterThan':
             case 'ValueGreaterThanEqual':
-                $expression = sprintf('%s.val() %s parseFloat("%s")', $target,
+                $expression = sprintf(
-                    $conditionOptions[$this->ConditionOption], $fieldValue);
+                    '%s.val() %s parseFloat("%s")',
+                    $target,
+                    $conditionOptions[$this->ConditionOption],
+                    $fieldValue
+                );
                 break;
             default:
                 throw new LogicException("Unhandled rule {$this->ConditionOption}");
@@ -228,14 +238,14 @@ class EditableCustomRule extends DataObject
     }
 
     /**
-     * Returns the opposite of the show/hide pairs of strings
+     * Returns the opposite visibility function for the value of the initial visibility field, e.g. show/hide. This
+     * will toggle the "hide" class either way, which is handled by CSS.
      *
      * @param string $text
-     *
      * @return string
      */
     public function toggleDisplayText($text)
     {
-        return (strtolower($text) === 'show') ? 'hide' : 'show';
+        return (strtolower($text) === 'hide') ? 'removeClass("hide")' : 'addClass("hide")';
     }
-}
+}

code/model/UserDefinedForm.php

@@ -760,26 +760,21 @@ JS
             $conjunction = $rule['conjunction'];
             $operations = implode(" {$conjunction} ", $rule['operations']);
             $target = $rule['targetFieldID'];
-            $initialState = $rule['initialState'];
-            $view = $rule['view'];
-            $opposite = $rule['opposite'];
 
             $result .= <<<EOS
 \n
     $('.userform').on('{$events}',
     "{$selectors}",
     function (){
-        if({$operations}) {
+        if ({$operations}) {
-            $('{$target}').{$view}();
+            $('{$target}').{$rule['view']};
         } else {
-            $('{$target}').{$opposite}();
+            $('{$target}').{$rule['opposite']};
         }
     });
 EOS;
-
         }
 
         return $result;
     }
-
 }

code/model/editableformfields/EditableFormField.php

@@ -1095,9 +1095,10 @@ class EditableFormField extends DataObject
             }
             $result['operations'][] = $expression['operation'];
 
-            //View/Show should read
+            // View/Show should read
+            $opposite = ($result['initialState'] === 'hide') ? 'show' : 'hide';
             $result['view'] = $rule->toggleDisplayText($result['initialState']);
-            $result['opposite'] = $rule->toggleDisplayText($result['view']);
+            $result['opposite'] = $rule->toggleDisplayText($opposite);
         }
 
         return (count($result['selectors'])) ? $result : null;

tests/EditableCustomRuleTest.php

@@ -28,4 +28,15 @@ class EditableCustomRuleTest extends SapphireTest
         //Check for greater than sign
         $this->assertContains('>', $result2['operation']);
     }
-}
+
+    /**
+     * Test that methods are returned for manipulating the presence of the "hide" CSS class depending
+     * on whether the field should be hidden or shown
+     */
+    public function testToggleDisplayText()
+    {
+        $rule1 = $this->objFromFixture('EditableCustomRule', 'rule1');
+        $this->assertSame('addClass("hide")', $rule1->toggleDisplayText('show'));
+        $this->assertSame('removeClass("hide")', $rule1->toggleDisplayText('hide'));
+    }
+}

tests/EditableFormFieldTest.php

@@ -205,12 +205,16 @@ class EditableFormFieldTest extends FunctionalTest
         $this->assertEquals(20, $attributes['data-rule-maxlength']);
     }
 
-	public function testFormatDisplayRules()
+    public function testFormatDisplayRules()
-	{
+    {
-		/** @var EditableCheckbox $checkbox */
+        $field = $this->objFromFixture('EditableFormField', 'irdNumberField');
-		$checkbox = $this->objFromFixture('EditableFormField', 'irdNumberField');
+        $displayRules = $field->formatDisplayRules();
-		$displayRules = $checkbox->formatDisplayRules();
+        $this->assertNotNull($displayRules);
-		$this->assertNotNull($displayRules);
+        $this->assertCount(1, $displayRules['operations']);
-		$this->assertCount(1, $displayRules['operations']);
+
-	}
+        // Field is initially visible, so the "view" method should be to hide it
+        $this->assertSame('addClass("hide")', $displayRules['view']);
+        // The opposite method should be to return it to its original state, i.e. show it again
+        $this->assertSame('removeClass("hide")', $displayRules['opposite']);
+    }
 }