Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force validation on non dirty fields? #32

Closed
dariooddenino opened this issue Sep 20, 2018 · 6 comments
Closed

Force validation on non dirty fields? #32

dariooddenino opened this issue Sep 20, 2018 · 6 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@dariooddenino
Copy link

Hi, not sure if I missed something in the code (and sorry for the spam), but here's what I need to do:

I have an email field and a "send email" checkbox. The email field becomes mandatory when the the checkbox is checked.
The problem is that I want the "missing email" error to appear as soon as the user checks the "send email" field and I had to resort to this solution which feels a little hacky:

      eval (HandleFormless (F.Changed f) next) = next <$ do
         ...
         -- if the value of the check changed:
          let e = F.getInput prx.email f.form
          void $ H.query' CP.cp1 unit $ F.modifyValidate_ prx.email e

I can't use validate_, as nothing will happen if the email field was still untouched.

@thomashoneyman
Copy link
Owner

The current design of the library only validates fields that are in a touched state. To run validation on the field, you need to ensure the field is touched. That's a bit unfortunate -- it might be nicer to allow validation to always run, and you can use the field's 'touched' value to decide if your validation should simply pass. On the other hand, that means you've got to write this code for all your validators that is the default in most cases.

I don't have a firm opinion one way or another, but as this seemed like a source of unnecessary boilerplate I opted to just have validators run on touched fields.

Short-term solution

OK -- enough background! Until anything about that design has been changed, this is one approach you could take. Here's a code snippet from the "real world" example for when you'd like to validate that one secret key field is equal to the other one:

renderSecretKey1 :: FormlessState -> FormlessHTML
renderSecretKey1 st =
 UI.input
   { label: "Secret Key 1"
   , help: UI.resultToHelp
             "Provide a secret identifier for the group"
             (F.getResult prx.secretKey1 st.form)
   , placeholder: "ia30<>Psncdi3b#$<0423"
   }
   [ HP.value $ F.getInput prx.secretKey1 st.form
   , HE.onValueInput $ HE.input \str -> F.AndThen
       (F.modifyValidate_ prx.secretKey1 str)
       (F.validate_ prx.secretKey2)
  ]

The tricky part about this situation is that secretKey2 needs to re-run its validation every time secretKey1 is modified, so it can check whether they are still equal or not. So when we modify secretKey1, we'll also run validation on secretKey2. The same event triggers both queries.

This is possible with the AndThen query from Formless, which simply lets you provide two queries to run in succession and can be chained together as many times as you'd like:

F.AndThen
   (F.DoThis value)
   (F.AndThen
      (F.DoThat value)
      (F.DoThen value)
    )

You could move this code...

let e = F.getInput prx.email f.form
void $ H.query' CP.cp1 unit $ F.modifyValidate_ prx.email e

... to instead be part of your rendering for the checkbox, so that when the checkbox is toggled, the email field is modified to its same value and validated. It's not THAT much better, but at least it's not cluttering up your eval anymore.

Longer-term solutions

There are some other ways that things could be better:

  1. I could create set vs. modify queries, so that modify instead takes a function. Then you don't have to pull out the value in the first place, and can just do modifyValidate_ prx.email identity. This might be useful in general.

  2. I could create queries for setting more than just input values, like setTouched and setResult so you can imperatively update a field to a particular value at any point. For example, you could then just have your email checkbox run setTouched prx.email.

  3. While this is more of a stretch, I could have validation not automatically ignore touched fields, and instead rely on you to decide how you want to treat touched fields. I'm reluctant to do this because of how much more boilerplate gets introduced because of it -- and because validation would now need to take the entire form field instead of just the input -- but there is an argument that could be made that I shouldn't be making these decisions on your behalf. That said, for the time being, I do not plan to go this direction.

Let me know if this helps!

@dariooddenino
Copy link
Author

dariooddenino commented Sep 21, 2018

Well, I have to disagree: this solution is way better as it helps keeping all form related things to the form component without leaking to the parent.
Sadly, right now I'm using some wrapper functions that build the form elements automatically from the symbol, so I have no easy way to plug that chained validation easily.
I will look into a solution, as it looks pretty good tbh :)

Regarding the long-term solutions:

  1. This is a good idea, also to keep consistency with halogen
  2. It looks a little dangerous and misleading on what's the best way to use the library. Maybe setTouched can be helpful, but I'd avoid setResult
  3. Yeah I don't like it. Maybe having a different validation function that ignores touched could be a partial solution, but probably it's not necessary.

Thanks again! :)

EDIT:
Ugh, I spent 3 hours abstracting my form components functions to accept an optional input handler just to notice that passing a AndThen wasn't actually the solution to my problem 🗡️
Oh well, at least I cleaned up my code and it was a good exercise...

@thomashoneyman
Copy link
Owner

EDIT: Ugh, I spent 3 hours abstracting my form components functions to accept an optional input handler just to notice that passing a AndThen wasn't actually the solution to my problem 🗡

Oh no! Did you find an alternate solution? Sorry that didn't work for you; I'd love to hear why.

@dariooddenino
Copy link
Author

Well, yes the problem was still that I had to validation on the untouched field, but in the end I decided it wasn't that important!
In the process I improved my code a bit, by moving all the logic out of the parent component.
Most of it went in the email validator:

checkoutEmail
  :: forall form m e o e1 o1 t.
     Newtype (form Record FormField)  { sendVoucher :: FormField e Boolean o, sendEmail :: FormField e1 Boolean o1 | t } =>
     Monad m =>
     Validation form m FieldError String (Maybe String)
checkoutEmail = Validation $ \form email ->
  let sV = F.getInput (SProxy :: SProxy "sendVoucher") form
      sE = F.getInput (SProxy :: SProxy "sendEmail") form
  in if sV || sE
     then runValidation (nonEmptyStr >>> validEmail >>> F.hoistFn_ Just) form email
     else runValidation (acceptEmpty validEmail) form email

And I ended up with this abstraction that will let me use chained validators in the future:

defaults ::
  forall rall rsub rx.
  Row.Union rsub rall rx =>
  Row.Nub rx rall =>
  { | rsub } ->
  { | rall } ->
  { | rall }
defaults = Record.merge

type FieldConfigRow' r =
  ( label       :: String
  , id_         :: Maybe String
  , help        :: String
  , required    :: Boolean
  , disabled    :: Boolean
  , placeholder :: String
  , expanded    :: Boolean
  | r
  )

type FieldConfigRow s pq cq cs form out m a i =
  FieldConfigRow'
  ( sym :: SProxy s
  , onInput :: Maybe (i -> Unit -> F.Query pq cq cs form out m Unit)
  )

type FieldConfig' = Record (FieldConfigRow' ())
type FieldConfig sym pq cq cs form out m a i = Record (FieldConfigRow sym pq cq cs form out m a i)

fieldConf' :: FieldConfig'
fieldConf' = { label: ""
             , id_: Nothing
             , help: ""
             , required: false
             , disabled: false
             , placeholder: ""
             , expanded: false
             }

fieldConf :: forall sym. FieldConfig sym
fieldConf = Record.merge { sym: SProxy :: SProxy sym } fieldConf'

fieldConf :: forall sym pq cq cs form out m a i. FieldConfig_ sym pq cq cs form out m a i
fieldConf = Record.merge { onInput: Nothing, sym: SProxy :: SProxy sym } fieldConf'

buildConf' ::
  forall rsub rx.
  Row.Nub rx (FieldConfigRow' ()) =>
  Row.Union rsub (FieldConfigRow' ()) rx =>
  { | rsub } ->
  FieldConfig'
buildConf' r = defaults r fieldConf'

buildConf ::
  forall sym rsub rx pq cq cs form out m a e i o t1 inputs.
  IsSymbol sym =>
  Newtype (form Variant F.InputField) (Variant inputs) =>
  Row.Cons sym (F.InputField e i o) t1 inputs =>
  Row.Nub rx (FieldConfigRow_ sym pq cq cs form out m a i) =>
  Row.Union rsub (FieldConfigRow_ sym pq cq cs form out m a i) rx =>
  { | rsub } ->
  FieldConfig_ sym pq cq cs form out m a i
buildConf r = defaults r fieldConf

inputHandler ::
  forall sym pq cq cs form out m a e i o inputs t1.
  IsSymbol sym =>
  Newtype (form Variant F.InputField) (Variant inputs) =>
  Row.Cons sym (F.InputField e i o) t1 inputs =>
  FieldConfig sym pq cq cs form out m a i ->
  (i -> Unit -> F.Query pq cq cs form out m Unit)
inputHandler { sym, onInput } = case onInput of
  Nothing -> F.modifyValidate sym
  Just oi -> oi


-- An example checkbox
checkbox ::
  forall sym pq cq cs form out m a e t0 t1 inputs rsub rx fields.
  IsSymbol sym =>
  Newtype (form Record F.FormField) { | fields } =>
  Newtype (form Variant F.InputField) (Variant inputs) =>
  Row.Cons sym (F.FormField e Boolean Boolean) t0 fields =>
  Row.Cons sym (F.InputField e Boolean Boolean) t1 inputs =>
  Row.Nub rx (FieldConfigRow sym pq cq cs form out m a Boolean) =>
  Row.Union rsub (FieldConfigRow sym pq cq cs form out m a Boolean) rx =>
  { | rsub} ->
  F.State form out m ->
  H.ParentHTML (F.Query pq cq cs form out m) cq cs m
checkbox conf { form } =
  let c :: FieldConfig sym pq cq cs form out m a Boolean
      c = buildConf conf
  in
   UI.singleField
   [ HH.label
     [ HP.class_ B.checkbox ]
     [ HH.input
       [ HP.type_ HP.InputCheckbox
       , HP.checked $ F.getInput c.sym form
       , HE.onChecked $ HE.input (inputHandler c)
       , HP.disabled c.disabled
       ]
     , HH.text c.label
     ]
   ]

This lets me build working formless fields with as little as:

Form.checkbox { label: "Hello", sym: (SProxy :: SProxy "helloField") } state

@thomashoneyman
Copy link
Owner

@dariooddenino This issue is closed in #34, using the first solution proposed above.

@thomashoneyman thomashoneyman self-assigned this Oct 20, 2018
@thomashoneyman thomashoneyman added this to the 0.3.0 milestone Oct 20, 2018
@thomashoneyman thomashoneyman added the enhancement New feature or request label Oct 20, 2018
@thomashoneyman
Copy link
Owner

Closed by #33 and #34 merged to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants