PurpleKingdomGames / tyrian

Elm-inspired Scala UI library.
https://tyrian.indigoengine.io/
MIT License
346 stars 26 forks source link

Consider invoking preventDefault on a form's button #110

Closed chuwy closed 1 year ago

chuwy commented 2 years ago

Given a following form:

<form>
  <input type=text name=email />
  <input type=password name=password />
  <button onclick=submit()>Login</button>
</form>

It must be quite common to do e.preventDefault() as first thing in submit callback, because otherwise clicking on the button makes the browser to send a form and reload current page.

It's possible in plain JS (and had to do in scalajs-react), but Tyrian doesn't allow me to call it, so I had to revert to a href=#. However, I believe it must be a defult behaviout for any SPA as we don't want to reload page.

davesmith00000 commented 2 years ago

I was a bit uncertain about just blanket disabling this, so I had a read around. This is Elm's solution to the problem, TL;DR: There is a separate way of invoking the event where you supply options for preventDefault and stopPropagation.

https://stackoverflow.com/a/38907948

hobnob commented 2 years ago

For clarity, Elm 0.19 prevents default on the submit action, which seems reasonable - we should probably have a similar approach to onSubmit

In terms of preventDefault in other events, I'm not sure whether I prefer the old Elm (config passed to the event) or the newer preventDefaultOn approach in Elm 0.19. Both work, and I think it's really a matter of preference, but a wider discussion I think for a different issue 🙂

davesmith00000 commented 2 years ago

I think there is a workaround here if anyone has a moment to try it? (@chuwy?)

onSubmit is defined as follows:

def onSubmit[M](msg: M): Attr[M] = onEvent("submit", (_: Tyrian.Event) => msg)

So for now, I think all you need to do is make something like this and drop it in in place for the onSubmit attribute:

val submitMyForm =
  onEvent("submit", { (e: Tyrian.Event) =>
    e.preventDefault()
    myMsg
  })

Maybe?

davesmith00000 commented 2 years ago

So! I'm not going to fix this right now because I'm still wondering how that should look (from an API perspective).

However, there is a workaround as above, and I've tried it out.