Application Javascript to-do pour Javascript natif

Sep 02 2020

J'ai fait une application simple pour le Javascript natif. C'est ma première fois, auparavant je n'utilisais que jQuery. Quelles recommandations seraient utiles?

https://codepen.io/denibudeyko/pen/MWyEmox

(function(window) {
  'use strict';

  // Initial Const 
  const input = document.getElementsByClassName('new-todo')[0]
  const todoList = document.getElementsByClassName('todo-list')[0]
  const items = document.querySelectorAll('.todo-list li')
  const clear = document.querySelector('.clear-completed')
  const todoCount = document.querySelector('.todo-count strong')

  // Add new task
  input.addEventListener('keyup', function(e) {

    var string = input.value;

    if (e.key == 'Enter') {
      const li = document.createElement('li')
      li.innerHTML = `
                    <div class="view">
                        <input class="toggle" type="checkbox" />
                        <label>${string}</label> <button class="destroy"></button> </div> <input class="edit" value="${string}">
                `;

      todoList.appendChild(li);
      const childrenItem = li.querySelector('input.toggle')

      childrenItem.addEventListener('click', event => {
        toggleTaskStatusEvent(event.target)
      })

      input.value = '';
      updateCount();
    }
  })

  function updateCount() {
    todoCount.textContent = document.querySelectorAll('.todo-list li:not(.completed)').length;
  }

  updateCount()


  // Toggle Status
  document.querySelectorAll('.todo-list li').forEach(item => {

    const toggleCheckbox = item.querySelector('input.toggle')
    toggleCheckbox.addEventListener('click', event => {
      toggleTaskStatusEvent(event.target)
    })
  })

  // ToggleTaskStatus
  function toggleTaskStatusEvent(e) {
    const liParent = e.closest('li')
    liParent.classList.toggle('completed')
    updateCount()
  }

  clear.addEventListener('click', function() {
    document.querySelectorAll('.todo-list li.completed').forEach(item => {
      item.remove()
    })
  })

  // FiltersButtons
  document.querySelectorAll('ul.filters li').forEach(item => {
    const filterButton = item.children[0];
    filterButton.addEventListener('click', event => {
      runFilter(event.target)
    })
  })

  // Function Filter
  function runFilter(item) {
    const notCompletedItemsFilter = document.querySelectorAll('.todo-list li:not(.completed)')
    const completedItemsFilter = document.querySelectorAll('.todo-list li.completed')
    const allItemsFilter = document.querySelectorAll('.todo-list li');

    var href = item.getAttribute('href')
    href = href.split('#/')[1]

    const activeButton = document.querySelector('ul.filters a.selected')
    activeButton.classList.remove('selected')
    item.classList.add("selected")

    if (href == 'active') {

      notCompletedItemsFilter.forEach(item => {
        item.style.display = 'block';
      })
      completedItemsFilter.forEach(item => {
        item.style.display = 'none';
      })

    } else if (href == 'completed') {
      notCompletedItemsFilter.forEach(item => {
        item.style.display = 'none';
      })
      completedItemsFilter.forEach(item => {
        item.style.display = 'block';
      })
    } else if (href == 'all') {
      allItemsFilter.forEach(item => {
        item.style.display = 'block'
      })
    }
  }

  // Edit Task Double click
  items.forEach(item => {
    item.addEventListener('dblclick', event => {
      editTask(item)
    })
  })

  // Edit Task
  function editTask(item) {
    var label = item.querySelector('label');
    var editButton = item.querySelector('.edit')
    editButton.style.display = 'block'

    editButton.addEventListener('keyup', function(event) {
      label.textContent = this.value
      if (event.key == 'Enter') {
        this.style.display = 'none';
      }
    })
  }

  // Function Remove Task
  items.forEach(item => {
    item.querySelector('.destroy').addEventListener('click', function(event) {
      item.remove()
      updateCount();
    })
  })
})(window);

.todo-list li .toggle {
  cursor: pointer;
}

html,
body {
  margin: 0;
  padding: 0;
}

button {
  margin: 0;
  padding: 0;
  border: 0;
  background: none;
  font-size: 100%;
  vertical-align: baseline;
  font-family: inherit;
  font-weight: inherit;
  color: inherit;
  -webkit-appearance: none;
  appearance: none;
  -webkit-font-smoothing: antialiased;
  -moz-osx-font-smoothing: grayscale;
}

body {
  font: 14px 'Helvetica Neue', Helvetica, Arial, sans-serif;
  line-height: 1.4em;
  background: #f5f5f5;
  color: #4d4d4d;
  min-width: 230px;
  max-width: 550px;
  margin: 0 auto;
  -webkit-font-smoothing: antialiased;
  -moz-osx-font-smoothing: grayscale;
  font-weight: 300;
}

 :focus {
  outline: 0;
}

.hidden {
  display: none;
}

.todoapp {
  background: #fff;
  margin: 130px 0 40px 0;
  position: relative;
  box-shadow: 0 2px 4px 0 rgba(0, 0, 0, 0.2), 0 25px 50px 0 rgba(0, 0, 0, 0.1);
}

.todoapp input::-webkit-input-placeholder {
  font-style: italic;
  font-weight: 300;
  color: #e6e6e6;
}

.todoapp input::-moz-placeholder {
  font-style: italic;
  font-weight: 300;
  color: #e6e6e6;
}

.todoapp input::input-placeholder {
  font-style: italic;
  font-weight: 300;
  color: #e6e6e6;
}

.todoapp h1 {
  position: absolute;
  top: -155px;
  width: 100%;
  font-size: 100px;
  font-weight: 100;
  text-align: center;
  color: rgba(175, 47, 47, 0.15);
  -webkit-text-rendering: optimizeLegibility;
  -moz-text-rendering: optimizeLegibility;
  text-rendering: optimizeLegibility;
}

.new-todo,
.edit {
  position: relative;
  margin: 0;
  width: 100%;
  font-size: 24px;
  font-family: inherit;
  font-weight: inherit;
  line-height: 1.4em;
  border: 0;
  color: inherit;
  padding: 6px;
  border: 1px solid #999;
  box-shadow: inset 0 -1px 5px 0 rgba(0, 0, 0, 0.2);
  box-sizing: border-box;
  -webkit-font-smoothing: antialiased;
  -moz-osx-font-smoothing: grayscale;
}

.new-todo {
  padding: 16px 16px 16px 60px;
  border: none;
  background: rgba(0, 0, 0, 0.003);
  box-shadow: inset 0 -2px 1px rgba(0, 0, 0, 0.03);
}

.main {
  position: relative;
  z-index: 2;
  border-top: 1px solid #e6e6e6;
}

.toggle-all {
  width: 1px;
  height: 1px;
  border: none;
  /* Mobile Safari */
  opacity: 0;
  position: absolute;
  right: 100%;
  bottom: 100%;
}

.toggle-all+label {
  width: 60px;
  height: 34px;
  font-size: 0;
  position: absolute;
  top: -52px;
  left: -13px;
  -webkit-transform: rotate(90deg);
  transform: rotate(90deg);
}

.toggle-all+label:before {
  content: '❯';
  font-size: 22px;
  color: #e6e6e6;
  padding: 10px 27px 10px 27px;
}

.toggle-all:checked+label:before {
  color: #737373;
}

.todo-list {
  margin: 0;
  padding: 0;
  list-style: none;
}

.todo-list li {
  position: relative;
  font-size: 24px;
  border-bottom: 1px solid #ededed;
}

.todo-list li:last-child {
  border-bottom: none;
}

.todo-list li.editing {
  border-bottom: none;
  padding: 0;
}

.todo-list li.editing .edit {
  display: block;
  width: 506px;
  padding: 12px 16px;
  margin: 0 0 0 43px;
}

.todo-list li.editing .view {
  display: none;
}

.todo-list li .toggle {
  text-align: center;
  width: 40px;
  /* auto, since non-WebKit browsers doesn't support input styling */
  height: auto;
  position: absolute;
  top: 0;
  bottom: 0;
  margin: auto 0;
  border: none;
  /* Mobile Safari */
  -webkit-appearance: none;
  appearance: none;
}

.todo-list li .toggle {
  opacity: 0;
}

.todo-list li .toggle+label {
  /*
            Firefox requires `#` to be escaped - https://bugzilla.mozilla.org/show_bug.cgi?id=922433
            IE and Edge requires *everything* to be escaped to render, so we do that instead of just the `#` - https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/7157459/
        */
  background-image: url('data:image/svg+xml;utf8,%3Csvg%20xmlns%3D%22http%3A//www.w3.org/2000/svg%22%20width%3D%2240%22%20height%3D%2240%22%20viewBox%3D%22-10%20-18%20100%20135%22%3E%3Ccircle%20cx%3D%2250%22%20cy%3D%2250%22%20r%3D%2250%22%20fill%3D%22none%22%20stroke%3D%22%23ededed%22%20stroke-width%3D%223%22/%3E%3C/svg%3E');
  background-repeat: no-repeat;
  background-position: center left;
}

.todo-list li .toggle:checked+label {
  background-image: url('data:image/svg+xml;utf8,%3Csvg%20xmlns%3D%22http%3A//www.w3.org/2000/svg%22%20width%3D%2240%22%20height%3D%2240%22%20viewBox%3D%22-10%20-18%20100%20135%22%3E%3Ccircle%20cx%3D%2250%22%20cy%3D%2250%22%20r%3D%2250%22%20fill%3D%22none%22%20stroke%3D%22%23bddad5%22%20stroke-width%3D%223%22/%3E%3Cpath%20fill%3D%22%235dc2af%22%20d%3D%22M72%2025L42%2071%2027%2056l-4%204%2020%2020%2034-52z%22/%3E%3C/svg%3E');
}

.todo-list li label {
  word-break: break-all;
  padding: 15px 15px 15px 60px;
  display: block;
  line-height: 1.2;
  transition: color 0.4s;
}

.todo-list li.completed label {
  color: #d9d9d9;
  text-decoration: line-through;
}

.todo-list li .destroy {
  display: none;
  position: absolute;
  top: 0;
  right: 10px;
  bottom: 0;
  width: 40px;
  height: 40px;
  margin: auto 0;
  font-size: 30px;
  color: #cc9a9a;
  margin-bottom: 11px;
  transition: color 0.2s ease-out;
}

.todo-list li .destroy:hover {
  color: #af5b5e;
}

.todo-list li .destroy:after {
  content: '×';
}

.todo-list li:hover .destroy {
  display: block;
}

.todo-list li .edit {
  display: none;
}

.todo-list li.editing:last-child {
  margin-bottom: -1px;
}

.footer {
  color: #777;
  padding: 10px 15px;
  height: 20px;
  text-align: center;
  border-top: 1px solid #e6e6e6;
}

.footer:before {
  content: '';
  position: absolute;
  right: 0;
  bottom: 0;
  left: 0;
  height: 50px;
  overflow: hidden;
  box-shadow: 0 1px 1px rgba(0, 0, 0, 0.2), 0 8px 0 -3px #f6f6f6, 0 9px 1px -3px rgba(0, 0, 0, 0.2), 0 16px 0 -6px #f6f6f6, 0 17px 2px -6px rgba(0, 0, 0, 0.2);
}

.todo-count {
  float: left;
  text-align: left;
}

.todo-count strong {
  font-weight: 300;
}

.filters {
  margin: 0;
  padding: 0;
  list-style: none;
  position: absolute;
  right: 0;
  left: 0;
}

.filters li {
  display: inline;
}

.filters li a {
  color: inherit;
  margin: 3px;
  padding: 3px 7px;
  text-decoration: none;
  border: 1px solid transparent;
  border-radius: 3px;
}

.filters li a:hover {
  border-color: rgba(175, 47, 47, 0.1);
}

.filters li a.selected {
  border-color: rgba(175, 47, 47, 0.2);
}

.clear-completed,
html .clear-completed:active {
  float: right;
  position: relative;
  line-height: 20px;
  text-decoration: none;
  cursor: pointer;
}

.clear-completed:hover {
  text-decoration: underline;
}

.info {
  margin: 65px auto 0;
  color: #bfbfbf;
  font-size: 10px;
  text-shadow: 0 1px 0 rgba(255, 255, 255, 0.5);
  text-align: center;
}

.info p {
  line-height: 1;
}

.info a {
  color: inherit;
  text-decoration: none;
  font-weight: 400;
}

.info a:hover {
  text-decoration: underline;
}


/*
        Hack to remove background from Mobile Safari.
        Can't use it globally since it destroys checkboxes in Firefox
    */

@media screen and (-webkit-min-device-pixel-ratio:0) {
  .toggle-all,
  .todo-list li .toggle {
    background: none;
  }
  .todo-list li .toggle {
    height: 40px;
  }
}

@media (max-width: 430px) {
  .footer {
    height: 50px;
  }
  .filters {
    bottom: 10px;
  }
}

hr {
  margin: 20px 0;
  border: 0;
  border-top: 1px dashed #c5c5c5;
  border-bottom: 1px dashed #f7f7f7;
}

.learn a {
  font-weight: normal;
  text-decoration: none;
  color: #b83f45;
}

.learn a:hover {
  text-decoration: underline;
  color: #787e7e;
}

.learn h3,
.learn h4,
.learn h5 {
  margin: 10px 0;
  font-weight: 500;
  line-height: 1.2;
  color: #000;
}

.learn h3 {
  font-size: 24px;
}

.learn h4 {
  font-size: 18px;
}

.learn h5 {
  margin-bottom: 0;
  font-size: 14px;
}

.learn ul {
  padding: 0;
  margin: 0 0 30px 25px;
}

.learn li {
  line-height: 20px;
}

.learn p {
  font-size: 15px;
  font-weight: 300;
  line-height: 1.3;
  margin-top: 0;
  margin-bottom: 0;
}

#issue-count {
  display: none;
}

.quote {
  border: none;
  margin: 20px 0 60px 0;
}

.quote p {
  font-style: italic;
}

.quote p:before {
  content: '“';
  font-size: 50px;
  opacity: .15;
  position: absolute;
  top: -20px;
  left: 3px;
}

.quote p:after {
  content: '”';
  font-size: 50px;
  opacity: .15;
  position: absolute;
  bottom: -42px;
  right: 3px;
}

.quote footer {
  position: absolute;
  bottom: -40px;
  right: 0;
}

.quote footer img {
  border-radius: 3px;
}

.quote footer a {
  margin-left: 5px;
  vertical-align: middle;
}

.speech-bubble {
  position: relative;
  padding: 10px;
  background: rgba(0, 0, 0, .04);
  border-radius: 5px;
}

.speech-bubble:after {
  content: '';
  position: absolute;
  top: 100%;
  right: 30px;
  border: 13px solid transparent;
  border-top-color: rgba(0, 0, 0, .04);
}

.learn-bar>.learn {
  position: absolute;
  width: 272px;
  top: 8px;
  left: -300px;
  padding: 10px;
  border-radius: 5px;
  background-color: rgba(255, 255, 255, .6);
  transition-property: left;
  transition-duration: 500ms;
}

@media (min-width: 899px) {
  .learn-bar {
    width: auto;
    padding-left: 300px;
  }
  .learn-bar>.learn {
    left: 8px;
  }
}

.destroy {
  cursor: pointer;
}

<!-- <link rel="stylesheet" href="node_modules/todomvc-common/base.css"> -->
<!-- <link rel="stylesheet" href="node_modules/todomvc-app-css/index.css"> -->

<section class="todoapp">
  <header class="header">
    <h1>todos</h1>
    <input class="new-todo" placeholder="What needs to be done?" autofocus>
  </header>
  <!-- This section should be hidden by default and shown when there are todos -->
  <section class="main">
    <input id="toggle-all" class="toggle-all" type="checkbox">
    <label for="toggle-all">Mark all as complete</label>
    <ul class="todo-list">
      <!-- These are here just to show the structure of the list items -->
      <!-- List items should get the class `editing` when editing and `completed` when marked as completed -->
      <li class='completed'>
        <div class="view">
          <input class="toggle" checked type="checkbox">
          <label>Taste JavaScript</label>
          <button class="destroy"></button>
        </div>
        <input class="edit" value="Taste JavaScript">
      </li>
      <li>
        <div class="view">
          <input class="toggle" type="checkbox">
          <label>Refactoring Code</label>
          <button class="destroy"></button>
        </div>
        <input class="edit" value="Refactoring Code">
      </li>
      <li>
        <div class="view">
          <input class="toggle" type="checkbox">
          <label>Rewrite app to react</label>
          <button class="destroy"></button>
        </div>
        <input class="edit" value="Rewrite app to react">
      </li>
    </ul>
  </section>
  <!-- This footer should hidden by default and shown when there are todos -->
  <footer class="footer">
    <!-- This should be `0 items left` by default -->
    <span class="todo-count"><strong>0</strong> item left</span>
    <!-- Remove this if you don't implement routing -->
    <ul class="filters">
      <li>
        <a class="selected" href="#/all">All</a>
      </li>
      <li>
        <a href="#/active">Active</a>
      </li>
      <li>
        <a href="#/completed">Completed</a>
      </li>
    </ul>
    <!-- Hidden if no completed items are left ↓ -->
    <button class="clear-completed">Clear completed</button>
  </footer>
</section>
<footer class="info">
  <p>Double-click to edit a todo</p>
  <p>Enter to add/edit task</p>

</footer>
<!-- Scripts here. Don't remove ↓ -->
<!-- <script src="node_modules/todomvc-common/base.js"></script> -->

Réponses

2 SᴀᴍOnᴇᴌᴀ Sep 02 2020 at 22:54

Problème d'interaction avec l'utilisateur

Il y a une faille avec le mécanisme de double-clic pour modifier - il ne fonctionne que pour les éléments existants, mais pour les éléments nouvellement ajoutés, le gestionnaire de double-clic n'est pas enregistré. La même chose est vraie pour les liens de destruction.

Au lieu d'ajouter un gestionnaire de clics à chaque élément de liste, la délégation d'événement pourrait être utilisée pour ajouter un gestionnaire de clics à la liste et déléguer l'événement d'édition à l'élément de liste.

JS

sélection d'éléments

Je sais que CertainPerformance a déjà abordé le code pour sélectionner des éléments. Pour la sélection des éléments de liste, la délégation d'événement peut être obtenue en convertissant ces lignes:

const todoList = document.getElementsByClassName('todo-list')[0]
const items = document.querySelectorAll('.todo-list li')

pour ça:

const todoList = document.getElementsByClassName('todo-list')[0];
const items = todoList.getElementsByTagName('li'); // <- active collection

Notez que getElementsByTagName()renvoie un ^{1 en} direct , il n'est donc pas nécessaire de réinterroger la liste une fois les éléments ajoutés ou supprimés ... et pour parcourir les éléments, ils peuvent être placés dans un tableau à l'aide de l' opérateur de propagation :HTMLCollection

[...items].forEach(...)

opérateurs de comparaison

Il est recommandé par beaucoup d'utiliser une égalité stricte lors de la comparaison des valeurs - de cette façon, il n'est pas nécessaire de convertir les types. Par exemple, dans cette ligne dans runFilter:

if (href == 'active') {

hrefprovient de var href = item.getAttribute('href')et cette méthode renvoie une chaîne ou null. Il n'y a donc pas besoin de coercition de type.

Méthodes répétées pour mettre à jour les propriétés d'affichage des éléments

Cette runFilterfonction a plusieurs forEach()boucles avec des fonctions fléchées comme celle-ci:

item => {
  item.style.display = 'block';
}

Les fonctions répétées pourraient être abstraites en fonctions nommées pour éviter les redondances.

De plus, au lieu de définir le style, le classList méthodes aiment add()et remove()pourraient être utilisées à la place - avec la classe CSS hidden.

CSS

Il existe un ensemble de règles pour htmlet body:

html,
body {
  margin: 0;
  padding: 0;
}

puis plus tard, il y en a un pour body:

body {

et il a ceci:

  margin: 0 auto;

Cela semble excessif ... l'un ou les deux pourraient être supprimés.

syntaxe de marge

Le CSS actuel contient ceci:

.todoapp {
  background: #fff;
  margin: 130px 0 40px 0;

La syntaxe de la marge peut être convertie de /* top | right | bottom | left */en /* top | horizontal | bottom */.

  margin: 130px 0 40px;

2 CertainPerformance Sep 02 2020 at 22:45

Vous avez

const input = document.getElementsByClassName('new-todo')[0]

Lors de la sélection d'un seul élément, c'est un peu plus approprié et plus simple à utiliser à la querySelectorplace:

const input = document.querySelector('.new-todo');

Je remarque que parfois vous utilisez des points-virgules, et parfois non. À moins que vous ne soyez un expert, je vous recommande de les utiliser, sinon ASI pourrait vous faire trébucher, ce qui entraînerait des bogues difficiles à comprendre. Pensez à utiliser un linter pour appliquer votre style préféré.

Vous avez

var string = input.value;
// ...
li.innerHTML = `
    <div class="view">
        <input class="toggle" type="checkbox" />
        <label>${string}</label> <button class="destroy"></button> </div> <input class="edit" value="${string}">
`;

Ne concaténez pas les chaînes HTML à partir de l'entrée utilisateur; si l'utilisateur utilise accidentellement des caractères avec la syntaxe HTML, comme <, cela peut entraîner une apparence inattendue et des éléments inattendus dans le DOM. Il peut également exécuter du code arbitraire, ce qui pourrait être un risque pour la sécurité (imaginez si un autre utilisateur dit: "Essayez de mettre cela comme une entrée todo, vous ne croirez pas ce qui se passe ensuite!" Et la victime se fait voler ses informations de connexion) .

Au lieu de cela, une fois que vous avez une référence au conteneur du texte, attribuez au conteneur textContent:

var string = input.value;

if (e.key == 'Enter') {
  const li = document.createElement('li')
  li.innerHTML = `
      <div class="view">
          <input class="toggle" type="checkbox" />
          <label></label>
          <button class="destroy"></button>
      </div>
      <input class="edit" value="${string}">
  `;
  li.querySelector('label').textContent = string;

Vous avez un varlà, mais vous utilisez également const. Si vous allez écrire dans ES2015 avec constet let, c'est génial - mais il vaut mieux l'utiliser partout. (Si vous écrivez dans ES2015, n'utilisez jamais var. Règle de peluchage )