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

feat/todo #4

Merged
merged 36 commits into from
Aug 26, 2024
Merged

feat/todo #4

merged 36 commits into from
Aug 26, 2024

Conversation

danifromecuador
Copy link
Owner

@danifromecuador danifromecuador commented Aug 14, 2024

In this PR, I've:

  • Created a feat/todo branch
  • Created a Todo component that will allow the user to:
    • Add a new goal
    • Mark that goal as completed
    • Delete all completed goals
    • See the percentage of completed goals
  • Reused Todo component in 3 components: Daily, Weekly, and Monthly goals, those components are being called on the Todos component and the only one given prop is their corresponding slice of the store
  • All data for each component is saved in the global store that is managed by Zustand
  • You can still use the Redux extension to see the global statue on the browser thanks to a Zustand feature even when this project doesn't use Redux
  • Saved goals components data on local storage, so that data will be preserved when refreshing the page
  • Conditional rendering for the Delete All Completed button and Completed percentage indicator
  • Applied minimal style and responsiveness, it will be improved in ahead PRs.
  • Behavior:
todos.behavior.mp4

branch feat/todo
branch feat/todo
depending if there is or not one or more completed or done goal
branch feat/todo
branch feat/todo
to avoid creating add and other functions for each daily weekly and monthly
branch feat/todo
because that prop will be added on the Store
branch feat/todo
as this prop will be given in the Store
branch feat/todo
Copy link
Collaborator

@Diegogagan2587 Diegogagan2587 left a comment

Choose a reason for hiding this comment

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

@danifromecuador Hi Buddy! ,

Status: Almost there! 🟠 🟠 🟠

:shipit:
Great Job So far! 🎉

Highlights

  • Each section (Daily Goals, Weekly Goals and Monthly Goals) is adding or removing tasks.
  • Nice Job rendering the progress shown as percentage.
  • Timer plays a sound when time is over

Suggested changes:

Specify action names

You may ask, what is the issue with that? Well, when the user executes an action like add a task, an action name will describe the action that has been executed, not adding action names will make debugging harder and the action would be named as anonymous in dev tools. as shown in the example below:
image
The problem will occur if you want do debug, you will not know which action to check because they are named as anonimous. To prevent this problem specify an action name on each action that updates the state as shown in the example below:

// todo_logic.js
export const add = (set, input, sliceID) => set((state) => ({
  [sliceID]: {
    ...state[sliceID],
    todos: [...state[sliceID].todos, { id: Date.now(), content: input }]
  }
}), false, 'todo/add')

after adding an action name you should be able to debug properly if there is an error:
image

Summary of pending task

  • Specify action names for each action

references

Zustand Issue: Actions Names in Devtools
Zustand Readme # Login Actions

If you think we should take another approach share it with the team!!! Thanks for your effort!

Cheers, and Happy coding!👏👏👏

Please, remember to tag me in your question so I can receive the notification._

Comment on lines 5 to 7
}
}))

Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be great to add an action name here as show on code review example.

Comment on lines 13 to 15
}
}))

Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be great to add an action name here as show on code review example.

Comment on lines 21 to 23
}
}))

Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be great to add an action name here as show on code review example.

Comment on lines 31 to 33
dones: []
}
}))
Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be great to add an action name here as show on code review example.

by passing a third parameter on Store functions when using Redux devtools
branch feat/todo
@danifromecuador
Copy link
Owner Author

Thanks for the valuable feedback, dear @Diegogagan2587, The required changes were made and now all the actions have a name that will be helpful when debugging.
image

Copy link
Collaborator

@Diegogagan2587 Diegogagan2587 left a comment

Choose a reason for hiding this comment

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

@danifromecuador Hi Buddy! ,

Status: Approved! 🟢 🟢 🟢

:shipit:
Great Job So far! 🎉

Highlights

  • Each section (Daily Goals, Weekly Goals and Monthly Goals) is adding or removing tasks.
  • Nice Job rendering the progress shown as percentage.
  • Timer plays a sound when time is over
  • Requested Changes were applied.

Suggested changes:

  • All suggested changes were applied.

If you think we should take another approach share it with the team!!! Thanks for your effort!

Cheers, and Happy coding!👏👏👏

Please, remember to tag me in your question so I can receive the notification._

@danifromecuador
Copy link
Owner Author

Thanks, dear reviewer!

@danifromecuador danifromecuador merged commit af5c1bf into develop Aug 26, 2024
3 checks passed
@danifromecuador danifromecuador deleted the feat/todo branch August 26, 2024 23:31
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