2

With Vue.js, I'm showing the list of items with a checkbox. Clicking on Checkbox will move the item down with strike-through. The issue is, when I click on the checkbox, the wrong checkbox is checked.

For eg, when I click on Apple checkbox, orange checkbox is checked.

Fiddle: https://jsfiddle.net/d6encxe1/

Here's my code,

var myApp = new Vue({
  el: '#myApp',
  data: {
    lists: [
      {title: 'Apple', isChecked: false},
      {title: 'Orange', isChecked: false},
      {title: 'Grapes', isChecked: false}
    ]
  },
  computed: {
    filterLists: function(){
      return _.orderBy(this.lists, ['isChecked', false]);
    }
  },
  methods: {
    completeTask: function(e, i){
      e.preventDefault();
      this.lists[i].isChecked = !this.lists[i].isChecked;
    }
  }
})
.completed{
  text-decoration: line-through;
  color: red;
}
<script src="https://unpkg.com/vue"></script>
<script src="https://cdn.jsdelivr.net/lodash/4.17.4/lodash.min.js"></script>

<div id="myApp">
  <ul>
      <li v-for="(list, index) in filterLists">
        <input type="checkbox" v-bind:id="'todo-' + index" v-on:change="completeTask($event, index)" />
        <span class="title" v-bind:class="{completed: list.isChecked}">{{list.title}}</span> 
        </li>
      
    </ul>
 </div>

1
  • try moving v-for to li and put a key attribute on li that is unique (can be title in your case) Commented Jun 16, 2017 at 9:11

3 Answers 3

4

Use a key. A key uniquely identifies an element in Vue. If you do not use a key, Vue will try to re-use existing elements for performance.

To give Vue a hint so that it can track each node’s identity, and thus reuse and reorder existing elements, you need to provide a unique key attribute for each item. An ideal value for key would be the unique id of each item.

You should always use a key when rendering list. Here I'm using the title of your list items, but ideally you should generate a unique key.

<li v-for="(list, index) in filterLists" :key="list.title">

Also you do not need to pass indexes around. Just pass the item itself.

v-on:change="completeTask(list)"

And in completeTask, check it off.

completeTask: function(task){
  task.isChecked = !task.isChecked
}

Finally, iterate over your li element and not your ul element.

Updated fiddle.

Sign up to request clarification or add additional context in comments.

1 Comment

I completely forget about the key. That fixed the issue.Thanks for pointing out index issue.. It's working now. Thanks!
4

The index screws things up, you can fix it by binding the checked state

<input type="checkbox" v-bind:checked="list.isChecked" v-bind:id="'todo-' + index" v-on:change="completeTask($event, index)" />

And changing the complete task to just pass the item in:

<input type="checkbox" v-bind:id="'todo-' + index" v-on:change="completeTask(list)" v-bind:checked="list.isChecked" />

See fiddle:

https://jsfiddle.net/d6encxe1/3/

You should remove the 'id' or use something other than the index of the loop because when you re-order the index doesn't change.

1 Comment

Another great solution without using :key. Thanks
1

You probably want to put v-for in <li> instead of <ul>, or your will get several <ul> elements.

And you didn't provide a key. You should provide a unique key for the items. For example, if the title is unique, you can use it as the key, or you may need to add another attribute like id.

Besides, you can pass the entire list item to the method instead of just the index, because the indexes are changeable in your case:

v-on:change="completeTask($event, list)"

Working example here: https://jsfiddle.net/0r2yb0z6/1/

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.