Applicazione Javascript to-do per Javascript nativo

Sep 02 2020

Ho creato una semplice applicazione da fare per Javascript nativo. Questa è la mia prima volta, in precedenza utilizzavo solo jQuery. Quali consigli sarebbero utili?

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> -->

Risposte

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

Problema di interazione dell'utente

C'è un difetto con il meccanismo del doppio clic per modificare: funziona solo per gli elementi esistenti, ma per gli elementi appena aggiunti, il gestore del doppio clic non è registrato. Lo stesso vale per i collegamenti di distruzione.

Invece di aggiungere un gestore dei clic a ogni elemento dell'elenco, è possibile utilizzare la delega dell'evento per aggiungere un gestore dei clic all'elenco e delegare l'evento di modifica all'elemento dell'elenco.

JS

selezionando gli elementi

So che CertainPerformance ha già toccato il codice per selezionare gli elementi. Per la selezione degli elementi dell'elenco, la delega dell'evento potrebbe essere ottenuta convertendo queste righe:

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

a questo:

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

Nota che getElementsByTagName()restituisce un 1 live, quindi non è necessario interrogare nuovamente l'elenco dopo che gli elementi sono stati aggiunti o rimossi ... e per scorrere gli elementi, possono essere inseriti in un array utilizzando l' operatore di diffusione :HTMLCollection

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

operatori di confronto

Molti raccomandano di utilizzare l'uguaglianza rigorosa quando si confrontano i valori, in questo modo non è necessario convertire i tipi. Ad esempio, in questa riga in runFilter:

if (href == 'active') {

hrefviene da var href = item.getAttribute('href')e quel metodo restituisce una stringa o null. Quindi non è necessaria la coercizione del tipo.

Metodi ripetuti per aggiornare le proprietà di visualizzazione degli elementi

Quella runFilterfunzione ha più forEach()cicli con funzioni freccia come questa:

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

Le funzioni ripetute potrebbero essere astratte in funzioni denominate per evitare ridondanze.

Inoltre, invece di impostare lo stile, i classListmetodi come add()e remove()potrebbero essere usati invece - con la classe CSS hidden.

CSS

C'è un set di regole per htmle body:

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

poi in seguito ce n'è uno per body:

body { 

e ha questo:

  margin: 0 auto;

Sembra eccessivo ... uno o entrambi potrebbero essere rimossi.

sintassi del margine

L'attuale CSS contiene questo:

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

La sintassi del margine può essere convertita da /* top | right | bottom | left */a /* top | horizontal | bottom */.

  margin: 130px 0 40px;
2 CertainPerformance Sep 02 2020 at 22:45

Avete

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

Quando si seleziona solo un singolo elemento, è un po 'più appropriato e più semplice da usare querySelectorinvece:

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

Noto che a volte usi il punto e virgola, ea volte no. A meno che tu non sia un esperto, ti consiglio di usarli, altrimenti ASI potrebbe farti inciampare, causando bug di difficile comprensione. Considera l' idea di utilizzare una linter per rafforzare il tuo stile preferito.

Avete

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}">
`;

Non concatenare stringhe HTML dall'input dell'utente; se l'utente utilizza accidentalmente caratteri con sintassi HTML, come <, ciò può comportare un aspetto imprevisto e elementi imprevisti nel DOM. Può anche eseguire codice arbitrario, che potrebbe essere un rischio per la sicurezza (immagina se un altro utente dicesse: "Prova a inserire questo come input di cose da fare, non crederai a quello che succede dopo!" E poi alla vittima vengono rubate le informazioni di accesso) .

Invece, una volta che hai un riferimento al contenitore del testo, assegna al contenitore 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;

Hai un varlì, ma stai anche usando const. Se hai intenzione di scrivere in ES2015 con conste let, è fantastico, ma è meglio usarlo ovunque. (Se stai scrivendo in ES2015, non usare mai var. Regola di lanugine )