OlivierBlanvillain / monadic-html

Tiny DOM binding library for Scala.js
https://olivierblanvillain.github.io/monadic-html/examples/
MIT License
225 stars 24 forks source link

Allow Rx[Option[T]] #29

Closed olafurpg closed 7 years ago

olafurpg commented 7 years ago

Failing test

  test("Updating option attribute") {
    val id: Var[Option[String]] = Var(Option("oldId"))
    val hr: Elem = <hr id={id}/>
    val div = dom.document.createElement("div")
    mount(div, hr)
    assert(div.innerHTML == """<hr id="oldId">""")
    id := Option("newId")
    assert(div.innerHTML == """<hr id="newId">""")
    id := None
    assert(div.innerHTML == """<hr>""")
  }
// error

[info] - Updating option attribute *** FAILED *** (5 milliseconds)
[info]   "<hr id="[Some(oldId)]">" did not equal "<hr id="[oldId]">" (tests.scala:62)
OlivierBlanvillain commented 7 years ago

Don't you prefer being explicit here:?

val hr: Node = id.map(_.fold(<hr>)(i => <hr id={i}>))

This kind of custom magic can be achieved using the transformAtom setting for elements, and IIRC it was not possible on attributes... But maybe it's doable now that we control the xml better?

olafurpg commented 7 years ago

The <hr> element could be a large form that I don't want to re-render on every update because I might loose focus on active <input> elements. I would prefer to pass in a Rx[Option[T]] to the attribute alone, leveraging precise binding.

OlivierBlanvillain commented 7 years ago

Do you have a use case where you actually want the attribute to be removed? Because <hr id={id.getOrElse("")}> should do the trick for ids...

olafurpg commented 7 years ago

Yes, for example disabled in <form disabled>...</form> should be removed if you want to activate that form in bootstrap. I could use null to remove the attribute, but I would prefer to use Option[T].

OlivierBlanvillain commented 7 years ago

I see. It's weird with disabled because we can't even be express with scala-xml:

scala> <form disabled></form>
<console>:1: error: in XML literal: '=' expected instead of '>'
<form disabled></form>
              ^
<console>:1: error: in XML literal: ' or " delimited attribute value or '{' scala-expr '}' expected
<form disabled></form>
               ^
<console>:1: error: in XML literal: '>' expected instead of 'f'
<form disabled></form>
                 ^
OlivierBlanvillain commented 7 years ago

hmm

scala> <form disabled={Some(null)}></form>
res4: scala.xml.Elem = <form></form>

scala> <form disabled=""></form>
res5: scala.xml.Elem = <form disabled=""></form>
olafurpg commented 7 years ago

The semantics I want

scala> <form disabled={None}></form>
scala> <form disabled={false}></form>
res4: scala.xml.Elem = <form></form>

scala> <form disabled=""></form>
scala> <form disabled={Some("")}></form>
scala> <form disabled={true}></form>
res5: scala.xml.Elem = <form disabled=""></form>

scala> <form disabled={Some(null)}></form> // BOOM! runtime error. don't do that shit.

Binding.scala allows booleans as optional attributes and I think that's quite nice.

OlivierBlanvillain commented 7 years ago

Ok I'm sold for putting semantic into Some/None. These would be compile time errors otherwise, so we might as well do what the user most likely wants!

olafurpg commented 7 years ago

What about the booleans? It's really common in frameworks like bootstrap that you don't care about the value inside Some(value), just if the attribute is active or not.

OlivierBlanvillain commented 7 years ago

I would personally be very surprised by

scala> <form disabled={false}></form>
res4: scala.xml.Elem = <form></form>

scala> <form disabled={true}></form>
res5: scala.xml.Elem = <form disabled=""></form>

None / Some("") seams to make more sense for this output... but whatever

olafurpg commented 7 years ago

Cool. Boolean is a nice way to show that the value inside is irrelevant, this is the most common case in my experience.