Alaa nasher w3 react #27
Conversation
Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 5.2.11 to 5.4.6. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/v5.4.6/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v5.4.6/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [braces](https://github.com/micromatch/braces) from 3.0.2 to 3.0.3. - [Changelog](https://github.com/micromatch/braces/blob/master/CHANGELOG.md) - [Commits](micromatch/braces@3.0.2...3.0.3) --- updated-dependencies: - dependency-name: braces dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
crevulus
left a comment
crevulus
left a comment
There was a problem hiding this comment.
Hi Alaa,
This is a lovely app, well done! Please check my comments and fix them as soon as you can. Also have you viewed your app in mobile view? The layout doesn't quite work - especially the favourite button. Good mobile UI/UX is a core part of frontend development so be sure to pay close attention to it.
|
|
||
| const runFetch = useCallback( | ||
| async (customUrl) => { | ||
| const finalUrl = customUrl ?? url; |
There was a problem hiding this comment.
The way you use your hook is a little overcomplicated.
You use your refetch function in App.jsx, and listen for changes in your useEffect. But think about how state works in React. You update your productsUrl which triggers another execution of this useFetch already.
| import { fetchApi } from "../utils/productsApi"; | ||
|
|
||
| export const useFetch = (url, options = {}) => { | ||
| const { immediate = true } = options; |
There was a problem hiding this comment.
Do you ever use this in your application code?
| const [isLoading, setIsLoading] = useState(true); | ||
| const [error, setError] = useState(null); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
You should be using your new useFetch hook to do all fetching logic.
| setError(null); | ||
|
|
||
| try { | ||
| if (Array.isArray(finalUrl)) { |
| if (isLoading) return <div>Loading...!</div>; | ||
| if (error) return <div>Something went wrong.</div>; | ||
|
|
||
| if (!favourites || favourites.length === 0) |
There was a problem hiding this comment.
This could be one if statement instead of two, seeing as it returns the same result (rendering your h2 message.)
3 participants
https://6967ff38e757df3829507863--alaa-week3.netlify.app