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

Fannys happy-thought-app #101

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

Conversation

Fannyhenriques
Copy link

@Fannyhenriques Fannyhenriques commented Oct 27, 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.

Kudos for creating your own timeAgo function! Next time maybe you want to try an external library 😉

Good job following the design so well!

All in all you've followed the requirements, but I left some comments for you to think about for upcoming projects 😊

<link
href="https://fonts.googleapis.com/css2?family=Inconsolata:[email protected]&family=Roboto:ital,wght@0,100;0,300;0,400;0,500;0,700;0,900;1,100;1,300;1,400;1,500;1,700;1,900&display=swap"
rel="stylesheet">
<title>Fannys Happy Thought App</title>
Copy link
Contributor

Choose a reason for hiding this comment

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

src/App.jsx Outdated
Comment on lines 10 to 12



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

src/App.jsx Outdated
Comment on lines 5 to 7
<>
<ThoughtPage />
</>
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 for React fragment if you're only returning one element

@@ -0,0 +1,166 @@
.thought-list {
list-style-type: none;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please set your VSC so one tab = 2 spaces. It gets easier to read

import "./DisplayThought.css"

export const DisplayThoughts = ({ happyThoughts, isLoading, postLike }) => {
if (isLoading) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here - hard to read with this super big indentation 😅

Comment on lines 3 to 4
import { useState, useEffect } from 'react'
import { DisplayThoughts } from "./DisplayThought"
Copy link
Contributor

Choose a reason for hiding this comment

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

Consistency with single or double quotes please

//postcall for likes on thoughts
const postLike = async (thoughtId) => {
try {
const response = await fetch(LIKE_URL.replace("THOUGHT_ID", thoughtId), {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ha - nice hack!

@Fannyhenriques
Copy link
Author

Thank you for your feedback, @HIPPIEKICK! 😊 I’m not sure why the indentation keeps reverting to 4 spaces when I’ve set the default to 2. 😅 I’ll look into it, but for now, I’ve updated the code based on your comments.

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