Aplicação: Todo-List

[rafaelpradoj]

As cores da sua versão da aplicação são diferentes das cores da aplicação mostrada na aula?

Não

A aplicação contém funcionalidades que não foram mostradas nas aulas?

Não

A aplicação contém funcionalidades da linguagem que não foram mostradas nas aulas?

Não

Link do repositório da aplicação (ou pasta pública no Google Drive)

https://todorafaelpradoj.netlify.app/

Maiores dificuldades durante a implementação

No início tive dificuldades para entender o atributo data, mas com a prática fui melhorando.

A refatoração do código, se tratando de funções isoladas, é algo que eu preciso focar mais. Consigo resolver os exercícios de boa, mas ainda não consigo bater o olho no código e pensar: Caramba! Consigo criar uma função pra isso e usar ela em mais partes do meu código.

Menores dificuldades durante a implementação

Manipulação de elementos HTML, criação de eventListeners, funções.

Também tô gostando muito do Bootstrap, uso bastante nos meus projetos pessoais. Porém sempre estudando HTML e CSS puros pra não ficar muito dependente de frameworks.

[MivlaM]

Olá, @rafaelpradoj.

O link que você postou na issue é referente à aplicação já hospedada no Netlify, para podermos prosseguir com a análise, solicito o link do repositório da aplicação no github ou pasta pública no Google Drive.

Uma outra observação, eu acessei a aplicação pelo link do Netlify, mas identifiquei que o primeiro dos critérios para que a análise seja feita está em falta, então sugiro que certifique-se que o repositório da aplicação também esteja de acordo com o critério.

O critério é o seguinte:

Critério 1: as cores da sua versão da aplicação devem ser diferentes das cores da aplicação mostrada na aula.

Isso é fundamental para que a sua versão da aplicação tenha uma aparência própria. Se quiser, você pode usar o bootstrap para evitar investir muita energia no design da interface.

Os critérios e regras para que a análise seja feita foram enviados no email "[Alunos CJRM] Análises de Aplicações".

Eu recomendo que você salve esse email em algum lugar e marque ele com estrela (no gmail). Por que ele é importante e você terá que voltar nele quando enviar as aplicações.

Avise por aqui quando fizer a mudança =)

Fico no aguardo.

[rafaelpradoj]

Desculpe pela confusão, aqui está o endereço correto https://github.com/rafaelpradoj/todoList

Quanto ao email, agradeço se puder me enviar novamente, devo ter apagado sem querer.

Obrigado!

[MivlaM]

Olá @rafaelpradoj.

O email foi enviado novamente às 22:25 do dia de ontem. Você pode conferir lá para ver se está tudo ok.

Passando para dizer que visualizei a issue e em até 10 dias úteis a análise será feita =)

[rafaelpradoj]

Recebido, muito obrigado!

[MivlaM]

Olá @rafaelpradoj.

Parabéns pelo esforço em construir a segunda aplicação do treinamento =)

A refatoração do código, se tratando de funções isoladas, é algo que eu preciso focar mais. Consigo resolver os exercícios de boa, mas ainda não consigo bater o olho no código e pensar: Caramba! Consigo criar uma função pra isso e usar ela em mais partes do meu código.

Tenha consciência que refatoração é uma habilidade que vai sendo elevada conforme vc pratica. E vc ainda praticará bastante durante o treinamento e será exposto há vários casos de uso.

Mas o mais importante num primeiro momento vc já fez, que é fazer funcionar. Refatoração vem depois =)

Vou deixar abaixo algumas observações sobre sua aplicação.

---

Indentação

Indentação é um dos pilares de um código mais legível.

Sabendo disso, você pode deixar o código mais fácil de entender ao indentar a marcação html dentro da template string dentro do bloco do if. No código atual, ela está na mesma linha que o todosContainer, talvez isso passe a impressão que elas são independentes, o que não é verdade. Portanto, faz mais sentido que a marcação esteja um pouco mais à direita. Ficaria assim:

// antes
if (inputValue.length) {
    todosContainer.innerHTML += `
    <li class="list-group-item d-flex justify-content-between align-items-center" data-todo="${inputValue}">
      <span>${inputValue}</span>
      <i class="far fa-trash-alt" data-trash="${inputValue}"></i>
    </li>`

    // ...
}

// depois
if (inputValue.length) {
    todosContainer.innerHTML += `
      <li class="list-group-item d-flex justify-content-between align-items-center" data-todo="${inputValue}">
        <span>${inputValue}</span>
        <i class="far fa-trash-alt" data-trash="${inputValue}"></i>
      </li>`

    // ...
}

---

Objeto event

Dentro da função addTodo, você percebe que na linha de código event.target.reset(), o objeto event está riscado? Isso acontece pois na função addTodo, o event não está sendo usado para nada. Isso significa que a expressão está usando o event global.

Mesmo que tenha funcionado, o ideal é que o event seja usado na expressão. Isso por que o event global foi descontinuado. O VSCode geralmente insere um traço no meio de propriedades e métodos que foram descontinuados.

Quando você insere o event como parâmetro de um callback de listener de evento e usa ele, ao invés de usar o event global, você está usando o Event interface =)

Uma possível solução, se fizer sentido para você, seria mover o trecho de código no qual o event é usado como parâmetro para a função na qual ele está sendo utilizado, dessa forma, o objeto event não estará mais riscado e o código se tornará mais limpo. No caso da sua aplicação, o event pode ser movido para dentro da função addNewTodos, ficaria assim:

// antes
const addTodo = inputValue => {
  if (inputValue.length) {
    // ...

    event.target.reset()
  }
}

// depois
const addNewTodos = event => {
  event.preventDefault()
  // ...

  event.target.reset()
}

---

Nomenclatura

Um pequeno ajuste ortográfico pode ser feito, faltou um "c" antes do "k" na função removeClikedTodos, portanto ela está ortograficamente incorreta. A versão correta seria removeClickedTodos.

// antes
const removeClikedTodos = event => {
  // ...
}

// depois
const removeClickedTodos = event => {
  // ...
}

Uma outra observação seria, se fizer sentido para você, renomear a const trashDataValue para algo como trashWasClicked para indicar que o ícone da lixeira foi clicado no todo. Assim fica mais claro para quem está lendo ou trabalhando com o código, até mesmo pela primeira vez, que o ícone da lixeira foi clicado.

// antes
const trashDataValue = clickedElement.dataset.trash

// depois
const trashWasClicked = clickedElement.dataset.trash

---

Refatoração

Você pode, se for interessante para você, mover o código de addNewTodos para a função addTodo e também o código de removeClickedTodos para removeTodo, nesse último caso, você economizará uma linha de código, pois a const clickedElement não será mais necessária. Um indicativo de que essas mudanças podem fazer sentido é a similaridade dos nomes das funções, em ambos os casos, isso poderia ser simplificado numa única função.

const addTodo = event => {
  event.preventDefault()
  const inputValue = event.target.add.value.trim()

  if (inputValue.length) {
    todosContainer.innerHTML += `
      <li class="list-group-item d-flex justify-content-between align-items-center" data-todo="${inputValue}">
        <span>${inputValue}</span>
        <i class="far fa-trash-alt" data-trash="${inputValue}"></i>
      </li>`
  }

  event.target.reset()
}

const removeTodo = event => {
  const trashWasClicked = event.target.dataset.trash
  const todo = document.querySelector(`[data-todo="${trashWasClicked}"]`)

  if (trashWasClicked) {
    todo.remove()
  }
}

Uma sugestão seria, se você achar que faz sentido, usar um early return dentro da função addTodo. A vantagem do early return é parar a execução da função naquele ponto caso a condição seja falsa, assim não se torna necessário ler a parte do código que vem em seguida. Além disso, usando um early return, você não precisa inserir o código da marcação html dentro do bloco do if, reduzindo então o número de indentações para essa marcação e tornando o código mais legível. Veja abaixo:

const addTodo = event => {
  // ...

  if (!inputValue.length) {
    return
  }

  todosContainer.innerHTML += `
    <li class="list-group-item d-flex justify-content-between align-items-center" data-todo="${inputValue}">
      <span>${inputValue}</span>
      <i class="far fa-trash-alt" data-trash="${inputValue}"></i>
    </li>`

Dentro desse if do early return, qualquer coisa pode ser feita, como por exemplo, fazer uma validação de input onde exibirá uma mensagem na tela indicando que o texto inserido é invalido. Sempre que possível e sempre que fizer sentido, é preferível usar um early return do que executar uma leitura de código que não necessariamente precisa ser lida =)

Um pequeno refactoring que você pode fazer na removeTodo é buscar o to-do no DOM apenas se ele de fato tiver que ser removido. Assim você elimina uma query desnecessária no DOM caso o elemento clicado não seja o de remoção do to-do =)

// antes
const removeTodo = event => {
  const trashWasClicked = event.target.dataset.trash
  const todo = document.querySelector(`[data-todo="${trashWasClicked}"]`)

  if (trashWasClicked) {
    todo.remove()
  }
}

// depois
const removeTodo = event => {
  const trashWasClicked = event.target.dataset.trash

  if (trashWasClicked) {
    const todo = document.querySelector(`[data-todo="${trashWasClicked}"]`)
    todo.remove()
  }
}

Você também pode, se achar que faz sentido, parar a execução dessa função no mesmo instante que a condição do if for falsa. Você pode fazer isso por meio de outro early return, a principal vantagem dele aqui é que o código que essa função se propõe a executar não está mais dentro de um bloco e consequentemente não é mais necessário que ele esteja indentado à direita.

const removeTodo = event => {
  const trashWasClicked = event.target.dataset.trash

  if (!trashWasClicked) {
    return
  }

  const todo = document.querySelector(`[data-todo="${trashWasClicked}"]`)
  todo.remove()
}

Vou deixar uma possível abordagem de refatoração do código que busca os to-dos. Acredito que isso pode te dar uma luz =)

O fato é que vc pode modificar um pouco da lógica para deixar esse código menos propenso a ter repetições.

Para entender de fato essa forma de refatorar a filtragem de to-dos que vou mostrar, você precisa escrever e executar cada código do passo a passo abaixo junto comigo, ok?

Olha só...

Uma forma de abstrair as repetições desse código é usar a transformação de dados ao seu favor.

Como assim?

É possível criar um array de objetos em que cada objeto, além de conter a referência do to-do no DOM, contém um boolean indicando se esse to-do deve ficar visível.

Eu "desfiz" as funções relacionadas à busca de to-dos e vamos ir codando passo a passo. Você pode declarar uma const todos que contenha as lis que representam os todos:

inputSearch.addEventListener('input', event => {
  const inputValue = event.target.value.trim().toLowerCase()
  const todos = Array.from(todosContainer.children)
})

Agora, você pode invocar console.log(todos) e buscar por eles, vai aparecer no console as lis de cada todo.

inputSearch.addEventListener('input', event => {
  const inputValue = event.target.value.trim().toLowerCase()
  const todos = Array.from(todosContainer.children)

  console.log(todos)
})

Agora, sabendo que todos precisa armazenar um array de objetos e cada objeto precisa conter uma propriedade todo que vai armazenar a li e uma propriedade que vai receber um boolean indicando se esse todo vai ficar vísivel ou não, se para cada li do array você precisa gerar um objeto e o array gerado vai ter a mesma quantidade de itens que o original, você pode usar o método map. Já que cada item do array ao qual o map está sendo encadeado representa um todo, você pode inserir um parametro todo e fazer a função retornar um objeto. A cada execução do callback do map, um objeto vai ser inserido no novo array que o map está gerando. Ênfase que você não pode remover os parênteses após ter declarado uma propriedade, pois caso contrário o Javascript vai interpretar a abertura de chave como bloco da função. Para que o Javascript considere essa abertura e fechamento de chave como um objeto, você vai precisar desses parênteses conhecidos como grouping operator, eles que fazem com que a precedência dessa expressão seja o objeto literal.

inputSearch.addEventListener('input', event => {
  const inputValue = event.target.value.trim().toLowerCase()
  const todos = Array.from(todosContainer.children).map(todo => ({
    todo
  }))

  console.log(todos)
})

Em seguida, acrescentamos uma propriedade shouldBeVisible que armazena um boolean indicando se o todo deve ficar visível ou não. Você pode fazer isso pela expressão todo.textContent.toLowerCase().includes(inputValue) em que todo.textContent.toLowerCase() retorna o texto do todo em letras minúsculas e nessa string, você está verificando por meio do método includes() se ela contém a string da inputValue.

inputSearch.addEventListener('input', event => {
  const inputValue = event.target.value.trim().toLowerCase()
  const todos = Array.from(todosContainer.children).map(todo => ({
    todo,
    shouldBeVisible: todo.textContent.toLowerCase().includes(inputValue)
  }))

  console.log(todos) /* [{…}, {…}, {…}] 👈🏻 cada objeto contém um boolean que indica se o to-do deve ficar visível */
})

Tudo certo até aqui?

Agora que há um array de objetos que indicam quais to-dos devem ficar visíveis, vc pode manipular as classes dos to-dos por meio do forEach considerando a propriedade shouldBeVisible:

inputSearch.addEventListener('input', event => {
  const inputValue = event.target.value.trim().toLowerCase()
  const todos = Array.from(todosContainer.children).map(todo => ({
    todo,
    shouldBeVisible: todo.textContent.toLowerCase().includes(inputValue)
  }))

  todos.forEach(({ todo, shouldBeVisible}) => {
    todo.classList.add(shouldBeVisible ? 'd-flex' : 'd-none')
    todo.classList.remove(shouldBeVisible ? 'd-none' : 'd-flex')
  })
})

Por fim, podemos nomear o callback do listener de evento como searchTodo, por exemplo. Nomear callbacks de listeners de eventos dá mais legibilidade ao seu código, porque ao bater o olho no nome da função, a ação que ela executa fica explícita:

const searchTodo = event => {
  const inputValue = event.target.value.trim().toLowerCase()
  const todos = Array.from(todosContainer.children).map(todo => ({
    todo,
    shouldBeVisible: todo.textContent.toLowerCase().includes(inputValue)
  }))

  todos.forEach(({ todo, shouldBeVisible}) => {
    todo.classList.add(shouldBeVisible ? 'd-flex' : 'd-none')
    todo.classList.remove(shouldBeVisible ? 'd-none' : 'd-flex')
  })
}

Entendeu a ideia?

Mas o mais importante num primeiro momento vc já havia feito, que é fazer funcionar. O que sugeri é apenas uma lapidação de um código que já funciona =)

---

Nas aulas seguintes, você verá outras formas de refatorar o código dessa aplicação.

Mais uma vez, parabéns pelo esforço =)

As observações fizeram sentido?

[MivlaM]

Show! Vou fechar a issue mas no que precisar, é só abrir uma nova =)