xilem_masonry: Add filter ("All", "Active", "Completed") to todomvc example

[Philipp-M]

This adds the filters of TodoMVC via checkboxes currently (as we currently don't have better widgets for that...).

It also revealed (at least) 2 bugs, one being fixed by #264 and another I (or someone else) needs to investigate (difficult to describe just add a few todos, check them, and click all the filters, it should be easy to reproduce).

[PoignardAzur]

Nicely done, but as you point out, this PR makes these other bugs very visible. I'd rather not merge it until they are fixed, since this is our showcase. ^^

[Philipp-M]

Sure, I also think this should only be merged, when it doesn't produce obvious bugs at this point :). But I think it's probably good for debugging these bugs.

[Philipp-M]

(Now this weird issue can be debugged, since #264 is merged)

[bram209]

I ran in (I think) the same issue that you are describing, put some reproduction steps here: https://xi.zulipchat.com/#narrow/stream/354396-xilem/topic/TodoMVC.20example/near/437314215

[Philipp-M]

Since #278 is merged, I've tested this again, it works now as expected

[jaredoconnell]

I tested this. I think this filter should be placed before the list: This makes it so that it doesn't move around when you're using it.

I do find that it's in need of polish. You're using multi-select checkboxes as radio buttons. If we're going with multi-select, you technically don't want an All option, and instead check both options. I don't know what level of polish we're going for this week.

[xStrom]

I think this filter should be placed before the list: This makes it so that it doesn't move around when you're using it.

For what it's worth, it also moves around in the examples on todomvc.com.

[Philipp-M]

Yes checkboxes are not ideal indeed, these should be exchanged with the right widgets, when they're implemented.

I'm not sure how much we're striving towards the original TodoMVC.

[PoignardAzur]

I've been looking through our PR backlog, and I'd be interested in merging this, if everything works correctly after a rebase.

I'm fine having checkboxes masquerade as combo buttons for now, as long as we track it in an issue. We can add a big ugly "TODO - replace with combo buttons" comment to the example.

[Philipp-M]

I've rebased it (+ added comment), and well it revealed yet another new bug^^ (this example seems to be a quite good test as well)

To reproduce: Press "Active" and then "Buy milk"

[DJMcNab]

Hmm, so the behaviour we see is that the checkbox "thinks" it is clicked, but without sending an action.

That's very strange

[Philipp-M]

I rebased this again, to check whether that issue resolved itself - not the case. I've found another issue (that is actually the issue for the other one as well): It's possible to toggle the checkboxes, whereas I'd expect, that these should behave more like radio-buttons, since they should be driven by xilem.

So the issue is related to two-way dataflow and non-keyed widgets/viewsequences. I.e. masonry modifies the checkbox value while it shouldn't do it (at least in this case). This opens the broader question about two-way/unidirectional dataflow/state again.

I think this is an example, that two-way dataflow can be a pitfall for declarative frameworks on top of masonry. I think it would be great to find a general clean solution for this issue (something like opt-in control of the state in masonry, as in this case).

I've pushed a "fix"/workaround, which should make this work as expected.

[PoignardAzur]

This seems like a very "brute force" solution to the two-way dataflow problem, buuuuut nobody has a better idea, so we may as well ship it.