Aplicación de tareas de Javascript para Javascript nativo

Sep 02 2020

Hice una sencilla aplicación de tareas pendientes para Javascript nativo. Esta es mi primera vez, anteriormente solo usaba jQuery. ¿Qué recomendaciones serían útiles?

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

Respuestas

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

Problema de interacción del usuario

Hay una falla con el mecanismo de doble clic para editar: solo funciona para elementos existentes, pero para los elementos recién agregados, el controlador de doble clic no está registrado. Lo mismo ocurre con los enlaces de destrucción.

En lugar de agregar un controlador de clic a cada elemento de la lista, la delegación de eventos podría usarse para agregar un controlador de clic a la lista y delegar el evento de edición al elemento de la lista.

JS

seleccionando elementos

Sé que CertainPerformance ya ha tocado el código para seleccionar elementos. Para la selección de elementos de la lista, la delegación de eventos podría lograrse convirtiendo estas líneas:

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

a esto:

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

Tenga en cuenta que getElementsByTagName()devuelve un ^{1 en} vivo, por lo que no es necesario volver a consultar la lista después de agregar o eliminar elementos ... y para iterar sobre los elementos, se pueden colocar en una matriz utilizando el operador de extensión :HTMLCollection

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

operadores de comparación

Muchos recomiendan usar la igualdad estricta al comparar valores; de esa manera, no es necesario convertir los tipos. Por ejemplo, en esta línea en runFilter:

if (href == 'active') {

hrefproviene var href = item.getAttribute('href')y ese método devuelve una cadena o null. Por lo tanto, no hay necesidad de coerción de tipos.

Métodos repetidos para actualizar las propiedades de visualización del elemento

Esa runFilterfunción tiene múltiples forEach()bucles con funciones de flecha como esta:

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

Las funciones repetidas podrían abstraerse en funciones nombradas para evitar redundancias.

Además, en lugar de establecer el estilo, los classListmétodos como add()y remove()podrían usarse en su lugar, con la clase CSS hidden.

CSS

Hay un conjunto de reglas para htmly body:

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

luego más adelante hay uno para body:

body {

y tiene esto:

  margin: 0 auto;

Eso parece excesivo ... uno o ambos podrían eliminarse.

sintaxis de margen

El CSS actual contiene esto:

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

La sintaxis de margen se puede convertir de /* top | right | bottom | left */a /* top | horizontal | bottom */.

  margin: 130px 0 40px;

2 CertainPerformance Sep 02 2020 at 22:45

Tu tienes

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

Al seleccionar un solo elemento, es un poco más apropiado y más terso usar querySelectoren su lugar:

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

Noto que a veces estás usando punto y coma y otras no. A menos que sea un experto, le recomendaría usarlos, de lo contrario, ASI puede hacerle tropezar y generar errores difíciles de entender. Considere usar un linter para reforzar su estilo preferido.

Tu tienes

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

No concatenar cadenas HTML de la entrada del usuario; si el usuario usa accidentalmente caracteres con sintaxis HTML, como <, esto puede resultar en una apariencia inesperada y elementos inesperados en el DOM. También puede ejecutar código arbitrario, lo que podría ser un riesgo para la seguridad (imagínese si otro usuario dijera: "Intente poner esto como una entrada de tareas pendientes, ¡no creerá lo que sucede a continuación!" Y luego le robarán la información de inicio de sesión a la víctima). .

En cambio, una vez que tenga una referencia al contenedor del texto, asigne al contenedor 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;

Tienes un varahí, pero también lo estás usando const. Si va a escribir en ES2015 con consty let, eso es genial, pero es mejor usarlo en todas partes. (Si está escribiendo en ES2015, nunca use var. Regla de linting )