@j0sephh123
Posted
- When you need to use logic in react components, but don't need to return JSX, as is in src/components/ScrollToTop/ScrollToTop.jsx - create a custom hook
- Haven't used
unwrap
forcreateAsyncThunk
, so thanks for introducing that to me :) src/routes/app/App.jsx
useEffect
seems to be missing dependencies. I suggest that you setup eslint and the hooks plugin - https://www.npmjs.com/package/eslint-plugin-react-hooks It is pain to setup, but will warn you for missing or incorrect dependencies for useEffect, useMemo, useCallback and so on.- Regarding your struggle with styles - you can look into SCSS modules, Tailwind or https://stitches.dev/. Really depends on your style. Personally I'm using SCSS modules at work and working on a project here with stitches and it is very nice - very convenient for a global theme. A bit hard to get into.
src/components/Header/Header.jsx
- you can create enum for themes, instead of using raw stringssrc/components/InputElement/InputElement.jsx
-'await' has no effect on the type of this expression
this is for setTimeout. Also, you can use the shorthand syntax for this:onChange={handleInput}
Also shorthand for this:{countryName && <div className="close" onClick={handleErase}></div>}
It is good that you have 1sec debounce after typing to avoid sending fast requests. You can also use https://usehooks.com/useDebounce/ or extract your own logic into a custom hook for reusability. (not that it is needed here)src/features/regions/Regions.jsx
-hidden={countryRegion === '' ? true : false}
can be simplyhidden={countryRegion === ''}
- same for
selected={countryRegion === region ? true : false}
- what is
firstRender
used for? It doesn't seem to do anything, you just set it in the useEffect. src/features/countries/Countries.jsx
InhandleScroll
avoid lenghty statements inside if. Create a variable and use it inside if. I will try to provide an idea. Note that there is a high probability that this syntax may not be working outside of the box, it is just to illustrate what I have in mind:
// Countries
const Countries = () => {
...
const isLoading = useSelector(selectCountriesIsLoading);
useHandleScroll(isLoading, () =>
dispatch(
loadCountries({ countryName: null, countryRegion: null, scroll: true })
)
);
...
And a new hook file
//src/features/countries/useHandleScroll.js
export default function useHandleScroll(isLoading, callback) {
const handleScroll = () => {
const ourVariableName = window.innerHeight + Math.ceil(window.pageYOffset) >= document.body.offsetHeight && !isLoading;
if (ourVariableName) {
callback();
}
};
useEffect(() => {
window.addEventListener("scroll", handleScroll);
return () => window.removeEventListener("scroll", handleScroll);
}, [isLoading]);
}
Marked as helpful
@Daniil034
Posted
@j0sephh123 Oh my god, I haven't thought anyone would actually go through my code. I'm ashamed ((( Yeah, it's bad.
src/routes/app/App.jsx
- it doesn't miss dependencies, I just forgot to discard those braces);
firstRender
is needed because without it the page would reload after 1 second on first load and after every return on main page from country page)
All other suggestions noted. Really, you taught me a lot)
Especially thank you for introducing custom hooks because I was going to use them in my next project along with all other thing I have in mind (as well as tool for working with css, maybe it will be scss as you said).
You are awesome, dude)
@j0sephh123
Posted
@Daniil034 There is nothing to be ashamed of. You have created something that is looking good and working. Of course, there are things that could be better, but with time and experience, you will refine these things.