Skip to content

todo app Nicolina#53

Open
Nicolinabl wants to merge 10 commits intoTechnigo:mainfrom
Nicolinabl:main
Open

todo app Nicolina#53
Nicolinabl wants to merge 10 commits intoTechnigo:mainfrom
Nicolinabl:main

Conversation

@Nicolinabl
Copy link
Copy Markdown

Copy link
Copy Markdown

@Npahlfer Npahlfer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job with the todo app Nicolina!
You have done a good job with using zustand and its store throughout your app.

There are a few things that you could do to make it even more solid.
You have removeTask defined twice in the store. One as a state variable and one redefined as a state function. It doesnt affect the app, but it would help to keep it clean if you removed that state variable and also the isCompleted variable above, which is never used.
And also defining colors and such as variables or in a theme file will also help to make the app easier maintain and look cleaner :)

I did notice that you have one place for all the icons which is superb! You could even split them up into their own components. That would have been even nicer!

But overall really good job with this one Nicolina!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants