Могу я задать вопрос по коду, как мегаджун JS?

Question

[Literarum]

Даже несмотря на то, что этот ToDo лист работает так, как и хотел, я знаю, что если что-то работает, это не всегда означает, что делает оно это правильным образом.

document.addEventListener("DOMContentLoaded", () => {
        const yes = document.createElement("button");

        const showMessage = () => {
          const messageWrapper = document.createElement("div");
          const message = document.createElement("p");

          message.innerText =
            "Чтобы очистить список, нажмите правую кнопку мыши";
          messageWrapper.classList.add("messageWrapper");

          setTimeout(() => {
            document.body.appendChild(messageWrapper);
            messageWrapper.appendChild(message);
            messageWrapper.classList.add("messageWrapperUnvis");
            setTimeout(() => {
              messageWrapper.classList.remove("messageWrapperUnvis");
            }, 100);
          }, 100);

          setTimeout(() => {
            messageWrapper.classList.add("messageWrapperUnvis");
            setTimeout(() => {
              messageWrapper.remove();
            }, 1000);
          }, 2500);
        };
        
        const modalWindow = () => {
          document.addEventListener("contextmenu", (e) => {
            const wrapper = document.createElement("div");
            const buttonsWrapper = document.createElement("div");
            const question = document.createElement("p");
            const no = document.createElement("button");
            let currentDiv = null;
            e.preventDefault();

            if (e.target.matches("#input")) {
              wrapper.remove();
              return;
            }

            document.addEventListener("contextmenu", () => {
              if (currentDiv) {
                currentDiv.remove();
              }
            });
            document.addEventListener("click", (e) => {
              if (currentDiv && !currentDiv.contains(e.target)) {
                currentDiv.remove();
              }
            });

            question.innerText = "Очистить список?";
            yes.innerText = "Да";
            no.innerText = "Нет";

            wrapper.classList.add("wrapper");
            buttonsWrapper.classList.add("buttonsWrapper");
            question.classList.add("question");
            yes.classList.add("yes");
            no.classList.add("no");

            yes.addEventListener("mouseenter", () => {
              yes.classList.toggle("buttonHover");
            });
            yes.addEventListener("mouseleave", () => {
              yes.classList.remove("buttonHover");
            });
            no.addEventListener("mouseenter", () => {
              no.classList.add("buttonHover");
            });
            no.addEventListener("mouseleave", () => {
              no.classList.remove("buttonHover");
            });

            currentDiv = wrapper;
            wrapper.appendChild(question);
            wrapper.appendChild(buttonsWrapper);
            buttonsWrapper.appendChild(yes);
            buttonsWrapper.appendChild(no);
            document.body.appendChild(wrapper);
            wrapper.style.left = e.clientX + 7 + "px";
            wrapper.style.top = e.clientY + 15 + "px";

            yes.addEventListener("click", (e) => {
              if (currentDiv) {
                currentDiv.remove();
                yes.classList.remove("buttonHover");
              }
            });
            no.addEventListener("click", (e) => {
              if (currentDiv) {
                currentDiv.remove();
                no.classList.remove("buttonHover");
              }
            });
          });
        };

        const addNewItem = () => {
          const input = document.querySelector("#input");
          const list = document.querySelector("#list");

          input.addEventListener("keypress", (e) => {
            const nListItem = document.createElement("li");
            const remove = document.createElement("span");

            yes.addEventListener("click", () => {
              nListItem.classList.add("unvisible");
              setTimeout(() => {
                list.innerHTML = "";
              }, 600);
            });

            if (e.key === "Enter") {
              if (input.value !== "") {
                nListItem.innerText = input.value;
                remove.innerHTML = '<img src="trash1.png" alt="">';
                remove.classList.add("unvisible");
                nListItem.appendChild(remove);
                list.appendChild(nListItem);
                input.value = "";
              }
            }

            remove.addEventListener("click", (e) => {
              nListItem.classList.add("remove");
              e.stopImmediatePropagation();
              setTimeout(() => {
                nListItem.remove();
              }, 1000);
            });

            nListItem.addEventListener("mouseenter", () => {
              remove.classList.add("visible");
              remove.classList.remove("unvisible");
            });
            nListItem.addEventListener("mouseleave", () => {
              remove.classList.remove("visible");
              remove.classList.add("unvisible");
            });

            let isClicked = false;
            nListItem.addEventListener("click", () => {
              if (!isClicked) {
                list.insertBefore(nListItem, list.firstChild);
                nListItem.classList.add("liChecked");
                isClicked = true;
              } else {
                nListItem.classList.remove("liChecked");
                isClicked = false;
              }
            });

            list.addEventListener("click", (e) => {
              if (e.target.tagName === "LI") {
                setTimeout(() => {
                  list.appendChild(e.target);
                }, 500);
              }
            });
          });
        };

        const setup = () => {
          setTimeout(() => {
            showMessage();
          }, 1000);
          modalWindow();
          addNewItem();
        };
        setup();
      });

Answer 1

[ThunderCat]

1) Плохой код, который выполняет задачу, лучше чем отсутствие кода
2) Перед работой составьте примерный план, например: "сначала получаем данные, потом с ними делаем это или это, потом выводим..." Это помогает не лепить откровенную фигню по ходу дела и не переписывать все по 10 раз.
3) Смысл рассматривать простыню из кода новичка отсутствует, так как это похоже на оценку походки будущей фотомодели по первым шагам в ходунках. Никто не пишет хороший код сразу, учиться и писать много - лучший путь обучения.
4) Смотрите на чужие готовые решения, думайте как использовать интересные приемы, именно свои выводы и ошибки делают вас программистом+, а не гов*окодером. Задатки и желание у вас есть, значит все получится.

Answer 2

[collapsoid]

1. Лишние манипуляции над домом.
appendChild нужно делать только после того, как элемент и все его дети будут настроены. Т.е. если в функции showMessage сначала настроить messageWrapper (задать класс; добавить в него message) и только потом сделать document.body.appendChild, то можно выиграть в производительности за счет меньшего количества обращений к DOM-дереву.

Эти строки вызывают вопрос, т.к. не очень понятно, зачем нужно добавлять элементу класс и через 100 миллисекунд его убирать. Выглядит, как что-то ненужное

messageWrapper.classList.add("messageWrapperUnvis");
setTimeout(() => {
  messageWrapper.classList.remove("messageWrapperUnvis");
}, 100);

2. Ивент листенеры нужно удалять за собой.
Нужно убирать отратобавшие event listeners, т.к. их накопление приводит к утечке памяти.
Также желательно пересмотреть подход к определению слушателей вообще, т.к. в функции modalWindow на каждое событие contextmenu устанавливается другое событие contextmenu, назначение которого только лишь в том, чтобы удалить предыдущий currentDiv. Имеет смысл вынести currentDiv в функцию modalWindow, а внутри слушателя делать необходимую проверку.

3. Нерациональный ранний выход из функции.
В функции modalWindow нет смысла создавать элементы, кроме wrapper, т.к. при срабатывании условия e.target.matches("#input") произойдет return и ранее созданные элементы просто окажутся бесполезными. Т.е. if нужно разместить непосредственно после wrapper.

4. Бесполезные элементы.
Функцию addNewItem я бы вообще переделал, т.к. внутри нее на каждый keypress производится куча действий над элементами, которые в итоге так и не попадут на страницу: nListItem и remove будут добавлены только при e.key === "Enter" && input.value !== "" (кстати, нет смысла плодить вложенные if'ы, когда можно использовать булевы выражения).

В общем, уделяй внимание каждой отдельной сущности, которую ты создаешь; помни о ее назначении и о том, как она будет взаимодействовать с другими сущностями на странице. Это поможет избежать ненужных операций и в конечном итоге сделает код чище.

Когда код небольшой части приложения становится осмысленным для создающего, появляется мотивация упорядочивать его в глобальных масштабах, т.е. решать архитектурные вопросы, что в совокупности с шишками, которые принесет этот процесс, в конечном итоге, сделает тебя более скилованным разработчиком.