ajnsit / concur-documentation

Documentation for Concur
https://ajnsit.github.io/concur-documentation/
66 stars 10 forks source link

inputWidget example doesn't work #3

Closed arianvp closed 5 years ago

arianvp commented 5 years ago

It actually needs some scary unsafeTargetValue stuff to work. Wondering if purescript-concur could make more nice wrappers so that it can be used as in this tutorial

ajnsit commented 5 years ago

Thanks for pointing it out, I need to update the documentation.

BTW the unsafeTargetValue function is only named so because it cannot statically guarantee that the event will have a target and the target will have a value. It is exactly as safe as doing event.target.value in Javascript.

I can provide a nicer sounding prop called onChangeValue which returns a String instead of an event object, but it will have to do a similar unsafe operation in the implementation. I think the current solution of exposing the unsafe-ness to the user, so that they are aware, is a better strategy than that.

I'm open to providing a nicer wrapper with static guarantees, which would ensure that these props can only be applied to input/select tags. But I'm not sure if the extra safety is worth the trouble.

  1. It would require maintaining a list of all possible props for each tag, which may be out of date.
  2. Integration with external components becomes more involved. For example, if there's a custom component with an onChange prop, then we will need to define a separate onChange prop for that component.
  3. Not sure how to handle custom attributes.
  4. There are other unsafe parts of HTML which we still don't handle. For example, you shouldn't put block level elements inside inline elements. UL/OL elements should always have LIs as children and nothing else. Most of these won't cause errors but should the DSL disallow these things too?

As another data point, I think Halogen does provide these static guarantees for its HTML DSL, so perhaps we can follow its model.

ajnsit commented 5 years ago

I change the documentation to include unsafeTargetValue and added a comment on the search for a better API. https://github.com/ajnsit/concur-documentation/commit/b5c8541a584a6b1e9b12d62214209e19645542ca

bbarker commented 5 years ago

I'm wondering about the current usage. Both existing examples have an extra array ([]) in their definitions. I wrote it like this to get them to compile (as in #5):


data Action = Changed String | Focused
inputWidget1 :: Widget HTML Action
inputWidget1 = input [(Changed <<< unsafeTargetValue) <$> onChange, Focused <$ onFocus]

type IWState = {focusCount:: Int, currentText :: String}
inputWidget2 :: IWState -> Widget HTML IWState
inputWidget2 st = input
  [ st {focusCount = st.focusCount+1} <$ onFocus
  , ((\s -> st {currentText = s}) <<< unsafeTargetValue) <$> onChange
  ]

Also, regarding usage of the widget, I might add (if I understand it correctly), that since these widgets exit as soon as they are focused upon, that they can't actually be used.

ajnsit commented 5 years ago

Yup you are right about the extra [] in the docs. Your PR looks good to go once you reinstate the section you inadvertantly deleted (I added a comment to your PR).

About the widget not being usable, virtual-dom is more magical than you'd think. If you don't change any properties on the input widget, the input widget is not actually removed from the dom and things just work. I will create an example for this.

ajnsit commented 5 years ago

Okay I added an example to the purescript-concur repo. Check the Count Focus example in the demo - https://ajnsit.github.io/purescript-concur/.

Source is at - https://github.com/ajnsit/purescript-concur/blob/master/examples/Test/FocusCount.purs.

bbarker commented 5 years ago

@ajnsit I've not used bind much in Haskell or PureScript, but it doesn't appear to be naturally tail recursive as used in the example - or is there some magic happening under the hood?

ajnsit commented 5 years ago

Concur uses trampolining to get tail recursion

ajnsit commented 5 years ago

Closing due to inactivity.