Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

REACT + VITE TO DO APP

P

@Chaffexd

Desktop design screenshot for the Todo app coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
3intermediate
View challenge

Design comparison


SolutionDesign

Solution retrospective


This took me a few days and had a few challenges on the way.. I decided to render the components for mobile different to how you might normally do it. I would really appreciate feedback on this!

Community feedback

Pavel B. 270

@Jagholin

Posted

Some of the things that I would do differently:

  • you don't really need Sun and Moon components. Since you are using vitejs, it supports importing image assets and you can just set the svg files either as src attribute for an <img> tag, or as background-image CSS property on a div.
  • I find it better to destructure props immediately in the function argument list. Like this:
const Sun = ({onClick}) => {...}

This way, it is immediately apparent what props a component expects to receive, and you dont need to read the rest of the function code to find it out. But this is just my preference :)

  • when you have dependent variables like incompleteCount in the ToDo component, you can use useMemo hook instead, and get rid of associated useEffect. Like so:
const incompleteCount = useMemo(
    () => toDoList.filter(item => !item.complete).length, 
    [toDoList]);
  • newList.splice(index, 0, newList.splice(newList.indexOf(draggedItem), 1)[0]) is a bit hard to read. I would write it out:
newList.splice(newList.indexOf(draggedItem), 1); 
newList.splice(index, 0, draggedItem);
  • I wouldn't use *.classList.add / remove functions, because they do the same thing as className JSX attribute. When you have multiple ways of doing the same thing, prefer to use that which is more idiomatic to the libraries you use.

  • This is a terrible way of generating "unique" ids: Math.floor(Math.random() * 5000000). Just use natural numbers in order: 0, 1, 2, 3 etc.

  • About your original way of dealing with mobile/desktop layouts... I don't see the point in doing it like this. Not only are you duplicating your code, but your site actually breaks if you resize it (ToDo component is unmounted and all data is lost)

PS and additional thing:

  • It is good that you use key prop where React requires you to, but it seems that you dont really understand what it supposed to do. key allows React to distinguish components in an array, giving them something to be identified by. index here is not a good key, since it is possible to rearrange todo elements. You already have task.id property, so use it instead.
<div 
                key={entry.id}
                ...
>

Marked as helpful

1

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord