silverstripe / silverstripe-userforms

UserForms module provides a visual form builder for the Silverstripe CMS. No coding required to build forms such as contact pages.
BSD 3-Clause "New" or "Revised" License
135 stars 225 forks source link

DropdownField always validates, even when legit selection made #824

Closed phptek closed 5 years ago

phptek commented 5 years ago

framework 4.2.1 / userforms 5.2.1 (also tried 5.2.2) / CWP 2.1.1 / PHP 7.1

I have a basic "Contact Us" form. If I configure it with >=1 dropdown menu; when I make a selection from that dropdown in the frontend, and then submit the form, the form always displays the message: "Please select a value within the list provided. (none) is not a valid option". This is displayed under a dropdown menu regardless of the selection made its options. I have screwed around with the dropdown's config and nothing works. Of course; removing all dropdown's, and the form submits just fine.

If I debug vendor/silverstripe/framework/src/Forms/SingleSelectField.php:validate(), the $selected variable is NULL. Is this possibly related to https://github.com/silverstripe/silverstripe-framework/issues/8022?

NightJar commented 5 years ago

Hi @phptek - are you able to confirm that the HTML output (i.e. before submission) is correct as to expectations please?

phptek commented 5 years ago

@NightJar What HTML output are you after exactly? There is no markup being submitted in the frontend form submission. Let me know exactly what you need and I'll get it to you.

To be clear: The form won't submit if there are 1 or more dropdown field's in it. There is always a validation error, regardless of the selection that is made.

NightJar commented 5 years ago

@phptek I meant the generated HTML that makes up the form, generally containing key information such as value, etc. as attributes. I'd like to rule out that there isn't a problem in rendering that's causing a null value to submit (correctly) every time.

Also, I take it that the configuration for the EditableDropdownField marks it as required? If not, then this is strange behaviour (considering it thinks the value is unselected).

phptek commented 5 years ago

OK, so the problem is probably that the <select> menu isn't being given a name attribute, even though one is displayed and apparently automatically provided and set in the CMS as "EditableTextField_ba610". Changing this to something else does nothing.

I have attached the form markup.

form.txt

NightJar commented 5 years ago

@phptek Are you able to confirm please whether or not your project is using a custom template for forms, form fields, dropdown field, or editable dropdown field in this context?

https://github.com/silverstripe/silverstripe-userforms/blob/7e91be9cb42c5430f67953ff809f78c79abdea9c/code/Model/EditableFormField/EditableDropdown.php#L61-L63

There is nothing too special about the Dropdown field used here, and I should imagine if the Name is unset (debugger can confirm) then it would be unset for all form fields as it is an inherited property from EditableFormField (not to mention one is set if the field is empty at write time).

https://github.com/silverstripe/silverstripe-userforms/blob/621bc754ac7361bf1e6ae56ba5171a36ae0964d7/code/Model/EditableFormField.php#L413-L420

phptek commented 5 years ago

themes/starter/templates/forms/UserFormsField_holder.ss

  1 <div id="$Name" class="field form-group {$ExtraClass} <% if $Message %>has-error<% end_if %>">
  2     <% include FormFieldLabel %>
  3     {$Field}
  4     <% include FormFieldMessage %>
  5     <% include FormFieldDescription %>
  6 </div>

themes/starter/templates/forms/FormField_holder.ss

  1 <div id="{$HolderID.ATT}" class="field form-group {$ExtraClass} <% if $Message %>has-error<% end_if %>">
  2     <% include FormFieldLabel %>
  3     {$Field}
  4     <% include FormFieldMessage %>
  5     <% include FormFieldDescription %>
  6 </div>

themes/starter/templates/forms/UserFormsDropdownField.ss

(Not used - adding an extra fake CSS class "foo" didn't show up)

1 <select {$getAttributesHTML('class')} class="$ExtraClass form-control foo" <% include AriaAttributes %> >
  2     <% loop $Options %>
  3         <option value="{$Value.ATT}" <% if $Selected %>selected="selected"<% end_if %> <% if $Disabled %>disabled="disabled"<% end_if %>>
  4             <% if $Title %>
  5                 {$Title.XML}
  6             <% else %>
  7                 &nbsp;
  8             <% end_if %>
  9         </option>
 10     <% end_loop %>
 11 </select>

In the watea theme, we have no relevant form-templates. I copied across the one from "starter" into the correct dir-structuire, but no dice.

I'll keep poking around...

NightJar commented 5 years ago

@phptek Have you built using Watea as a base? (or is it otherwise one of the themes listed in the cascade?)

phptek commented 5 years ago

Starter + Watea yes.

phptek commented 5 years ago

Sorry, I should have added that info at the beginning in hindsight. Until today, I had assumed it was a backend problem.

NightJar commented 5 years ago

welp, that could be an issue! :s - not with userforms though! Lets just double check what providing a parameter to getAttributesHTML does... but I suspect this will be the cause (note the complete lack of a name attribute specification)

phptek commented 5 years ago

OK, hacking this file with name="TEST" the name attribute appears on the <select> element: themes/starter/templates/SilverStripe/UserForms/Model/EditableFormField/EditableDropdown.ss....

phptek commented 5 years ago

It's missing {$getAttributesHTML('class')}... (Added it, and it works)

NightJar commented 5 years ago

Yeah, the parameter is an exclude, not a filter. So the name attribute should still be present.

~Which file did you edit to make it work @phptek ?~ The useforms templates in starter @NightJar

themes/starter/templates/SilverStripe/UserForms/Model/EditableFormField/EditableDropdown.ss

phptek commented 5 years ago

I added {$getAttributesHTML('class')} to:

themes/starter/templates/SilverStripe/UserForms/Model/EditableFormField/EditableDropdown.ss

I just went looking in the cwp-starter-theme repo, and couldn't find an equivalent version of this template without {$getAttributesHTML('class')}...

  1 <select {$getAttributesHTML('class')} class="dropdown form-control" <% if $RightTitle %>aria-describedby="{$Name}_right_title"<% end_if %>>
  2 <% loop $Options %>
  3         <option value="$Value.XML"<% if $Selected %> selected="selected"<% end_if %><% if $Disabled %> disabled="disabled"<% end_if %>>$Title.XML</option>
  4 <% end_loop %>
  5 </select>
phptek commented 5 years ago

@NightJar - I'm happy to close this now, as the form with the offending menu is now working.

NightJar commented 5 years ago

🤔 image note field name is set here.

Granted I'm testing dev branches here, but lets compare: 5.2.x-dev to 5.2.1 (a few changes) You mentioned also trying 5.2.2, that has essentially no changes.

watea has no changes from the latest release (which judging by other version numbers I'm assuming you're using)

starter theme also has no changes in relation to form fields.

NightJar commented 5 years ago

Are you able to confirm it was project code @phptek ? (git blame perhaps?) Or are you just happy with a work around?

phptek commented 5 years ago

@NightJar AFAICT, the code I have added should have been in there already, when a member of our team added it. So yes, I am "happy" to close this. Sorry to have bothered you. Nothing to see here.

NightJar commented 5 years ago

Neat, thanks for clarifying :) Glad you got it resolved!