Todo list作成アプリですが、削除ボタンがうまく効かないので修正方法を教えてください。

todo APPですが、登録した各todoに対して、削除ボタンが付いてます。
一番上のtodo_削除ボタンを押して対応todoを削除するといったん削除されているように見えますが、ページを更新するとなぜか一番下のtodoが削除されます。2番目、3番目のtodo_削除ボタンも同じです。
理由を教えてください。

html
1<!DOCTYPE html>
2<html lang="en">
3<head>
4    <meta charset="UTF-8">
5    <meta name="viewport" content="width=device-width, initial-scale=1.0">
6    <title>To Do List</title>
7    <!-- CSS only -->
8<link href="https://cdn.jsdelivr.net/npm/bootstrap@5.0.0-beta1/dist/css/bootstrap.min.css" rel="stylesheet" integrity="sha384-giJF6kkoqNQ00vy+HMDP7azOuL0xtbfIcaT9wjKHr8RbDVddVHyTfAAsrekwKmP1" crossorigin="anonymous">
9<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/bootstrap-icons@1.3.0/font/bootstrap-icons.css">
10<style>
11    .green {
12        color:lightgreen;
13    }
14    .red {
15        color:lightcoral;
16    }
17    .blue {
18        color:skyblue
19    }
20    #itemList { font-size: 1.5rem;}
21    .show { display: block;}
22    .hide { display: none;}
23</style>
24</head>
25<body>
26<div class="container">
27    <div class="row">
28        <div class="col mx-auto col-md-8 mt-3 text-center">
29        <div class="alert align-items-center" id="message"></div>
30        <form id="itemForm" >
31            <h3 class=""> TO DO LIST</h3>
32            <div class="input-group mb-3">
33                <input type="text" class="form-control"  id="itemInput" value="" placeholder="Please enter name...">
34                <button class="btn btn-outline-primary">ADD ITEM</button>
35            </div>
36        </form>
37        <ul class="nav nav-tabs">
38            <li class="nav-item" data-type="all">
39              <a class="nav-link active" href="#">All</a>
40            </li>
41            <li class="nav-item" data-type="todo">
42              <a class="nav-link" href="#">To Do</a>
43            </li>
44            <li class="nav-item" data-type="done">
45              <a class="nav-link" href="#">Completed</a>
46            </li>
47          </ul>
48        <!-- list items -->
49        <ul class="list-group list-group-flush" id="itemList"></ul>
50        <input type="hidden" id="filterType" value="all">
51        <input type="hidden" id="citem" value="">
52        </div>
53    </div>
54   
55</div>
56<script src="main.js"></script>
57</body>
58</html>

JavaScript
1// selet elements in DOM
2
3const form = document.querySelector("#itemForm");
4const itemInput = document.querySelector("#itemInput");
5const itemList = document.querySelector("#itemList");
6const messageDiv = document.querySelector("#message");
7const clearButton = document.querySelector("#clearBtn");
8const filters = document.querySelectorAll(".nav-item");
9
10// create empty item list
11let todoItems = [];
12
13const showAlert = function (message, msgClass) {
14  console.log("msg");
15  messageDiv.innerHTML = message;
16  messageDiv.classList.add(msgClass, "show");
17  messageDiv.classList.remove("hide");
18  setTimeout(() => {
19    messageDiv.classList.remove("show",msgClass);
20    messageDiv.classList.add("hide");
21  }, 3000);
22  return;
23};
24// filter tab items
25const getItemsFilter = function (type) {
26  let filterItems = [];
27  console.log(type);
28  switch (type) {
29    case "todo":
30      filterItems = todoItems.filter((item) => !item.isDone);
31      break;
32    case "done":
33      filterItems = todoItems.filter((item) => item.isDone);
34      break;
35    default:
36      filterItems = todoItems;
37  }
38  getList(filterItems);
39};
40
41// update item
42const updateItem = function (itemIndex, newValue) {
43  console.log(itemIndex);
44  const newItem = todoItems[itemIndex];
45  newItem.name = newValue;
46  todoItems.splice(itemIndex, 1, newItem);
47  setLocalStorage(todoItems);
48};
49
50// remove/delete item
51const removeItem = function (item) {
52  const removeIndex = todoItems.indexOf(item);
53  todoItems.splice(removeIndex, 1);
54};
55
56//bi-check-circle-fill  // bi-check-circle
57// handle item
58const handleItem = function (itemData) {
59  const items = document.querySelectorAll(".list-group-item");
60  items.forEach((item) => {
61    if (
62      item.querySelector(".title").getAttribute("data-time") == itemData.addedAt
63    ) {
64      // done
65      item.querySelector("[data-done]").addEventListener("click", function (e) {
66        e.preventDefault();
67        const itemIndex = todoItems.indexOf(itemData);
68        const currentItem = todoItems[itemIndex];
69        const currentClass = currentItem.isDone
70          ? "bi-check-circle-fill"
71          : "bi-check-circle";
72        currentItem.isDone = currentItem.isDone ? false : true;
73        todoItems.splice(itemIndex, 1, currentItem);
74        // todoItems.splice(itemIndex, noofelem, element);
75        setLocalStorage(todoItems);
76        //console.log(todoItems[itemIndex]);
77        const iconClass = currentItem.isDone
78          ? "bi-check-circle-fill"
79          : "bi-check-circle";
80
81        this.firstElementChild.classList.replace(currentClass, iconClass);
82        const filterType = document.querySelector("#filterType").value;
83        getItemsFilter(filterType);
84      });
85      // edit
86      item.querySelector("[data-edit]").addEventListener("click", function (e) {
87        e.preventDefault();
88        itemInput.value = itemData.name;
89        document.querySelector("#citem").value = todoItems.indexOf(itemData);
90        return todoItems;
91      });
92
93      //delete
94      item
95        .querySelector("[data-delete]")
96        .addEventListener("click", function (e) {
97          e.preventDefault();
98          if (confirm("Are you sure want to delete?")) {
99            itemList.removeChild(item);
100            removeItem(item);
101            setLocalStorage(todoItems);
102            showAlert("Item has been deleted.", "alert-success");
103            return todoItems.filter((item) => item != itemData);
104          }
105        });
106    }
107  });
108};
109// get list items
110const getList = function (todoItems) {
111  itemList.innerHTML = "";
112  if (todoItems.length > 0) {
113    todoItems.forEach((item) => {
114      const iconClass = item.isDone
115        ? "bi-check-circle-fill"
116        : "bi-check-circle";
117      itemList.insertAdjacentHTML(
118        "beforeend",
119        `<li class="list-group-item d-flex justify-content-between align-items-center">
120          <span class="title" data-time="${item.addedAt}">${item.name}</span> 
121          <span>
122              <a href="#" data-done><i class="bi ${iconClass} green"></i></a>
123              <a href="#" data-edit><i class="bi bi-pencil-square blue"></i></a>
124              <a href="#" data-delete><i class="bi bi-x-circle red"></i></a>
125          </span>
126        </li>`
127      );
128      handleItem(item);
129    });
130  } else {
131    itemList.insertAdjacentHTML(
132      "beforeend",
133      `<li class="list-group-item d-flex justify-content-between align-items-center">
134        No record found.
135      </li>`
136    );
137  }
138};
139
140// get localstorage from the page
141const getLocalStorage = function () {
142  const todoStorage = localStorage.getItem("todoItems");
143  if (todoStorage === "undefined" || todoStorage === null) {
144    todoItems = [];
145  } else {
146    todoItems = JSON.parse(todoStorage);
147    //console.log("items", todoItems);
148  }
149  getList(todoItems);
150};
151// set list in local storage
152const setLocalStorage = function (todoItems) {
153  localStorage.setItem("todoItems", JSON.stringify(todoItems));
154};
155
156document.addEventListener("DOMContentLoaded", () => {
157  form.addEventListener("submit", (e) => {
158    e.preventDefault();
159    const itemName = itemInput.value.trim();
160    if (itemName.length === 0) {
161      showAlert("Please enter name.", "alert-danger");
162      return;
163    } else {
164      // update existing Item
165      const currenItemIndex = document.querySelector("#citem").value;
166      if (currenItemIndex) {
167        updateItem(currenItemIndex, itemName);
168        document.querySelector("#citem").value = "";
169        showAlert("Item has been updated.", "alert-success");
170      } else {
171        // Add new Item
172        const itemObj = {
173          name: itemName,
174          isDone: false,
175          addedAt: new Date().getTime(),
176        };
177        todoItems.push(itemObj);
178        // set local storage
179        setLocalStorage(todoItems);
180        showAlert("New item has been added.", "alert-success");
181      }
182
183      getList(todoItems);
184      // get list of all items
185    }
186    console.log(todoItems);
187    itemInput.value = "";
188  });
189
190  // filters
191  filters.forEach((tab) => {
192    tab.addEventListener("click", function (e) {
193      e.preventDefault();
194      const tabType = this.getAttribute("data-type");
195      document.querySelectorAll(".nav-link").forEach((nav) => {
196        nav.classList.remove("active");
197      });
198      this.firstElementChild.classList.add("active");
199      document.querySelector("#filterType").value = tabType;
200      getItemsFilter(tabType);
201    });
202  });
203
204  // load items
205  getLocalStorage();
206});
207

m.ts10806

2021/08/22 09:56 編集

＞削除ボタンが付いてますが、一番上todo_削除ボタンを押すとなぜか一番下のtodoが削除されます。コードコピペで確認しましたが 1 2 3 と作って１を削除するときちんと１が削除されました。 2 3 と残りました。 >一番下のtodo_削除ボタン以外で削除するとこれはどういう方法ですか？また、確認しているブラウザとバージョンも提示してください。こちらはWindows10 , FireFox91.0.1（64bit）です。

m.ts10806

2021/08/22 09:58

あぁ理解しました一度画面更新が必要と言うことですね。

m.ts10806

2021/08/22 10:01

正確には →削除ボタンを押すと見た目上は押したレコードが削除されるが画面更新を押すと一番最後が削除された状態になるおそらく、ローカルストレージ上のデータの扱いが正しくないのでしょうね。

kinsncn

2021/08/22 11:00

ご理解の通りです。ご丁寧にありがとうございました。

m.ts10806

2021/08/22 11:11

文章調整しておいてください。起きている現象（操作手順）は、なるべく正しく

kinsncn

2021/08/24 13:27

文書を分かりやすく修正しました。

行動規範の内容に同意します

回答1件

js
1const removeItem = function (item) {
2  const removeIndex = todoItems.indexOf(item);
3  console.log(removeIndex)

↑ここで-1(要素が見つかっていない)となってるのが原因と思います。

itemが持ってるのがDOMで、todoItemsが持ってるのがJSONオブジェクトなので当然かもしれません。
data属性にユニークな情報（indexとかIDとなるもの）を持たせて照合させたほうが良いように思います。
場合により、LocalStorageに持たせるJSON文字列も実装上で扱いやすいように変更したほうがいいかもしれません。

別案：
今の実装だと
1.DOMに反映
2.ローカルストレージに反映

と二重管理になっていて、バグが起きやすい状況になってます。
それなら
1.ローカルストレージに反映
2.DOM全削除して反映したローカルストレージからDOMを構築

としたほうが実装もシンプルで短くなるんじゃないでしょうか。

投稿2021/08/22 11:24

編集2021/08/22 11:33

m.ts10806

総合スコア80850