Skip to content

Conversation

@tildetilde
Copy link

tildetilde and others added 30 commits April 28, 2025 17:19
Added components LikeCounter, ThoughtItem and Spinner.
fetching and transforming data from API
fixed the error message when clicking submit button
Changed back to happy instead of thankful and changed timing of mylikedmessage
Pulsating heart when liking post
@tildetilde
Copy link
Author

Sorry for the delay, we missed to do the PR! This is our Happy thoughts project @Idahel :)

index.html Outdated
<html lang="en">
<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="./vite.svg" />

Choose a reason for hiding this comment

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

Adding a custom Favicon can help add some individuality and can be a fun addition

};
setThoughts([newThought, ...thoughts]);

const audio = new Audio(plingSound);

Choose a reason for hiding this comment

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

I love this

}
};

const handleLike = async (id: string) => {

Choose a reason for hiding this comment

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

To make the code easier to read, it could be a good idea to split it into a separate component. This way, it might also be easier to update things in the future.

@KidFromCalifornia
Copy link

Hey Ida and Tilde! I really liked your project. The page was visually easy to read and felt fun to explore the features.

One thing I noticed is that since you changed the original design elements, it would have been nice to improve the accessibility of the page. For example, some of the color contrast could be higher to make things clearer, and turning the shadow into an animation took away from that a bit. I also saw that the layout didn’t use much semantic HTML, which makes it harder to follow the structure and also affects accessibility. It was a little tricky to figure out how the different parts of the page were grouped — like what was part of the main content, what was a section, what was a heading, and so on.

In the code, I saw you used regular functions instead of arrow functions. That’s totally fine, but I was just curious if there was a reason for that. Maybe it’s easier with TypeScript? I don’t have a lot of experience with TypeScript myself, so it was actually cool to see how you used it. It gave me a better idea of what it looks like in a real project.

Also, I liked that you used Tailwind. I haven’t used it much before, so it was fun to see it in action. The audio and animation features were great too — they made the page more interactive and fun. One suggestion would be to add some ARIA labels, especially for things like sounds or animations. That way, it’s easier for screen readers to pick up on what is happening when someone clicks or hovers.

Another thing I noticed is that the app page felt a little hard to read. I think some of the functions could have been split into separate components to make things clearer. Right now, it’s a bit hard to tell which part belongs to what, and breaking things down might help with organization and readability.

Overall though, I think it looks great. I really liked the direction you took with it. You used a couple of tools and styles I’m not super familiar with, and it gave me some ideas for what I might want to try out in future projects.

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.

Please update your app with this new link (you can use cmd+shift+F to search for all the instances in a repo).

https://happy-thoughts-api-4ful.onrender.com/

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.

Looks great! Just the a11y requirement left (95 in Lighthouse score and sufficient contrasts) 💪

@tildetilde tildetilde requested a review from HIPPIEKICK May 28, 2025 10:14
@tildetilde
Copy link
Author

Thank you! :) Did some accessibility improvements accordingly.

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.

I was just made aware that a11y was not a requirement for this project, so I'm sorry for "forcing" you to improve it 😅

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.

4 participants