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

Happy Thoughts Project #96

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

Happy Thoughts Project #96

wants to merge 27 commits into from

Conversation

gittebe
Copy link

@gittebe gittebe commented Oct 26, 2024

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.

Good job on creating this happy thoughts app Gitte! Looks really good and I like that you added the little animation when you submit a happy thought 🥰

You have a clean App.jsx which is good practice 👍 Also nice to see that you abstracted the fetches like we did in class. Remember that one of the benefits with async/await is that we can wrap it in a try/catch, would love to see that next time!

Some notes about clean code:

  • No need for a lot of empty lines - remove! One empty line at the end of each file is convention, but not in the beginning or middle of a file
  • Please set your VSC to make 2 spaces for one tab (indentation). When the indentation is more than that it becomes so much harder too read

And some more kudos:

  • Really nice with the character counter
  • Good structure and nice to see that you're breaking your project up into components like this ⭐
  • timeDifference function! 💪

Keep up the good work Gitte!

Copy link

@joheri1 joheri1 left a comment

Choose a reason for hiding this comment

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

Well done with this project, Gitte! I learned so much from reading your code – thank you for that! I’ll definitely be using some of the smart design choices you made, like separating the API functions and hooks into their own components. How genius! This approach really makes the app scalable and easy to maintain.

I'm also really impressed that you tackled all the stretch goals! It was fun to see how they could be solved, like the cute animation on 'Submit' and the character counter with its warning.

Seeing how you used PropTypes has inspired me to try them out too. What a great way to validate data, keep the code clean, and avoid bugs!

I'm glad I picked your code to review!

src/App.jsx Outdated
Copy link

Choose a reason for hiding this comment

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

Nice that you kept the App.jsx so clean!

}
}

// this function handles the likes of the thought by its ID. It calls the function likeThought, and if successful, updates the local state with the new number of hearts by mapping over the previous thoughts
Copy link

Choose a reason for hiding this comment

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

I really appreciate the sections with comments! They make it much easier to understand the purpose of each part. Great job on keeping the code so readable!


Time.propTypes = {
createdAt: PropTypes.string.isRequired,
}
Copy link

Choose a reason for hiding this comment

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

Good job with adding the extra empty line in each section

@@ -0,0 +1,20 @@

import PropTypes from "prop-types"
Copy link

Choose a reason for hiding this comment

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

Really cool that you're using PropTypes! A solid approach for controlling and validating when the animation should show. Great choice!

)
}

LikeButton.propTypes = {
Copy link

Choose a reason for hiding this comment

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

Nice that you're using PropTypes to validate the data, it really helps keeping the code reliable.

CharacterCounter.propTypes = {
currentLength: PropTypes.number.isRequired,
maxChars: PropTypes.number.isRequired,
}
Copy link

Choose a reason for hiding this comment

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

I really liked this counter and it worked like a charm too! It gives the app even more of an old-school vibe, which I love. The isOverLimit logic is clear and easy to follow. Great job!

export const likeThought = async (thoughtId) => {
const response = await fetch(`${BASE_URL}/${thoughtId}/like`, { method: "POST" })
if (!response.ok) throw new Error("Failed to like thought")
}
Copy link

Choose a reason for hiding this comment

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

Smart to make the API functions standalone! That makes it easier to reuse them across other components since you don't have to duplicate the code.

}, [])

return { thoughts, setThoughts, loading, getThoughts }
}
Copy link

Choose a reason for hiding this comment

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

Great choice to put the hooks in their own component. Really smart design!

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.

3 participants