Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

To do app - using zustand for state management #34

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Sherrydev11
Copy link

Netlify link

https://taskmastertodoapp.netlify.app/

Collaborators

Collabed with: https://github.com/smily342

Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

Hi Sherry and Brian! Good job on completing this project. Your Zustand store looks well structured and you've met the requirements in general. I'd like to ask you to please clean up the code and to make sure you're VSC is set to 2 spaces for indentation. Also, remember that you don't need to import React.

Apart from that, keep up the good work!

Comment on lines +16 to +19
<section className="task-count">
<p>Total Tasks: {totalTasks}</p>
<p>Uncompleted Tasks: {uncompletedTasks}</p>
</section>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this could've been its own component?

@@ -0,0 +1,40 @@
// This component handles the input and allows adding new tasks.
import React, { useState } from "react";
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to import React

import { useTodoStore } from "../store/todoStore";

export const TaskInput = () => {
const [inputValue, setInputValue] = useState("");
Copy link
Contributor

Choose a reason for hiding this comment

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

2 spaces indentation is enough

Comment on lines +22 to +28
<input
type="text"
value={inputValue}
onChange={(e) => setInputValue(e.target.value)}
placeholder="Add a new task"
aria-label="Task input" // Accessibility for screen readers.
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget labels for form elements!!

Comment on lines +36 to +40





Copy link
Contributor

Choose a reason for hiding this comment

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

One empty line is enough

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