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

Multiple child node fix #134

Merged
merged 1 commit into from
Dec 24, 2017
Merged

Conversation

mariusmuja
Copy link
Collaborator

Fixes a bug when there are multiple child nodes and only one emits.

On top of #130.

Copy link
Member

@fdietze fdietze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

I have to mention, that these kinds of annoyances only occur, because we abuse an event library to represent (UI) state. We need workarounds because we allow an inconsistent state to be defined in the first place (instead of requiring a default value in each variable). This results in inserting fake-content in the form of empty string-nodes or absent attributes which is not always the desired solution. If it is not totally clear what I'm saying here, please ask.

It is possible to define further, similarly failing tests.

  it should "partially render component even if parts not present" in {
    val messagesColor = PublishSubject[String]
    val messagesBgColor = PublishSubject[String]
    val childString = PublishSubject[String]

    val vNode = div(
      color <-- messagesColor,
      backgroundColor <-- messagesBgColor,
      child <-- childString
    )

    val node = document.createElement("div").asInstanceOf[html.Div]
    document.body.appendChild(node)
    OutWatch.renderInto(node, vNode).unsafeRunSync()

    node.innerHTML shouldBe "<div></div>"
    node.style.color shouldBe ""
    node.style.backgroundColor shouldBe ""

    childString.onNext("fish")
    node.innerHTML shouldBe "<div>fish</div>"
    node.style.color shouldBe ""
    node.style.backgroundColor shouldBe ""

    messagesColor.onNext("red")
    node.innerHTML shouldBe "<div>fish</div>"
    node.style.color shouldBe "red"
    node.style.backgroundColor shouldBe ""

    messagesBgColor.onNext("blue")
    node.innerHTML shouldBe "<div>fish</div>"
    node.style.color shouldBe "red"
    node.style.backgroundColor shouldBe "blue"
  }

We have to ask ourselves if this should be the desired behavior. What to do instead, or if we can forbid inconsistent state via the type system. For Monix, see monix/monix#279.

always related: #96

@fdietze fdietze merged commit 5ef03ac into outwatch:master Dec 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants