Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Countries: React, redux-toolkit with RTK Query, change theme

Yuriy 155

@faizov

Desktop design screenshot for the REST Countries API with color theme switcher coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
  • API
4advanced
View challenge

Design comparison


SolutionDesign

Solution retrospective


A little practice with redux toolkit rtk query. Change theme, use localStorage, prefers colors.

Community feedback

j0sephh123 140

@j0sephh123

Posted

  • Why create-react-app ? It is perfectly fine, just curious.
  • .map<React.ReactNode> why typing that? It is inferred by TS compiler.
  • Noticed that in some components you are importing React, I don't think it is necessary

src/copmonents/pageLayout/index.tsx

  • you are defining the typed hooks from redux docs in here src/app/hooks.ts, but you are still using the original ones
  • for theme you have type string, here it'd be a good place to use enum or union type, since it can be only 2 values
  • onChangeTheme function is doing some logic inside the component dispatch(setTheme(theme !== "light" ? "light" : "dark")); I would suggest that you actually do that logic inside the reducer.

src/pages/home/index.tsx

  • dataAll and allLoading are not good names for variables.
  • byRegionLoading is unused variable
  • you could great benefit from es-lint, it gives me a few warning, such as not returning a value from filter function, React Hook useEffect has a missing dependency: 'closeOpenMenus'. Either include it or remove the dependency array.
  • you would benefit from creating components for Error and Loading.
  • there is a lot of repetition in result.map, you could do some destructuring there.
  • Since const data: CountryApi | undefined it would be good to handle the fetching in one component and only have a child component that receives data when we are certain it is only CountryApi. Thus, you can avoid checking on severa places if it is undefined.
  • probably can extract hideSelect && what will be here in a custom component, it would need to take only one prop handleChangeRegion, since the listSelect is a hard coded array.
  • I think that you can type region to a union type, currently it is a string. You can have all the possible values here: src/pages/home/config.ts

src/copmonents/card/index.tsx

  • capital && you are checking for capital truthiness here, but even if it is an empty array, it is still truthy, so there is no need for that check. A better approach would be to check its length.

src/@types/index.ts

  • you could add the type for capital also

src/pages/country/index.tsx

  • See here you are using again Loading and Error. You are repeating yourself - extract them as components
  • Same note for country here. Create a child component, check if country is NOT undefined, then pass it to the child component, thus you will avoid all the ? checks that you have for country.
  • why is CountriesCode CapitalCase?

Marked as helpful

2

Yuriy 155

@faizov

Posted

@j0sephh123 Thanks for the extensive advice! I'll try to make corrections soon.

I don't quite understand about the first question. What other options are there?) I want to learn react completely.

In case of .map<React.ReactNode> I get an error ts

0
j0sephh123 140

@j0sephh123

Posted

@faizov I suggest you look into https://nextjs.org/docs/getting-started

0
Yuriy 155

@faizov

Posted

@j0sephh123 Thanks!

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord