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

todomvc example not working correctly #76

Open
emi2k01 opened this issue Feb 10, 2021 · 2 comments
Open

todomvc example not working correctly #76

emi2k01 opened this issue Feb 10, 2021 · 2 comments

Comments

@emi2k01
Copy link

emi2k01 commented Feb 10, 2021

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
Copy link
Collaborator

Wow weird. Uh... contributions welcome ;)

@nt8r
Copy link
Contributor

nt8r commented Mar 6, 2021

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

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

No branches or pull requests

3 participants