Country Api by dev

Community feedback

• 

  Anton•515

  @antaryaPosted about 2 years ago

  Hi 👋,

  Great job, it looks really good.

  I have a couple of suggestions/improvements, though:

  HTML/CSS

  [1. Country flag images look distorted on the main and the details page. On the main page, you can remove height so it can grow based on width. And the details page, instead of specifying height, you can remove the height attribute and limit max-width to a value of 400px, for example.

  [2. The class names are a bit long; it will be hard to work with them in the future. As you are already using BEM (http://getbem.com/naming/), instead of

  • country-details__content__details__information you can have country-details__info
  • country-details__content__details__information__container-1 country-details__info-main
  • country-details__content__details__information__container-2 - country-details__info-secondary

  [3. As far as I understood, the border country elements on details pages should be a link to a country itself.

  JS/REACT

  [4. For HTML Validations error that can be seen in the report, I think all is related to illegal characters in href, which you can fix by wrapping URL params in encodeURI. (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURI, https://stackoverflow.com/questions/332872/encode-url-in-javascript)

  [5. For numbers to be user-friendly, format them so that instead of 35000322, you will have 35,000,322. (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat)

  [6. When you search for a country, and the result is empty, it shows the last result. Should not it be an empty list?

  [7. It is better to be consistent and use one format for variable names. For example, you have filterref, searchRef, and country_results. (https://javascript.info/variables#variable-naming)

  [8. Using constant for API URL or helper method will allow to change it quickly, and it will not be hardcoded in multiple places.

  const API_URL = "https://restcountries.com/v2/";
  
  fetch(`${API_URL}/name/${param.countryname}?fullText=true`);
  

  [9. The value of mode is passed to each component that needs it. There is a term for it prop drilling, which is okay in some cases, but as this value is shared across all application components, you can use Provider Pattern (https://www.patterns.dev/posts/provider-pattern/) instead, which is using Context (https://reactjs.org/docs/context.html). Using this pattern will remove unnecessary duplication and be more maintainable. styled-components has a helper component to do precisely that - https://styled-components.com/docs/advanced#theming.

  [10. Regarding variable name mode - it is hard to understand the meaning without reading the code; instead, you can name it isDark | isLight | theme if it has 'dark', 'light' values.

  [11. In MainContent, you are using ref for input. Why not directly use useState (https://www.w3schools.com/react/react_forms.asp)?

  [12. For the select field, you will benefit from using libraries like https://react-select.com/home as it is flexible and follows best practices.

  [13. You can use useMemo to avoid unnecessary processing/calculations and be more performant. It will only recalculate the value of filteredCountries on filter or country_results change. Example:

    // https://reactjs.org/docs/hooks-reference.html#usememo
    const filteredCountries = useMemo(() => {
        if (filter) {
            const filterValue = filter.toLowerCase();
            /* You can also prepare lowercase region name on the cleaning stage, so it is done only once and not on each call of useMemo */
            return country_results.filter((value) => value.region.toLowerCase() === filterValue);
        }
        return country_results;
    }, [filter, country_results]);
    ...
    <Grid className="grid">
      {filteredCountries.length > 0 && filteredCountries.map((country, countryIndex) =>
          <GridCard result={country} key={countryIndex} mode={props.mode} />
      )}
    </Grid>
  

  [14. I noticed that data is fetched every time user navigates to country-api. What do you think of getting all data once and then reusing it?

  GENERAL

  [15. For some reason, when you reload the detail page, it shows a 404 error.

  [16. You can use prettier, which can format your code automatically on save, so code formatting is consistent. For example, you will not have two different formats:

  const CountryInDetails = ({ mode }) => {
  const NavBar=(props)=>{
  

  https://create-react-app.dev/docs/setting-up-your-editor/#formatting-code-automatically

  [17. For commit messages, it will help others if you add a descriptive message and follow a specific format, so all is consistent (https://www.conventionalcommits.org/en/v1.0.0/). You can even make a check before committing if you follow those rules by using a tool like commitlint - https://commitlint.js.org/#/

  Let me know if you have questions. I hope this will be helpful.

  Keep up the good work 🚀. Cheers!

  Marked as helpful

  0

  ---

  

  Debadutta Panda•300

  @debadutta98Posted about 2 years ago

  Thank you for your valuable suggestion @antarya

  0