Search code examples
javascriptvuejs2vue-cli-3

Removing an item from array: Wrong item being removed


I have a page of posts with a list of media attachments. I want the user to be able to manually delete an attachment(s) when he clicks the "delete" link. This event is working, however, the problem is that the actual item being deleted is not the one that is being clicked! What am I doing wrong?

Here's my code and a live demo here: [codesandbox] (click POSTS-->choose a post ID to view post details --> click EDIT post)

EditPost.vue <template> section:

...snip..
    <ul>
      <li>Media Attachments
        <template>
          <ul v-if="attachmentsFileNames && attachmentsFileNames.length">
            <li v-for="(mediaAttachment, index) in attachmentsFileNames" :key="index">
              <a href="#">{{ mediaAttachment }}</a>&nbsp;
              <button @click.prevent="deleteMediaAttachment(mediaAttachment, index)">Delete me!</button>
            </li>
          </ul>
          <p v-else>No Media Attachments</p>
        </template>
      </li>
    </ul>

EditPost.vue <script>:

...snip..
data() {
    return {
      post: {},
      editPostFormIsVis: false,
      attachmentsArray: attachments
    };
  },
  created() {
    this.getPost();
  },
  methods: {
    getPost() {
      axios
        .get(
          "https://jsonplaceholder.typicode.com/posts/" + this.$route.params.id
        )
        .then(resp => {
          this.post = resp.data;
        })
        .catch(err => {
          console.log(err);
        });
    },
    editPost() {
      this.editPostFormIsVis = true;
    },
deleteMediaAttachment: function(item, index) {
      if (this.attachmentsArray.attachments[index] === item) {
        // The template passes index as the second parameter to avoid indexOf,
        // it will be better for the performance especially for one large array
        // (because indexOf actually loop the array to do the match)
        this.attachmentsArray.attachments.splice(index, 1);
      } else {
        let found = this.attachmentsArray.attachments.indexOf(item);
        this.attachmentsArray.attachments.splice(found, 1);
      }
    }
  },
  computed: {
    emailAttachmentsFileNames() {
      if (this.attachmentsArray.emailAttachments) {
        const emailAttachmentsFileNameArray = this.attachmentsArray.emailAttachments.map(
          item => {
            const tokens = item.split("/");
            return tokens[tokens.length - 1];
          }
        );
        return emailAttachmentsFileNameArray;
      } else {
        return null;
      }
    },
    attachmentsFileNames() {
      if (this.attachmentsArray.attachments) {
        const attachmentsFileNameArray = this.attachmentsArray.attachments.map(
          item => {
            const tokens = item.split("/");
            return tokens[tokens.length - 1];
          }
        );
        return attachmentsFileNameArray;
      } else {
        return null;
      }
    }
  }
...snip...

Solution

  • Your problem is that you send in the item from the attachmentsFileNames and try to match it to an item in the original attachments array. The first contains the filename while the second contains the full path so they won't match. For example:

    console.log(item);                                     // star_wars_luke.jpeg
    console.log(this.attachmentsArray.attachments[index]); // attachments/2019/201900002020/star_wars_luke.jpeg
    

    That's why this.attachmentsArray.attachments[index] === item is false, then this.attachmentsArray.attachments.indexOf(item) returns -1 and consequently, this.attachmentsArray.attachments.splice(-1, 1) always removes the last item in the array.

    Since you parsed the attachmentsFileNames array yourself from this.attachmentsArray.attachments, you can rely on the index corresponding correctly so those checks aren't even needed. Just remove them and run this.attachmentsArray.attachments.splice(index, 1) and it should work.