Conversation
Appilistus
left a comment
Appilistus
left a comment
There was a problem hiding this comment.
Great job Julia! The app looks good and works as it should. I think you covered most of the requirements (only the lighthouse score is missing). The code is clean and easy to follow. It's great that you've managed to build the app with such a small number of components! I added some comments about accessibility, hope it helps. But overall you have done a good job! well done!
| useEffect(() => { | ||
| const init = async () => { | ||
| // RECEIVING also called GETing | ||
| const response = await fetch("https://happy-thoughts-api-4ful.onrender.com/thoughts") | ||
|
|
||
| const returnedResponseValueInJSON = await response.json() | ||
|
|
||
| const newArrayWithThoughts = returnedResponseValueInJSON.map((item) => { | ||
| return { | ||
| id: item._id, | ||
| likes: item.hearts, | ||
| text: item.message, | ||
| createdAt: item.createdAt | ||
| } | ||
| }) | ||
|
|
||
| setThoughts(newArrayWithThoughts) | ||
| } |
There was a problem hiding this comment.
It would be good to add error handling in case API fetch doesn't work:)
There was a problem hiding this comment.
Yes I agree, that would be a good addition! :)
| ) | ||
| <> | ||
| <GlobalStyle /> | ||
| <AppWrapper className="AppWrapper"> |
There was a problem hiding this comment.
Does this need to have className?:)
| @@ -0,0 +1,96 @@ | |||
| import React, { useState } from "react" | |||
| <Input | ||
| rows={3} | ||
| value={text} | ||
| onChange={handleChange} | ||
| maxLength={MAX_LEN} | ||
| name="text" | ||
| /> |
There was a problem hiding this comment.
Lighthouse is warning about there is no label here. You might wanna add aria-label for better accessibility:)
|
|
||
| const Counter = styled.p` | ||
| font-size: 15px; | ||
| color: #c9c8c8; |
There was a problem hiding this comment.
Lighthouse complains about the color contrast here. You need to change the color darker for better accessibility:)
| ) | ||
| } | ||
|
|
||
| const Span = styled.span` |
There was a problem hiding this comment.
The font size might be a bit too small.
| } | ||
|
|
||
| const Span = styled.span` | ||
| float: right; |
There was a problem hiding this comment.
I havent seen float in a while! :) It's not considered deprecated, but barely anyone uses it any more. Try to use flex or grid instead and float can cause issues if not handled correctly. In this case it works fine though!
src/App.jsx
Outdated
| useEffect(() => { | ||
| const init = async () => { | ||
| // RECEIVING also called GETing | ||
| const response = await fetch("https://happy-thoughts-api-4ful.onrender.com/thoughts") |
There was a problem hiding this comment.
A good thing is to have stuff like text and numbers that are used in more than one place as global constants. That way it's easy to change them, and you only need to change them in one place!
Npahlfer
left a comment
Npahlfer
left a comment
There was a problem hiding this comment.
Generally good, it would have been nice to see some of the stretch goals like the loading and local storage, but your code is easy to follow, but try to make use of shared constants :)
3 participants
Netlify link: https://happy-thoughts-pr.netlify.app/