Issue with controlled inputs

[KristianAN]

Issue with controlled inputs

[Tvaroh]

@davesmith00000 yep, I'm on 0.6.2. I'll make an example. 👍

[KristianAN]

Works for me now as well in 0.6.2.

[davesmith00000]

@Tvaroh That's an interesting question. Is it idiomatic? I don't think it's typical to do that, no, but that certainly wouldn't make it wrong. I think I'd rather ask which approach produces the most maintainable readable code in your scenario.

Generally, a Msg per update type is nice and clean, but it certainly can lead to enormous and unwieldy update functions.

If I had only a few fields, I'd probably just go with a Msg per field. If I had many fields then it would make sense to find a smarter way to manage the updates. One way is to do what you are suggesting and have one Msg like this:

enum Msg:
  case UpdateFromField(update: Model => Model)

That would work. I'm not sure how clean and readable the result would be, but it would work. Try it?

An alternative is to do something like this:

// The usual suspects
final case class Model(fields: Fields):
  def updateFields(next: Fields): Model = this.copy(fields = next)

enum Msg:
  case UpdateFromField(update: FieldMsg)

def update(model: Model): Msg => (Model, Cmd[IO, Msg]) =
  case UpdateFromField(fieldMsg) =>
    val updated = model.updateFields(
      model.fields.update(fieldMsg)
    )
    (updated, Cmd.None)

// These could live in a separate file / module and encaspulate your form updates
final case class Fields(name: String, email: String, password: String):
  def updateName(value: String): Fields = this.copy(name = value)
  def updateEmail(value: String): Fields = this.copy(email = value)
  def updatePassword(value: String): Fields = this.copy(password = value)
  def update: FieldMsg => Fields =
    case FieldMsg.Name(n) => updateName(n)
    case FieldMsg.Email(e) => updateEmail(e)
    case FieldMsg.Password(p) => updatePassword(p)

enum FieldMsg:
  case Name(next: String), Email(next: String), Password(next: String)

I've quickly made that up so it's probably wrong, and it's definitely not the shortest implementation! ...but it's clean, reasonably well typed, exhaustive, and easy to follow.

Hope that helps!

[Tvaroh]

@davesmith00000 yeah, I tried it, and so far it looks pretty good. Thanks for the input.

[Tvaroh]

I'm having a similar issue: on a trivial login screen I render email and password fields, and also a link to the register screen, where there's also name field between those two. But when this navigation happens, the name field value gets the value of the password input from the login screen. Similarly, email field value also gets carried over. I don't even have value attribute nor onInput handlers on the inputs. It looks like DOM diff algorithm is borked. Unless I have to change something.

[davesmith00000]

Sorry for the slow response @krined-dev! This works for me with Tyrian 0.6.2 - I don't know if that means it was fixed in the last release (I did do some changes to properties) or if I'm missing the point? 😄

https://scribble.ninja/u/davesmith00000/bwiylqvyptenautnxugprfhgbyw

package example

import cats.effect.IO
import tyrian.Html.*
import tyrian.*

import scala.scalajs.js.annotation.*

@JSExportTopLevel("TyrianApp")
object Main extends TyrianApp[Msg, Model]:

  def init(flags: Map[String, String]): (Model, Cmd[IO, Msg]) =
    ("", Cmd.None)

  def update(model: Model): Msg => (Model, Cmd[IO, Msg]) =
    case Msg.Set(value) => (value, Cmd.None)

  def view(model: Model): Html[Msg] =
    div(
      form(
        input(
          `type` := "text",
          id     := "myInput",
          value  := model,
          onInput(s => Msg.Set(s.filter(_.isDigit)))
          // onInput { e =>
          //   org.scalajs.dom.document
          //     .getElementById("myInput")
          //     .asInstanceOf[org.scalajs.dom.html.Input]
          //     .value = model // This makes it work
          //   Msg.Set(e.filter(_.isDigit))
          // }
        )
      ),
      p(model)
    )

  def subscriptions(model: Model): Sub[IO, Msg] =
    Sub.None

type Model = String

enum Msg:
  case Set(value: String)

[davesmith00000]

But when this navigation happens, the name field value gets the value of the password input from the login screen. Similarly, email field value also gets carried over.

Hi @Tvaroh! Thanks for reporting. Can you make sure you're on Tyrian 0.6.2? We had a similar issue before the last release which was fixed (although it was about checkboxes...). I thought it was the vdom diff too but it turned out to be the way I was handling properties vs attributes. If it still doesn't work, would you mind trying to make a minimal example that I can test with?

[Tvaroh]

@davesmith00000 here it is:

import cats.effect.IO
import tyrian.*
import tyrian.Html.*

import scala.scalajs.js.annotation.*

@JSExportTopLevel("TyrianApp")
object Main extends TyrianApp[Msg, Model]:

  override def init(flags: Map[String, String]): (Model, Cmd[IO, Msg]) =
    (Model.Login(), Cmd.None)

  override def update(model: Model): Msg => (Model, Cmd[IO, Msg]) =
    case Msg.ToLogin => (Model.Login(), Cmd.None)
    case Msg.ToRegister => (Model.Register(), Cmd.None)

  override def view(model: Model): Html[Msg] =
    model match
      case Model.Login() =>
        form(
          h1("Login"),

          label(`for` := "email")("Email"),
          input(`type` := "email", id := "email", name := "email", placeholder := "Email", required),

          label(`for` := "password")("Password"),
          input(`type` := "password", id := "password", name := "password", placeholder := "Password", required),

          button(id := "login-button", `type` := "submit")("Login"),

          p(text("Don't have an account? "), a(onClick(Msg.ToRegister))("Register"))
        )
      case Model.Register() =>
        form(
          h1("Register"),

          label(`for` := "email")("Email"),
          input(`type` := "email", id := "email", name := "email", placeholder := "Email", required),

          label(`for` := "name")("Name"),
          input(`type` := "name", id := "name", name := "name", placeholder := "Name", required),

          label(`for` := "password")("Password"),
          input(`type` := "password", id := "password", name := "password", placeholder := "Password", required),

          button(id := "login-button", `type` := "submit")("Register"),

          p(text("Already have an account? "), a(onClick(Msg.ToLogin))("Login"))
        )

  override def subscriptions(model: Model): Sub[IO, Msg] =
    Sub.None

  def main(args: Array[String]): Unit =
    launch("myapp")

enum Model:
  case Login()
  case Register()

enum Msg:
  case ToLogin
  case ToRegister

Try entering something into email and password and switching between the views.

[davesmith00000]

Hi @Tvaroh,

Ok I've had a look. So you're on the right lines, this is a wrinkle of working with a VirtualDom (and would have been similar to this original issue before the last release). There is no fix for me to do (I don't think), but I can show you how to make it work with the example below.

What's going on is that Tyrian has no idea what is in the text boxes, and the only things being set are the value properties - but that's being done in the browser, not the App. So when you click the button to switch views, the VDom does the right thing, it mutates the view to be what you told it to be, but the property (i.e. the values in the input fields), that property only exists in the browsers view of the world at this point, and so the values seem to float around.

To get the result you want, you need to hook the value property of each field into Tyrian's life cycle and store the data on purpose.

You can paste this into https://scribble.ninja/ and it should work. I'm not promising this is good modelling, I did it very quickly! 😄

package example

import cats.effect.IO
import tyrian.*
import tyrian.Html.*

import scala.scalajs.js.annotation.*

@JSExportTopLevel("TyrianApp")
object Main extends TyrianApp[Msg, Model]:

  override def init(flags: Map[String, String]): (Model, Cmd[IO, Msg]) =
    (Model.initial, Cmd.None)

  override def update(model: Model): Msg => (Model, Cmd[IO, Msg]) =
    case Msg.ToLogin =>
      (model.copy(state = ModelState.Login), Cmd.None)

    case Msg.ToRegister =>
      (model.copy(state = ModelState.Register), Cmd.None)

    case Msg.UpdateEmail(value) =>
      (model.updateEmail(value), Cmd.None)

    case Msg.UpdateName(value) =>
      (model.updateName(value), Cmd.None)

    case Msg.UpdatePassword(value) =>
      (model.updatePassword(value), Cmd.None)

  override def view(model: Model): Html[Msg] =
    model.state match
      case ModelState.Login =>
        form(
          h1("Login"),

          label(`for` := "email")("Email"),
          input(
            `type` := "text",
            id := "email",
            name := "email",
            placeholder := "Email",
            required,
            value := model.login.email,
            onInput(Msg.UpdateEmail(_))
          ),

          label(`for` := "password")("Password"),
          input(
            `type` := "text",
            id := "password",
            name := "password",
            placeholder := "Password",
            required,
            value := model.login.password,
            onInput(Msg.UpdatePassword(_))
          ),

          button(id := "login-button", `type` := "submit")("Login"),

          p(text("Don't have an account? "), a(onClick(Msg.ToRegister))("Register"))
        )
      case ModelState.Register =>
        form(
          h1("Register"),

          label(`for` := "email")("Email"),
          input(
            `type` := "text",
            id := "email",
            name := "email",
            placeholder := "Email",
            required,
            value := model.register.email,
            onInput(Msg.UpdateEmail(_))
          ),

          label(`for` := "name")("Name"),
          input(
            `type` := "text",
            id := "name",
            name := "name",
            placeholder := "Name",
            required,
            value := model.register.name,
            onInput(Msg.UpdateName(_))
          ),

          label(`for` := "password")("Password"),
          input(
            `type` := "text",
            id := "password",
            name := "password",
            placeholder := "Password",
            required,
            value := model.register.password,
            onInput(Msg.UpdatePassword(_))
          ),

          button(id := "login-button", `type` := "submit")("Register"),

          p(text("Already have an account? "), a(onClick(Msg.ToLogin))("Login"))
        )

  override def subscriptions(model: Model): Sub[IO, Msg] =
    Sub.None

  def main(args: Array[String]): Unit =
    launch("myapp")

final case class Model(state: ModelState, login: Login, register: Register):
  def updateEmail(value: String): Model =
    this.copy(
      login = login.copy(email = value),
      register = register.copy(email = value)
    )
  def updateName(value: String): Model =
    this.copy(
      register = register.copy(name = value)
    )
  def updatePassword(value: String): Model =
    this.copy(
      login = login.copy(password = value),
      register = register.copy(password = value)
    )
object Model:
  val initial: Model = Model(ModelState.Login, Login("", ""), Register("", "", ""))

enum ModelState:
  case Login
  case Register

final case class Login(email: String, password: String)
final case class Register(email: String, name: String, password: String)

enum Msg:
  case ToLogin
  case ToRegister
  case UpdateEmail(value: String)
  case UpdateName(value: String)
  case UpdatePassword(value: String)

Hope that helps. I should document this stuff somewhere...

[davesmith00000]

I'm going to close this for now as I think it's working as expected. Please feel free to re-open if there is more to discuss.

[Tvaroh]

@davesmith00000 thanks, it works. Is it idiomatic to create a single msg type holding a lambda for state update, instead of introducing custom message for each field in the app?