From f1c408d3f4f7506b1a142a3d4670608bfe91c3af Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 13 Aug 2015 11:31:37 +1200 Subject: [PATCH] BUG Fix form submission BUG Fixed display logic --- .../UserFormFieldEditorExtension.php | 28 +----- code/formfields/UserFormsStepField.php | 37 ++++++++ code/forms/UserForm.php | 29 ++++--- code/model/UserDefinedForm.php | 38 +++----- .../EditableCheckboxGroupField.php | 10 +++ .../EditableCountryDropdownField.php | 4 + .../editableformfields/EditableDropdown.php | 4 + .../editableformfields/EditableFieldGroup.php | 7 ++ .../EditableFieldGroupEnd.php | 8 ++ .../editableformfields/EditableFormField.php | 87 +++++++++++++++++-- .../EditableFormHeading.php | 7 ++ .../EditableLiteralField.php | 8 ++ .../EditableMultipleOptionField.php | 8 ++ .../editableformfields/EditableRadioField.php | 6 ++ javascript/UserForm.js | 2 +- templates/Includes/UserFormProgress.ss | 34 ++++---- templates/Includes/UserFormStepErrors.ss | 8 ++ templates/Includes/UserFormStepNav.ss | 46 ++++------ templates/UserForm.ss | 41 ++------- templates/UserFormsStepField.ss | 16 ++++ 20 files changed, 273 insertions(+), 155 deletions(-) create mode 100644 templates/Includes/UserFormStepErrors.ss create mode 100644 templates/UserFormsStepField.ss diff --git a/code/extensions/UserFormFieldEditorExtension.php b/code/extensions/UserFormFieldEditorExtension.php index 29e342b..8e6f47e 100644 --- a/code/extensions/UserFormFieldEditorExtension.php +++ b/code/extensions/UserFormFieldEditorExtension.php @@ -37,7 +37,7 @@ class UserFormFieldEditorExtension extends DataExtension { $this->createInitialFormStep(true); $editableColumns = new GridFieldEditableColumns(); - $fieldClasses = $this->getEditableFieldClasses(); + $fieldClasses = singleton('EditableFormField')->getEditableFieldClasses(); $editableColumns->setDisplayFields(array( 'ClassName' => function($record, $column, $grid) use ($fieldClasses) { if($record instanceof EditableFormField) { @@ -121,32 +121,6 @@ class UserFormFieldEditorExtension extends DataExtension { $fields->add($step); } - /** - * @return array - */ - public function getEditableFieldClasses() { - $classes = ClassInfo::getValidSubClasses('EditableFormField'); - - // Remove classes we don't want to display in the dropdown. - $editableFieldClasses = array(); - foreach ($classes as $class) { - if(in_array($class, array('EditableFormField', 'EditableMultipleOptionField')) - || Config::inst()->get($class, 'hidden') - ) { - continue; - } - - $singleton = singleton($class); - if(!$singleton->canCreate()) { - continue; - } - - $editableFieldClasses[$class] = $singleton->i18n_singular_name(); - } - - return $editableFieldClasses; - } - /** * Ensure that at least one page exists at the start */ diff --git a/code/formfields/UserFormsStepField.php b/code/formfields/UserFormsStepField.php index efcf563..8cddc6f 100644 --- a/code/formfields/UserFormsStepField.php +++ b/code/formfields/UserFormsStepField.php @@ -4,4 +4,41 @@ * Represents a page step in a form, which may contain form fields or other groups */ class UserFormsStepField extends UserFormsCompositeField { + + private static $casting = array( + 'StepNumber' => 'Int' + ); + + /** + * Numeric index (1 based) of this step + * + * Null if unassigned + * + * @var int|null + */ + protected $number = null; + + public function FieldHolder($properties = array()) { + return $this->Field($properties); + } + + /** + * Get the step number + * + * @return int|null + */ + public function getStepNumber() { + return $this->number; + } + + /** + * Re-assign this step to another number + * + * @param type $number + * @return $this + */ + public function setStepNumber($number) { + $this->number = $number; + return $this; + } } diff --git a/code/forms/UserForm.php b/code/forms/UserForm.php index cf16333..8477442 100644 --- a/code/forms/UserForm.php +++ b/code/forms/UserForm.php @@ -22,6 +22,12 @@ class UserForm extends Form { $this->getRequiredFields() ); + // Number each page + $stepNumber = 1; + foreach($this->getSteps() as $step) { + $step->setStepNumber($stepNumber++); + } + if($controller->DisableCsrfSecurityToken) { $this->disableSecurityToken(); } @@ -45,24 +51,21 @@ class UserForm extends Form { } /** - * @return int + * @return bool */ public function getDisplayErrorMessagesAtTop() { - return $this->controller->DisplayErrorMessagesAtTop; + return (bool)$this->controller->DisplayErrorMessagesAtTop; } /** - * @return array + * Return the fieldlist, filtered to only contain steps + * + * @return ArrayList */ - public function getNumberOfSteps() { - $steps = new ArrayList(); - $numberOfSteps = $this->controller->Fields()->filter('ClassName', 'EditableFormStep')->Count(); - - for($i = 0; $i < $numberOfSteps; $i++) { - $steps->push($i); - } - - return $steps; + public function getSteps() { + return $this->Fields()->filterByCallback(function($field) { + return $field instanceof UserFormsStepField; + }); } /** @@ -123,9 +126,7 @@ class UserForm extends Form { ->filter('Required', true) ->column('Name'); $required = new RequiredFields($requiredNames); - $this->extend('updateRequiredFields', $required); - return $required; } diff --git a/code/model/UserDefinedForm.php b/code/model/UserDefinedForm.php index 5b5f215..1286966 100755 --- a/code/model/UserDefinedForm.php +++ b/code/model/UserDefinedForm.php @@ -394,15 +394,11 @@ class UserDefinedForm_Controller extends Page_Controller { if($this->Fields()) { foreach($this->Fields() as $field) { - $fieldId = $field->Name; - - if($field instanceof EditableFormHeading) { - $fieldId = 'UserForm_Form_' . $field->Name; - } + $holderSelector = $field->getSelectorHolder(); // Is this Field Show by Default if(!$field->ShowOnLoad) { - $default .= "$(\"#" . $fieldId . "\").hide();\n"; + $default .= "{$holderSelector}.hide();\n"; } // Check for field dependencies / default @@ -411,22 +407,8 @@ class UserDefinedForm_Controller extends Page_Controller { // Get the field which is effected $formFieldWatch = EditableFormField::get()->byId($rule->ConditionFieldID); - if($formFieldWatch->RecordClassName == 'EditableDropdown') { - // watch out for multiselect options - radios and check boxes - $fieldToWatch = "$(\"select[name='" . $formFieldWatch->Name . "']\")"; - $fieldToWatchOnLoad = $fieldToWatch; - } else if($formFieldWatch->RecordClassName == 'EditableCheckboxGroupField') { - // watch out for checkboxs as the inputs don't have values but are 'checked - $fieldToWatch = "$(\"input[name='" . $formFieldWatch->Name . "[" . $rule->FieldValue . "]']\")"; - $fieldToWatchOnLoad = $fieldToWatch; - } else if($formFieldWatch->RecordClassName == 'EditableRadioField') { - $fieldToWatch = "$(\"input[name='" . $formFieldWatch->Name . "']\")"; - // We only want to trigger on load once for the radio group - hence we focus on the first option only. - $fieldToWatchOnLoad = "$(\"input[name='" . $formFieldWatch->Name . "']:first\")"; - } else { - $fieldToWatch = "$(\"input[name='" . $formFieldWatch->Name . "']\")"; - $fieldToWatchOnLoad = $fieldToWatch; - } + $fieldToWatch = $formFieldWatch->getSelectorField($rule); + $fieldToWatchOnLoad = $formFieldWatch->getSelectorField($rule, true); // show or hide? $view = ($rule->Display == 'Hide') ? 'hide' : 'show'; @@ -436,7 +418,7 @@ class UserDefinedForm_Controller extends Page_Controller { // @todo encapulsation $action = "change"; - if($formFieldWatch->ClassName == "EditableTextField") { + if($formFieldWatch instanceof EditableTextField) { $action = "keyup"; } @@ -454,11 +436,11 @@ class UserDefinedForm_Controller extends Page_Controller { // and what should we evaluate switch($rule->ConditionOption) { case 'IsNotBlank': - $expression = ($checkboxField || $radioField) ? '$(this).prop("checked")' :'$(this).val() != ""'; + $expression = ($checkboxField || $radioField) ? '$(this).is(":checked")' :'$(this).val() != ""'; break; case 'IsBlank': - $expression = ($checkboxField || $radioField) ? '!($(this).prop("checked"))' : '$(this).val() == ""'; + $expression = ($checkboxField || $radioField) ? '!($(this).is(":checked"))' : '$(this).val() == ""'; break; case 'HasValue': @@ -507,7 +489,7 @@ class UserDefinedForm_Controller extends Page_Controller { $watch[$fieldToWatch][] = array( 'expression' => $expression, - 'field_id' => $fieldId, + 'holder_selector' => $holderSelector, 'view' => $view, 'opposite' => $opposite, 'action' => $action @@ -526,9 +508,9 @@ class UserDefinedForm_Controller extends Page_Controller { foreach($values as $rule) { // Register conditional behaviour with an element, so it can be triggered from many places. $logic[] = sprintf( - 'if(%s) { $("#%s").%s(); } else { $("#%2$s").%s(); }', + 'if(%s) { %s.%s(); } else { %2$s.%s(); }', $rule['expression'], - $rule['field_id'], + $rule['holder_selector'], $rule['view'], $rule['opposite'] ); diff --git a/code/model/editableformfields/EditableCheckboxGroupField.php b/code/model/editableformfields/EditableCheckboxGroupField.php index 17ea2fa..bac39f7 100755 --- a/code/model/editableformfields/EditableCheckboxGroupField.php +++ b/code/model/editableformfields/EditableCheckboxGroupField.php @@ -44,4 +44,14 @@ class EditableCheckboxGroupField extends EditableMultipleOptionField { } return $result; } + + public function getSelectorField(EditableCustomRule $rule, $forOnLoad = false) { + // watch out for checkboxs as the inputs don't have values but are 'checked + // @todo - Test this + if($rule->FieldValue) { + return "$(\"input[name='{$this->Name}[]'][value='{$rule->FieldValue}']\")"; + } else { + return "$(\"input[name='{$this->Name}[]']:first\")"; + } + } } diff --git a/code/model/editableformfields/EditableCountryDropdownField.php b/code/model/editableformfields/EditableCountryDropdownField.php index 2485658..741f51b 100644 --- a/code/model/editableformfields/EditableCountryDropdownField.php +++ b/code/model/editableformfields/EditableCountryDropdownField.php @@ -38,4 +38,8 @@ class EditableCountryDropdownField extends EditableFormField { public function getIcon() { return USERFORMS_DIR . '/images/editabledropdown.png'; } + + public function getSelectorField(EditableCustomRule $rule, $forOnLoad = false) { + return "$(\"select[name='{$this->Name}']\")"; + } } \ No newline at end of file diff --git a/code/model/editableformfields/EditableDropdown.php b/code/model/editableformfields/EditableDropdown.php index a0aa534..a8d6ad4 100755 --- a/code/model/editableformfields/EditableDropdown.php +++ b/code/model/editableformfields/EditableDropdown.php @@ -38,4 +38,8 @@ class EditableDropdown extends EditableMultipleOptionField { $this->doUpdateFormField($field); return $field; } + + public function getSelectorField(EditableCustomRule $rule, $forOnLoad = false) { + return "$(\"select[name='{$this->Name}']\")"; + } } \ No newline at end of file diff --git a/code/model/editableformfields/EditableFieldGroup.php b/code/model/editableformfields/EditableFieldGroup.php index 24c0bfe..8ccd735 100644 --- a/code/model/editableformfields/EditableFieldGroup.php +++ b/code/model/editableformfields/EditableFieldGroup.php @@ -17,6 +17,13 @@ class EditableFieldGroup extends EditableFormField { */ private static $hidden = true; + /** + * Non-data field type + * + * @var type + */ + private static $literal = true; + public function getCMSFields() { $fields = parent::getCMSFields(); $fields->removeByName(array('MergeField', 'Default', 'Validation', 'DisplayRules')); diff --git a/code/model/editableformfields/EditableFieldGroupEnd.php b/code/model/editableformfields/EditableFieldGroupEnd.php index e1a6e6f..51afe3c 100644 --- a/code/model/editableformfields/EditableFieldGroupEnd.php +++ b/code/model/editableformfields/EditableFieldGroupEnd.php @@ -16,6 +16,14 @@ class EditableFieldGroupEnd extends EditableFormField { * @var bool */ private static $hidden = true; + + /** + * Non-data type + * + * @config + * @var bool + */ + private static $literal = true; public function getCMSTitle() { $group = $this->Group(); diff --git a/code/model/editableformfields/EditableFormField.php b/code/model/editableformfields/EditableFormField.php index 1dc0c2a..ece9c34 100755 --- a/code/model/editableformfields/EditableFormField.php +++ b/code/model/editableformfields/EditableFormField.php @@ -16,6 +16,22 @@ class EditableFormField extends DataObject { */ private static $hidden = false; + /** + * Define this field as abstract (not inherited) + * + * @config + * @var bool + */ + private static $abstract = true; + + /** + * Flag this field type as non-data (e.g. literal, header, html) + * + * @config + * @var bool + */ + private static $literal = false; + /** * Default sort order * @@ -176,21 +192,25 @@ class EditableFormField extends DataObject { } // Validation - $fields->addFieldsToTab( - 'Root.Validation', - $this->getFieldValidationOptions() - ); - + $validationFields = $this->getFieldValidationOptions(); + if($validationFields) { + $fields->addFieldsToTab( + 'Root.Validation', + $this->getFieldValidationOptions() + ); + } + $allowedClasses = array_keys($this->getEditableFieldClasses(false)); $editableColumns = new GridFieldEditableColumns(); $editableColumns->setDisplayFields(array( 'Display' => '', - 'ConditionFieldID' => function($record, $column, $grid) { + 'ConditionFieldID' => function($record, $column, $grid) use ($allowedClasses) { return DropdownField::create( $column, '', EditableFormField::get() ->filter(array( - 'ParentID' => $this->ParentID + 'ParentID' => $this->ParentID, + 'ClassName' => $allowedClasses )) ->exclude(array( 'ID' => $this->ID @@ -651,4 +671,57 @@ class EditableFormField extends DataObject { ->setAttribute('placeholder', _t('EditableFormField.TITLE', 'Title')) ->setAttribute('data-placeholder', _t('EditableFormField.TITLE', 'Title')); } + + /** + * Get the JS expression for selecting the holder for this field + * + * @return string + */ + public function getSelectorHolder() { + return "$(\"#{$this->Name}\")"; + } + + /** + * Gets the JS expression for selecting the value for this field + * + * @param EditableCustomRule $rule Custom rule this selector will be used with + * @param bool $forOnLoad Set to true if this will be invoked on load + */ + public function getSelectorField(EditableCustomRule $rule, $forOnLoad = false) { + return "$(\"input[name='{$this->Name}']\")"; + } + + + /** + * Get the list of classes that can be selected and used as data-values + * + * @param $includeLiterals Set to false to exclude non-data fields + * @return array + */ + public function getEditableFieldClasses($includeLiterals = true) { + $classes = ClassInfo::getValidSubClasses('EditableFormField'); + + // Remove classes we don't want to display in the dropdown. + $editableFieldClasses = array(); + foreach ($classes as $class) { + // Skip abstract / hidden classes + if(Config::inst()->get($class, 'abstract', Config::UNINHERITED) || Config::inst()->get($class, 'hidden') + ) { + continue; + } + + if(!$includeLiterals && Config::inst()->get($class, 'literal')) { + continue; + } + + $singleton = singleton($class); + if(!$singleton->canCreate()) { + continue; + } + + $editableFieldClasses[$class] = $singleton->i18n_singular_name(); + } + + return $editableFieldClasses; + } } diff --git a/code/model/editableformfields/EditableFormHeading.php b/code/model/editableformfields/EditableFormHeading.php index 90c84ef..c23b01e 100755 --- a/code/model/editableformfields/EditableFormHeading.php +++ b/code/model/editableformfields/EditableFormHeading.php @@ -11,6 +11,8 @@ class EditableFormHeading extends EditableFormField { private static $plural_name = 'Headings'; + private static $literal = true; + private static $db = array( 'Level' => 'Int(3)', // From CustomSettings 'HideFromReports' => 'Boolean(0)' // from CustomSettings @@ -57,6 +59,7 @@ class EditableFormHeading extends EditableFormField { public function getFormField() { $labelField = new HeaderField($this->Name, $this->EscapedTitle, $this->Level); $labelField->addExtraClass('FormHeading'); + $labelField->setAttribute('data-id', $this->Name); $this->doUpdateFormField($labelField); return $labelField; } @@ -80,4 +83,8 @@ class EditableFormHeading extends EditableFormField { public function getFieldValidationOptions() { return false; } + + public function getSelectorHolder() { + return "$(\":header[data-id='{$this->Name}']\")"; + } } diff --git a/code/model/editableformfields/EditableLiteralField.php b/code/model/editableformfields/EditableLiteralField.php index eaba903..109d4a9 100644 --- a/code/model/editableformfields/EditableLiteralField.php +++ b/code/model/editableformfields/EditableLiteralField.php @@ -13,6 +13,14 @@ class EditableLiteralField extends EditableFormField { private static $plural_name = 'HTML Blocks'; + /** + * Mark as literal only + * + * @config + * @var bool + */ + private static $literal = true; + /** * Get the name of the editor config to use for HTML sanitisation. Defaults to the active config. * diff --git a/code/model/editableformfields/EditableMultipleOptionField.php b/code/model/editableformfields/EditableMultipleOptionField.php index 31b4b58..9574f3f 100644 --- a/code/model/editableformfields/EditableMultipleOptionField.php +++ b/code/model/editableformfields/EditableMultipleOptionField.php @@ -14,6 +14,14 @@ */ class EditableMultipleOptionField extends EditableFormField { + + /** + * Define this field as abstract (not inherited) + * + * @config + * @var bool + */ + private static $abstract = true; private static $has_many = array( "Options" => "EditableOption" diff --git a/code/model/editableformfields/EditableRadioField.php b/code/model/editableformfields/EditableRadioField.php index 3bce18b..fff5809 100755 --- a/code/model/editableformfields/EditableRadioField.php +++ b/code/model/editableformfields/EditableRadioField.php @@ -35,4 +35,10 @@ class EditableRadioField extends EditableMultipleOptionField { $this->doUpdateFormField($field); return $field; } + + public function getSelectorField(EditableCustomRule $rule, $forOnLoad = false) { + // We only want to trigger on load once for the radio group - hence we focus on the first option only. + $first = $forOnLoad ? ':first' : ''; + return "$(\"input[name='{$this->Name}']{$first}\")"; + } } diff --git a/javascript/UserForm.js b/javascript/UserForm.js index 1aca9c1..3ed68c1 100644 --- a/javascript/UserForm.js +++ b/javascript/UserForm.js @@ -139,7 +139,7 @@ jQuery(function ($) { */ UserForm.prototype.setCurrentStep = function (step) { // Make sure we're dealing with a form step. - if (!step instanceof FormStep) { + if (!(step instanceof FormStep)) { return; } diff --git a/templates/Includes/UserFormProgress.ss b/templates/Includes/UserFormProgress.ss index 7496a89..95f57b7 100644 --- a/templates/Includes/UserFormProgress.ss +++ b/templates/Includes/UserFormProgress.ss @@ -1,20 +1,18 @@ -<% cached "UserForms_Navigation", $LastEdited %> -<% if $NumberOfSteps.Count > "1" %> - <% end_if %> -<% end_cached %> diff --git a/templates/Includes/UserFormStepErrors.ss b/templates/Includes/UserFormStepErrors.ss new file mode 100644 index 0000000..8e97344 --- /dev/null +++ b/templates/Includes/UserFormStepErrors.ss @@ -0,0 +1,8 @@ +<% if $Steps.Count > 1 %> + +<% end_if %> diff --git a/templates/Includes/UserFormStepNav.ss b/templates/Includes/UserFormStepNav.ss index c776a3c..fd9720c 100644 --- a/templates/Includes/UserFormStepNav.ss +++ b/templates/Includes/UserFormStepNav.ss @@ -1,29 +1,21 @@ -<% if $FirstLast == "first last" %> -<% else %> - <% end_if %> diff --git a/templates/UserForm.ss b/templates/UserForm.ss index 7badf6d..e90ca23 100644 --- a/templates/UserForm.ss +++ b/templates/UserForm.ss @@ -1,44 +1,19 @@ -<% include UserFormProgress %> -
+ +<% include UserFormProgress %> +<% include UserFormStepErrors %> <% if $Message %> -

$Message

+

$Message

<% else %> - -<% end_if %> - -<% if $NumberOfSteps.Count > "1" %> - + <% end_if %>
<% if $Legend %>$Legend<% end_if %> - - <% if $FormFields%><% loop $FormFields %> -
- <% if $Top.DisplayErrorMessagesAtTop %> - - <% end_if %> - - <% loop $Children %> - $FieldHolder - <% end_loop %> - - <% include UserFormStepNav ContainingPage=$Top %> -
- <% end_loop %><% end_if %> - + <% loop $Fields %> + $FieldHolder + <% end_loop %>
diff --git a/templates/UserFormsStepField.ss b/templates/UserFormsStepField.ss new file mode 100644 index 0000000..12c7847 --- /dev/null +++ b/templates/UserFormsStepField.ss @@ -0,0 +1,16 @@ +
+ <% if $Form.DisplayErrorMessagesAtTop %> + + <% end_if %> + + <% loop $Children %> + $FieldHolder + <% end_loop %> + + <% include UserFormStepNav %> +