silverstripe-admin icon indicating copy to clipboard operation
silverstripe-admin copied to clipboard

TreeDropdownField fails to validate after clearing value in react forms

Open blueo opened this issue 1 year ago • 6 comments

When using a Treedropdownfield in a react form (eg the Linkfield Module SiteTreeLink) the field will pass a 'required' validation if the field is cleared - even though this should reset the field to an 'empty' state.

Steps to reproduce:

  1. open a FormBuilder Form with a required TreeDropdownField - eg the Link Field module silverstripe/linkfield
  2. Try submitting the form without any changes - observe the field will be correctly marked as 'required'
  3. Add a value to the TreeDropdownField
  4. Clear the TreeDropdownField using the 'x' symbol
  5. submit the form - observe that even though the field is 'empty' it will now pass validation

Investigation it seems that when clearing the field it gets set to a value of 0 https://github.com/silverstripe/silverstripe-admin/blob/2/client/src/components/TreeDropdownField/TreeDropdownField.js#L425 which at some point gets converted to a string.

This means that when Validator.js checks the value of the field it sees '0' and this passes the 'required' validation rule as it is a non-empty string: image by contrast, when the field loads - the value is "" image

Seems we should find another way to clear the field to an empty string instead of 0. If I update SINGLE_EMPTY_VALUE to equal "" then the validation works as expected

this was tested on v 4.13.1 of the admin module - unsure if this is still a problem in v5 at this stage.

Acceptance criteria

  • A common empty value is defined. (probably an empty string)
  • The default value when instantiating the field is the common empty value.
  • When you clear a pre-existing value, the field is reset the the common empty value.

PR

  • https://github.com/silverstripe/silverstripe-admin/pull/1578

blueo avatar Sep 06 '23 00:09 blueo

this was tested on v 4.13.1 of the admin module - unsure if this is still a problem in v5 at this stage

Just to clarify, you have a link to code in the 2 branch, which is specifically CMS 5 code. Can you please update your link if it's incorrect, or else update the description to indicate that both CMS 4 and 5 are affected, if both are affected?

GuySartorelli avatar Sep 06 '23 02:09 GuySartorelli

I don't want to test this with linkfield as that's not a supported module and I'm not familiar with its code.

open a FormBuilder Form with a required TreeDropdownField

Can you please provide me with reproduction instructions for setting this up without using linkfield? Could be in asset-admin or possible elemental inline-editable block if that uses the formbuilder.

GuySartorelli avatar Sep 06 '23 02:09 GuySartorelli

I confirm, this is the issue in CMS4 and CMS5 as well. Replacing SINGLE_EMPTY_VALUE = 0 to SINGLE_EMPTY_VALUE = '' fix this issue and TreeDropdownField fails validation if it's required and empty.

sabina-talipova avatar Sep 07 '23 03:09 sabina-talipova

ah great thanks @sabina-talipova I was having some trouble getting it going in another place!

blueo avatar Sep 07 '23 05:09 blueo

Relates to https://github.com/silverstripe/silverstripe-elemental/issues/329

mfendeksilverstripe avatar Oct 25 '23 02:10 mfendeksilverstripe

We spent half an hour arguing about what the proper empty value should be and coludn't reach a sensible decision. We'll create a SPIKE to review the option and wider implications.

maxime-rainville avatar Oct 29 '23 21:10 maxime-rainville