Skip to content
OverviewSolutions
7
Comments
7

Ben Andrew Merritt

@b-a-merritt345 points

Aspiring Front-End Developer Catholic Volunteer and Missionary Iraq War Veteran

Latest solutions

View more solutions

Latest comments

  • Ben Andrew Merritt•345
    @b-a-merritt

    Hey!

    Looks good! I wanted to point out that at the 1440px-width breakpoint, the CSS gets glitchy because of the ruleset in :root. Additionally about the CSS, I think it would look cleaner if you didn't specify the flag's height so that some of the figures aren't larger than the others.

    Also, I'm not on the best internet and this app has a large fetch. Perhaps consider adding some of that data in local storage to speed it up on subsequent loads?

    Marked as helpful
  • Ben Andrew Merritt•345
    @b-a-merritt

    Hey!

    So one thing I saw was that you are using background-size: cover for all the queries. What might help you getting the background image to be perfectly sized is to use a different value. It's syntax is

    background-size: [WIDTH-VALUE] [HEIGHT-VALUE]

    Those values can be %, px, ems, etc. I think % are easiest. Since you are using two backgrounds, the svg and the color, it would be best to use

    background-size: [WIDTH-VALUE] [HEIGHT-VALUE], cover

    because the value after the comma refers to your second background. Without doing the project, I cannot tell you what values would be best. I think you'll find it quite quickly.

    Good job and Happy Coding!

  • Ben Andrew Merritt•345
    @b-a-merritt

    Hey!

    The filter feature is a little tricky. The reason why yours is having issues is because it's not rendering the results after selection. You need a useEffect() hook that induces a re-render with your query state in the dependency array.

    Also, I think it would be better if you saved the country data into a variable, then filtered the results into another variable that you then passed to your Components, instead of fetching the data with each new query.

    Anyway, good job and happy coding!

    Marked as helpful
  • Ben Andrew Merritt•345
    @b-a-merritt

    Hey!

    I think your intuition to use flex-box is spot on, but it of course cannot be used with a position: absolute item. (An absolute-positioned item is removed from the flow, and flex-box items need to be in the flow).

    One way you could accomplish this is to put your main-container div in another container, position that container absolute with a set height of 100vh and a width of 100vw, then use flex-box on the main-container inside of the new container.

    Whew! That seems to me like too much work.

    What might be better is if you used the "background-image" CSS property on main rather than displaying an HTML <img/> element. This way, you could just immediately set main to 100vh and 100vw while positioning main-container with flex-box right away.

    I think you did not do this because you wanted to display two different images - one for mobile and one for desktop. Sorry for explaining this if you already knew it, but you can use a media query to display a different image at a different screen with by using the following code in your stylesheet:

    
main {
  background-image: url('RELATIVE_PATH_FOR_MOBILE_HERE');
}

@media only screen and (min-width: WIDTH_AT_WHICH_YOU_WANT_TO_CHANGE_HERE) {
  background-image: url('RELATIVE_PATH_FOR_DESKTOP_HERE');
}

    If that wasn't the reason or that's not helpful, I'd be interested to know!

    Happy coding!

    Marked as helpful
  • Ben Andrew Merritt•345
    @b-a-merritt

    Hey! Good job completing this one! I just finished it too. One big bug I had (and you have too) is that certain countries do not have some properties we pass as props that end up being undefined. Try clicking on Antarctica, for example, which does not have a capital (and presumably other props as well).

    It seems to me the best solution is to add defaultProps https://reactjs.org/docs/typechecking-with-proptypes.html#default-prop-values

    Happy coding!

    Marked as helpful
  • Ben Andrew Merritt•345
    @b-a-merritt

    One small thing: clicking on the button to change the theme doesn't work. I had to dive into dev tools to figure out that I had to click on the numbers. This was backwards for me in terms of UI / UX.

    If you agree, moving your buttons to where switch-button div lives and making their ::before contents equal to the numbers (essentially swapping containers) would solve this.

    If not, at least at a hover state to the numbers so people like me might figure it out more intuitively.

    Either way, I'm a huge fan of how clean your JavaScript is written and commented!

    Marked as helpful
View more comments