search

roger-melo-treinamentos / curso-de-js-roger-melo

Repositório de informações do CJRM
491 stars 173 forks source link

Aplicação: To Do List #5152

Closed silasgq closed 1 year ago

silasgq commented 1 year ago

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

Sim

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://drive.google.com/drive/folders/1r-lAwR7zqxjKq_rc1cpHi1UO7Wv41RGy?usp=sharing

Maiores dificuldades durante a implementação

Menores dificuldades durante a implementação

Roger-Melo commented 1 year ago

Olá @silasgq!

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

silasgq commented 1 year ago

Certo @Roger-Melo, obrigado!

MivlaM commented 1 year ago

Olá @silasgq.

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

Refatoração do código mais uma vez foi algo desafiador, mas acredito que isso faça parte do processo de aprendizagem;

Nomeação de variáveis e funções.

Ambos assuntos listados em maiores dificuldades serão abordados no decorrer da análise. O que é importante é que mesmo com as dificuldades relatada, você conseguiu fazer funcionar =)

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

Boas decisões

Os callbacks de eventos foram bem nomeados =)

O estilo da aplicação que você desenvolveu está bem diferente do que foi mostrado na aula. Isso é bom, pois numa situação onde um recrutador veja dois to-dos de alunos do CJRM e eles estão muito parecidos, isso pode passar a impressão que aconteceu um copia e cola de algum lugar. Isso não aconteceria com seu código, pois ele tem um estilo único =)

Indentação com quatro espaços ao invés de dois

Recomendo que ao invés de 4 espaços para indentação, use 2. E isso vale para qualquer linguagem (HTML, CSS, JS, etc).

Se um dia vc quiser enviar alguma sugestão ou modificação de artigo pro MDN, por exemplo, seu código terá que seguir o code-guideline deles:

E não só o MDN, mas muitas outras fontes recomendam que o código tenha 2 espaços ao invés de 4.

Seu olho irá cansar menos, pq suas linhas de código serão menos extensas, o que impacta na legibilidade e carga cognitiva na leitura.

À partir daqui, os exemplos do seu código que postarei abaixo já estarão formatados com 2 espaços, ao invés de 4.

Para configurar espaços no VSCode, dá uma olhada aqui:

Vc verá que nesse parágrafo, há um link para o get started de Settings do VSCode. Recomendo dar uma lida, mas não invista muito tempo nisso =)

Comentários no código

No código do seu index.html, há uma marcação HTML comentada. Não é uma regra, mas evite comentários.

Comentários podem mentir. Se o código for modificado e o comentário correspondente não for atualizado, o comentário perde o sentido. Isso sem mencionar o retrabalho de atualizar comentários.

Nomes de variáveis e funções

Se fizer sentido para você, toDoItems dentro de searchToDo pode ser substituída por todos, assim o código diminui de comprimento e facilita a leitura e escrita.

// antes
const toDoItems = Array.from(document.querySelectorAll('.listToDos--item'))

const inputSearchValue = (event.target.value).toLowerCase().trim()

Dentro da mesma função, você também pode mudar o nome de inputSearchValue pelos mesmos motivos citados acima, o novo nome ficaria como inputValue.

// depois
const todos = Array.from(document.querySelectorAll('.listToDos--item'))

const inputValue = (event.target.value).toLowerCase().trim()

Uma outra observação, as consts declaradas no início do código podem ter seus nomes reorganizados de forma que soem mais naturais segundo as normas ortográficas da língua inglesa. O que você pode fazer aqui, se achar que é coerente, é trocar o nome de todoFormAdd e de toDoInputSearch para formAddToDo e inputSearchToDo respectivamente.

// antes
const toDoFormAdd = document.querySelector('.todo--formAdd')
const toDoInputSearch = document.querySelector('.todo--formSearch input')
// depois
const formAddTodo = document.querySelector('.todo--formAdd')
const inputSearchToDo = document.querySelector('.todo--formSearch input')

Algo similar também pode ser aplicado à const clickedOnDeleteIcon dentro da função removeToDo. Se achar que faz sentido, essa const poderia ser renomeada para deleteIconWasClicked.

// antes
const clickedOnDeleteIcon = Array.from(event.target.classList).includes('delete-icon')
// depois
const deleteIconWasClicked = Array.from(event.target.classList).includes('delete-icon')

No intuito de melhorar a legibilidade do código, você também pode renomear a função hideOrShowItem para algo como filterTodos. Dessa forma, ao bater o olho no nome da função, a ação que ela executa fica explícita. O parâmetro element pode ser renomeado para algo mais claro em relação a seu papel no código, uma sugestão seria substituí-lo por todo, principalmente pelo fato de que seu papel é receber cada "todo" do array todos. A const toDoItem, se achar que faz sentido, pode ter seu nome modificado para algo mais correspondente ao que ela está armazenando, uma sugestão seria algo como todoText. As modificações ficariam da seguinte forma:

// antes
const hideOrShowItem = element => {
  const toDoItem = (element.textContent).toLowerCase().trim()
  const isContains = toDoItem.includes(inputAddValue)

  // ...
}
// depois
const filterTodos = todo => {
  const todoText = (todo.textContent).toLowerCase().trim()
  const isContains = todoText.includes(inputAddValue)

  // ...
}

Por fim, dentro da função addToDo, você pode renomear a const inputAddValue para algo mais fácil de ler para quem bater o olho, mais curto e que tenha o mesmo sentido, uma sugestão seria, se achar que é coerente, renomeá-la para inputValue.

// antes
const addToDo = event => {
  // ...

  const inputAddValue = event.target.addToDo.value.trim()

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

  const inputValue = event.target.addToDo.value.trim()

  // ...
}

Refatoração função addToDo

Há algumas refatorações que podemos fazer nessa função. A primeira delas é que você pode, se for interessante para você, mover o código de addItemToDo para a função addToDo. Um indicativo de que essa mudança pode fazer sentido é a similaridade dos nomes das funções, em ambos os casos, isso poderia ser simplificado numa única função. 

const addToDo = event => {
  // ...

  const inputValue = event.target.addToDo.value.trim()

  toDoList.innerHTML += ` <li class="listToDos--item u-center-elements">
                              <span class="todo--name">${inputValue}</span>
                              <i class="fa-solid fa-delete-left delete-icon"></i>
                            </li>`

  // ...
}

Uma outra observação seria em relação ao operador ternário. Eu não vejo necessidade de usar ternário para adicionar o todo, o uso ideal do ternário é quando baseado em uma condição, você precisa obter um valor. O ideal é usá-los para fazer algo com o valor que eles retornam ou seja, quando você precisar de um valor baseado em uma expressão, use ternário. No caso do seu código, acho que pode ser mais interessante não usarmos o ternário e substituí-lo nessa parte da lógica por um if com early return. 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 => {
  event.preventDefault()

  const inputValue = event.target.addToDo.value.trim()

  if (inputValue.length < 3 || inputValue === ''){
    alert("Por favor, insira um texto valido")  
    return
  }

  toDoList.innerHTML += ` <li class="listToDos--item u-center-elements">
                            <span class="todo--name">${inputValue}</span>
                            <i class="fa-solid fa-delete-left delete-icon"></i>
                        </li>`

  event.target.reset()
}

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

Refatoração função removeToDo

Se você quiser que o código fique mais conciso, uma sugestão é usar contains ao invés de includes na const deleteIconWasClicked. Por meio desse método, você não precisa transformar classList em array. Ficaria assim:

// antes
const deleteIconWasClicked = Array.from(event.target.classList).includes('delete-icon')
// depois
const deleteIconWasClicked = event.target.classList.contains('delete-icon')

Aqui também é um caso não ideal para o ternário, porém próprio para usarmos a técnica do early return novamente. 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. A principal vantagem dele aqui é que o código que essa função se propõe a executar não vai estar dentro de um bloco, além de ser muito mais simples e vantajoso que usar o ternário.

const removeToDo = event => {
    const deleteIconWasClicked = event.target.classList.contains('delete-icon')
    const parentclickedElement = event.target.parentElement

    if (!deleteIconWasClicked) {
        return
    }

    parentclickedElement.remove()
}

Você também pode fazer uma refatoração na função addToDo e na removeToDo por meio dos atributos data, em breve você chegará nas aulas sobre esses atributos. Mas para você ter uma ideia, você pode inserir o atributo data-todo na li do todo e colocar seu valor como o mesmo conteúdo da tag de span, depois adicionar um outro atributo data-trash no elemento html i da lixeira com novamente o mesmo conteúdo de span:


<li class="..." data-todo="Estudar Método MAP">
  <span>Estudar Método MAP</span>
  <i class="..." data-trash="Estudar Método MAP"></i>
</li>

<li class="..." data-todo="Estudar Método FILTER">
  <span>Estudar Método FILTER</span>
  <i class="..." data-trash="Estudar Método FILTER"></i>
</li>

<li class="..." data-todo="Estudar Método REDUCE">
  <span>Estudar Método REDUCE</span>
  <i class="..." data-trash="Estudar Método REDUCE"></i>
</li>

Depois, na função addToDo, você pode adicionar, dentro da template string, a nova marcação html atualizada com os atributos data. data-trash vai receber o conteúdo textual do todo, que no caso é o inputValue. Na li, o data-todo vai receber uma interpolação com o texto do todo do inputValue:

const addToDo = event => {
  // ...
    toDoList.innerHTML += `
      <li class="..." data-todo="${inputValue}">
        <span>${inputValue}</span>
        <i class="..." data-trash="${inputValue}"></i>
      </li>`
  // ... 
}

Após inserir os atributos data acima na marcação html, na função removeToDo, vamos usá-los também para fazer uma refatoração no lugar do parentElement. Não é bom usar parentElement, pois é um design de código que tende a ser frágil. Se futuramente a marcação html dessa aplicação mudar e o parentElement não for mais o elemento que você tinha em mente quando escreveu o código, o seu código quebra. O que não acontece quando você usa os atributos data. Por isso, você pode fazer o seguinte:

Você pode criar uma const trashWasClicked que vai receber o valor retornado pelo dataset que contém uma propriedade trash. Ou seja, quando a lixeira for clicada, o valor do atributo data-trash vai ser atribuida para a const trashWasClicked. Ficaria assim:

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

  if (!trashWasClicked) {
    return
  }

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

Ao clicar fora da lixeira, essa const vai armazenar undefined, que por ser um valor falsy, o if do early return vai ser executado. Quando a lixeira for clicada, trashWasClicked vai estar armazenando uma string preenchida, e uma string preenchida é um valor truthy, o if não será executado.

Você pode criar uma const todo que vai conter o todo, ou seja, a li que deve ser removida. Para selecionar essa li, você pode usar os atributos data. O data-todo tem o mesmo valor que data-trash, assim, você vai selecionar a li que contém o texto que trashWasClicked está armazenando, pois tanto data-todo quanto data-trash contém essa string que trashWasClicked está armazenando. Após a const todo receber a li que deve ser removida, você pode invocar o método todo.remove() para excluir a li escolhida do DOM.

Você irá aprender mais sobre os atributos data e suas particularidades nas próximas aulas do treinamento =)

Refatoração função searchToDo

Você pode deixar o código mais curto e com o mesmo funcionamento se usar a propriedade children ao invés de querySelectorAll no código armazenado pela const todos. Acredito que a legibilidade também se torne mais simples da seguinte forma:

// antes
const todos = Array.from(document.querySelectorAll('.listToDos--item'))
// depois
const todos = Array.from(toDoList.children)

Também pode ser interessante remover a const isContains e inserir o método includes que ela estava armazenando no próprio todoText. Fazendo isso, podemos também refatorar a lógica do ternário, fazendo-o abstrair as repetições e de benefício deixá-lo mais simples escrevendo menos código. Uma implementação ideal do ternário poderia ser feita ao fazer todo.style.display receber a condição de todoText, as refatorações ficariam da seguinte forma:

// antes
const filterTodos = todo => {
  const todoText = (todo.textContent).toLowerCase().trim()
  const isContains = todoText.includes(inputValue)

  isContains ? todo.style.display = 'flex' : todo.style.display = 'none'
}
// depois
const filterTodos = todo => {
  const todoText = (todo.textContent).toLowerCase().trim().includes(inputValue)

  todo.style.display = todoText ? 'flex' : 'none'
}

Você tem as minhas congratulações! =). A lógica que você usou para pesquisa e filtro dos todos é ainda mais interessante do que a lógica que costumo sugerir nessa parte da aplicação, porém ainda é válido eu te mostrar uma outra possível abordagem de refatoração do código aqui.

Acredito que isso pode tornar seu código melhor ainda =)

O intuito aqui é usarmos classList ao invés de estilo inline (style).

Inline style não é uma boa prática de coding, porque mistura código de estilo com código de marcação (HTML) e conteúdo, e isso pode deixar o código mais difícil de ler e entender. Por isso, quando possível é interessante evitar inserir estilo inline. O fato é que vc pode modificar um pouco da lógica para deixar esse código numa única função e menos propenso a ter repetições.

Olha só...

Vamos aproveitar parte do código que você escreveu e vamos inserir essa parte numa função searchToDo. Até o momento, temos uma const todos que contem as lis que representam os todos e uma const inputValue que recebe o valor escrito pelo usuário.

const searchToDo = event => {
  const todos = Array.from(toDoList.children)
  const inputValue = (event.target.value).toLowerCase().trim()
}

Agora, vamos criar um código em style.css de uma classe hide que contém display: none e uma classe show que contém display: flex. Essas serão as classes responsáveis para indicar quando deve ou não aparecer o todo relativo à busca feita pelo valor recebido no inputValue.

.show {
  display: flex;
}

.hide {
  display: none;
}

Feito isso, podemos percorrer cada um dos todos por meio do forEach, e após isso, podemos criar uma const shouldBeVisible que irá armazenar um boolean indicando se a li do todo deve ficar vísivel ou não dependendo do que o usuário escrever, depois disso, podemos manipular as classes dos to-dos considerando shouldBeVisible. Ficaria assim:

const searchToDo = event => {
  const todos = Array.from(toDoList.children)
  const inputValue = (event.target.value).toLowerCase().trim()

  todos.forEach(todo => {
    const shouldBeVisible = (todo.textContent).toLowerCase().trim().includes(inputValue)
    todo.classList.add(shouldBeVisible ? 'show' : 'hide')
    todo.classList.remove(shouldBeVisible ? 'hide' : 'show')
  })
}

Por fim, para dar mais legibilidade ao código e também deixar sua execução de forma explícita, podemos armazenar essa parte do código numa função própria chamada filterTodos e depois executá-la para cada um dos todos por meio de um forEach. Ficaria assim:

const searchToDo = event => {
  const todos = Array.from(toDoList.children)
  const inputValue = (event.target.value).toLowerCase().trim()

  const filterTodos = todo => {
    const shouldBeVisible = (todo.textContent).toLowerCase().trim().includes(inputValue)
    todo.classList.add(shouldBeVisible ? 'show' : 'hide')
    todo.classList.remove(shouldBeVisible ? 'hide' : 'show')
  }

  todos.forEach(filterTodos)
}

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 commented 1 year ago

Vou fechar a issue aqui, se tiver alguma nova dúvida, sinta-se livre para abrir uma nova issue =)