intercellular / cell

A self-driving web app framework
https://www.celljs.org
MIT License
1.5k stars 94 forks source link

Setting $text to empty string does nothing? #20

Closed arpsmack closed 7 years ago

arpsmack commented 7 years ago

I'm curious why setting $text to "" is ignored (https://github.com/intercellular/cell/blob/develop/cell.js#L139)

This means you can't clear the contents of a cell, no? This breaks things, like your "Super simple synchronized input" example. Type a bunch of stuff in the text field, hit Ctrl+A, backspace. The old text still remains because setting $text to "" was ignored.

guscost commented 7 years ago

I think updates to a $text in a node do not update the actual text in the DOM for some reason, you have to update $components at a higher level.

With this PR text nodes can be described as just a string: https://github.com/intercellular/cell/pull/21

And the more verbose syntax could be removed entirely, eliminating this issue.

guscost commented 7 years ago

Here's a demo with plain strings added to $components in order to render a total. It uses a flux-esque approach that I'm tinkering with: https://gist.github.com/guscost/0e52f4b3c3cf680ba20b43477e81da36

Caffeinix commented 7 years ago

@guscost: @gliechtenstein commented on #13 that there is already a way to add plain text nodes to $components:

  $components: [{
    $type: "img",
    src: "http://localhost/img.jpg"
  }, {
    $type: "text",
    $text: "Plain Text"
  }]

Using plain strings in the $components array might be cleaner, and I agree that it would be nice if the semantics of $text weren't so different from those of $components, but at least it's possible to get the correct behavior today.

guscost commented 7 years ago

Have you tried updating the $text of a node in the $update function? Either I'm missing something or that's a pitfall, since it wasn't updating when I tried to do it.

gliechtenstein commented 7 years ago

I have just fixed this with the latest commit here https://github.com/intercellular/cell/commit/3ed08cfda7e24b5672ff667ad926147d1da24059

Please pull from the develop branch and retry, and let me know if that fixes it. I can confirm it's working on mine.

gliechtenstein commented 7 years ago

@guscost as for your last comment about $text update inside $update() not triggering a render, that's not supposed to happen.

Could you share a full example? That would help me understand the situation better.

arpsmack commented 7 years ago

Just tried the synchronization demo with the dev code and it works. Nice.