bodil / vgtk

A declarative desktop UI framework for Rust built on GTK and Gtk-rs
http://bodil.lol/vgtk/
Other
1.05k stars 37 forks source link

todomvc example not working correctly #76

Open emi2k01 opened 3 years ago

emi2k01 commented 3 years ago

Reproduce

When you add n elements to the app, then go to the active tab and start toggling the tasks from top to bottom, the bottom tasks will re-toggle the top tasks. This is because of the way the filtering is done.

Suppose we have the following tasks, ["task1", "task2", "task3", "task4"], and only "task4" is active.

When the Active filter is applied, we get an iterator containing only ["task4"] and because the program uses .enumerate(), it gives "task4" the index 0 because it's the only element in the active tasks. So, when we toggle "task4" which has the index 0, the program instead toggles ["task1"]. That is, the program uses the index of the filtered tasks to delete from the global vector of tasks.

Another problem?

But I think there's another problem and I just started playing with vgtk so I don't know know if I'm just doing something wrong or not.

After I tried to fix the problem I just described, I noticed that the indices weren't being updated and I think there's an error with the vdom-diffing code.

It doesn't matter what index is passed to Item::render, the value of the index passed to Msg::Filter is always based on the position.

Apply the following patch to see what I mean.

diff --git a/examples/todomvc/src/app.rs b/examples/todomvc/src/app.rs
index cb6f1b32..7a2433dc 100644
--- a/examples/todomvc/src/app.rs
+++ b/examples/todomvc/src/app.rs
@@ -81,8 +81,12 @@ impl Model {
                 <ScrolledWindow Box::expand=true Box::fill=true>
                     <ListBox selection_mode=SelectionMode::None>
                         {
-                            self.filter(self.filter).enumerate()
-                                .map(|(index, item)| item.render(index))
+                            self.items.iter().enumerate().filter(|(_, item)| match self.filter {
+                                Filter::All => true,
+                                Filter::Active => !item.done,
+                                Filter::Completed => item.done,
+                            })
+                            .map(|(index, item)| item.render(index))
                         }
                     </ListBox>
                 </ScrolledWindow>
@@ -151,6 +155,7 @@ impl Component for Model {
                 self.clean = false;
             }
             Msg::Toggle { index } => {
+                println!("TOGGLE {}", index);
                 Arc::make_mut(&mut self.items)[index].done = !self.items[index].done;
                 self.clean = false;
             }
diff --git a/examples/todomvc/src/items.rs b/examples/todomvc/src/items.rs
index c8ad1eed..40de2ac2 100644
--- a/examples/todomvc/src/items.rs
+++ b/examples/todomvc/src/items.rs
@@ -34,6 +34,9 @@ impl Item {
         } else {
             self.task.clone()
         };
+
+        println!("INDEX OF \"{}\" is {} ", self.task, index);
+
         gtk! {
             <ListBoxRow>
                 <Box spacing=10 orientation=Orientation::Horizontal>

Here's a video showing what I mean: https://streamable.com/1il11r

When the item b moves to the old position of a, it acts as if it were a. If I delete or toggle b, the action will be applied to a. Then when c gets to the old position of a or b, the same happens.

Here's my fork with the patch applied: https://github.com/emi2k01/vgtk

pepijndevos commented 3 years ago

Wow weird. Uh... contributions welcome ;)

nt8r commented 3 years ago

My guess is that this is related to the // FIXME need to store and match IDs comment in patch_handlers in gtk_state.rs.