Todo app with NEXT js and Tailwind CSS

Community feedback

• 

  Daniel•120

  @dvenomb98Posted about 2 years ago

  To your question: rework your solution so you use flex or grid layout with zero absolute positions. And let me know. I am 100% sure thats problem.

  Marked as helpful

  0

  ---

  

  Ivana Rajic•270

  @ivanarajicPosted about 2 years ago

  @dvenomb98 thank you, flex solved the problem.

  0

  

  Daniel•120

  @dvenomb98Posted about 2 years ago

  @ivanarajic Happy to hear it !

  0
• 

  Daniel•120

  @dvenomb98Posted about 2 years ago

  Hi, nice work.. few suggestions:

  1/ Todos.js: line 148: could be just todos.length instead of todos.length > 0

  2/ TodoFilter.js: u could create list of buttons and map over instead of copying it every single time with different value.

  something like: const buttons = [{label: Active, value: active}]

  and then u could do something like: buttons.map(b => ( <p classNames={filterStatus === b.value ? "text-red" : "text-blue"} onClick={() => handleClick(b.value)}> {b.label} </p> ))`

  3/ zero needs to use absolute position in project like this. instead use flex or grid

  4/ You really dont need two different states for todos (todos and filtered todos). It is not efficient and doesnt really work like that. Instead try use some filter logic before mapping over to component props.

  U could do: todos.filter(todo => { if (!searchValue) return

  if (searchValue === todo.name) return todo }) .map(t => ( <Todo .{...props} /> )

  just quick example, but you shouldnt really use 2 different states.

  Marked as helpful

  0

  ---

  

  Ivana Rajic•270

  @ivanarajicPosted about 2 years ago

  @dvenomb98 thank you so much for the suggestions, I'm gonna refactor my code!

  0

  

  Daniel•120

  @dvenomb98Posted about 2 years ago

  @ivanarajic Would do it like this https://ibb.co/Vg7FWnc, then u can delete useEffect and filtered todos state

  1

  

  Ivana Rajic•270

  @ivanarajicPosted about 2 years ago

  @dvenomb98 thanks, I like your solution.

  0