-
Notifications
You must be signed in to change notification settings - Fork 10
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
Atk technical #13
base: main
Are you sure you want to change the base?
Atk technical #13
Conversation
ToddLGarrison
commented
Mar 1, 2024
- Create TrendingList component to render list of trending items from getTrending api.
- Implement TrendingList component on App.tsx
- Add try/catch block to getTrending fetch, allow for handing of successful and unsuccessuf/error/empty response
- Add try/catch to useEffect in App.tsx to provide clear error messaging fetchTrending function
- Resolved form input (handleNameUpdate) not updating.
- Implement useMemo() on ContentContainer.tsx to resolve performance issue due to numOfFactors() function. Required removal of () on line19. Function is called within numOfFactors on line 14.
setTrendingRecipes(trending) | ||
setErrorMessage(null) | ||
} catch (error) { | ||
console.error('Error in fetching trending data: ', error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A potential error message on this console could be "Error in fetching trending data: Error in fetching trending data: ...status" since you have the error message format set up already in the api function try catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just one small thing that is very nit-picky, nice job!