REST Countries API using React and CSS

Community feedback

• 

  Anton•515

  @antaryaPosted about 3 years ago

  Hello Davide,

  Great start with React 🚀.

  I like the idea of the load-more button that you added 👍. Couple ideas/suggestions/improvements for your implementation:

  Issues

  [1. If you search for a country that is not yet visible, the list will be empty. It is happening because there is a slice of data before filtering. Changing the order of slice and filter should do the trick.

  [2. When country borders is not defined single-country page raises an error. For starters, you can assign an empty array, so it is working.

    const {
      alpha2Code,
      ...
      // if borders is undefined it will set it to empty array
      borders = []
    } = country;
  

  Take a look at Destructuring search for "Default values" for an object.

  [3. The load more button stays on the page, even when filtered data has less data than the visibleCards amount. A simple condition will fix that.

  [4. If you reload the single-country page or navigate to it directly, there is an error in the console. That is caused because at this point country list is not loaded yet.

  React

  [5. Countries page is calculating countries variable every time it renders. The trigger of render might be something other than search/filter/visibleCard change. One of the ways to eliminate unnecessary calculation is to use useEffect({...setFilteredCountries(...filtered countries) }, [query, fitler, visibleCards]). And inside component return {filteredCountries.map((c) => <Card ... />)}.

  [6. As far as I can see, the API does not support pagination, but it supports fetching specific country/countries. For the single-country page, there is no need to know about all countries. You might load specific country data directly and after it border countries; even better before that, checking context if all countries are already loaded and if yes, get it from there instead. What I am saying is that fetching all countries on the single-country page is redundant.

  [7. Currently, a global context has data related to the theme, country list, query, and filter. If we check the usage of those, it would be: theme: whole application country list: countries page/maybe single country page query: only countries page filterData: only countries page

  It looks like only theme/maybe countries should be accessible by the whole app, and the rest can be moved to a specific page. If you keep adding new pages to your app, those pages will probably not need to fetch allCountries, and they do not need to have query/filterData. The simplest solution would be to move query/filterData state to Countries page, and trigger fetching all countries from Countries page but save it in context as you have it right now. And for the single-country page to check if allCountries is already loaded, use that if not - fetch single country data.

  [8. It would be nice to have a loading indicator while filtering/loading more and fetching single country data.

  [9. You might reset visibleCards to the initial value every time you use a query or filter.

  [10. On the Countries page, the same logic is executed for country filtering. You can combine those:

    if (item.region === filterData) {
      ...same
    }
    else if (filterData === "All") {
      ...same
    }
    // instead you can do
    if (filterData === 'All' || item.region === filterData) {
      ...same
    }
  }
  

  Also, you can improve it by checking query value. There is no need to use a query if it is an empty string.

  [11. main tag element and `back-to-top' are excellent candidates to move to separate component and reuse on all pages or move it to the app component. e.g.:

    const Layout = ({children}) => (
      <main className="main-content">{children}</main>
      <BackToTop />
    )
  

  [12. const [search] = useState(["name"]); - there is no need to have it as a state you can have it as a const out of the component. e.g.: const searchFields = ['name'];

  [13. There are both versions in code; I suggest sticking to the last one as you are not repeating yourself.

  <i className=  {darkMode ? "fa fa-search icon darkmode" : "fa fa-search icon"}></i>
  <i className={`fa fa-search icon${darkMode ? "darkmode" : ""}}></i>
  

  If you find yourself using a lot of conditions inside className, you may take a look at this libraries clsx, classnames

  JS

  [14. Instead of classList.add() and classList.remove() you can use classList.toggle("className", flag)

  [15. Things like the number of items per page are a good candidate to move to a constant and out of component to be reused, e.g. const COUNTRIES_PER_PAGE=49.

  [16. There is no need to call query.toLowerCase() each time you check if it is part of the country name. As query is not changing, set a variable before filtering and use it instead. It might be fast now, but there can be some heavy calculation involved in the called method in other applications.

  HTML

  [17. Currently, users need to guess that card is clickable unless the user hovers the image. If you make the whole card clickable, it might be better for the user experience.

  [18. The usage of semantic elements like footer and article is misleading. I would make a footer child of body and place content inside, and the article will contain all country info.

  [19. Instead of alt="Country flag route", you can change it to include country name so that the screen reader will read to which country user will be navigated.

  [20. Change theme switch label when changing theme for better user experience Light Mode/Dark Mode.

  [21. To write less instead of writing lists by hand:

    <option value="All">Filter by Region</option>
    <option value="Africa">Africa</option>
    <option value="Americas">America</option>
    ...
  
    // you can create an array of regions
    const regions = ['All', 'Africa', 'Americas' ...];
  
    // and later output
    {regions.map((region) => (<option key={region} value={region}>{region}</option>)}
  
    // cleaner code, easier to update, you can later use API to get all regions
  

  CSS

  [22. The issue with the select input arrow icon can be fixed by removing background-image: inherit; from .select-dark, and linear-gradient from:

    select,
    .select-dark {
      /* replace this */
      background-image: url('data:image/svg+xml;...'), linear-gradient(to bottom, #ffffff 0%,#fff 100%);
      /* with this */
      background-image: url('data:image/svg+xml;...');
    }
  

  That should do the trick. In general, it is hard to style select input. Design of select input is much more difficult most of the time, so you will be forced to use an external library. Here are examples of libraries that you can use instead: For react react-select For js select2

  [23. You are already using the theme class on the body tag instead of having theme-related classes on each element. You can reuse parts of element styles that are shared and apply only differences based on body theme class.

  .btn {
    /*
      all shared styles
      and one for the light theme
    */
  }
  .dark .btn {
    /*
      specific to dark theme
    */
  }
  

  By doing that you can remove code like <div className= {darkMode ? "country-info-dark" : "country-info"}> as you will just have <div className="country-info"> and body theme class will trigger theme specific overwrites.

  [24. There are minor differences when switching from light to dark:

  • Country flag image height is changing. It would also be nice to have a fixed height for the image, so it is consistent.
  • .country-detail > p font-weight is changing to lighter for some reason.

  Resources

  • To format code and be consistent, you can use Prettier. Also, you can add configuration so every time you commit, it formats it CRA-Formatting Code Automatically

  • Take a look at another style for className naming format BEM

  As I said only have a couple of suggestions 😬😅. I hope it is not overwhelming and will be helpful.

  Keep up the good work! Cheers!

  Marked as helpful

  2

  ---

  

  Davide•1,725

  @Da-vi-dePosted about 3 years ago

  @antarya Hi, Thank you times infinity to have taken the time to look at my code and write such a wonderful and useful feedback! I really do appreciate it :-) As soon as i can i want to try my best to make those improvements, one step at a time! I need to reread everything more times with calm and the code editor opened in order to not be overwhelming.

  Thank you so much again :-)

  0

  

  Anton•515

  @antaryaPosted about 3 years ago

  @Da-vi-de Happy to help. See you on the coding trail 😉

  0