Skip to content
OverviewSolutions
7
Comments
7
Ben Andrew Merritt
@b-a-merritt

All comments

  • Ben Andrew Merritt•345
    @b-a-merrittPosted almost 3 years ago

    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
    0
  • Julian•280
    @Julr09Submitted almost 3 years ago

    Right now i'm having trouble with the background images, can't make them exactly like the design provided, so any advice or resource regarding to that or anything else would be appreciate. Thank you.

    Huddle landing page

    1
    Ben Andrew Merritt•345
    @b-a-merrittPosted almost 3 years ago

    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!

    1
  • Ben Andrew Merritt•345
    @b-a-merrittPosted almost 3 years ago

    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
    0
  • Noemi Gali•280
    @NomiDomiSubmitted almost 3 years ago

    As always, would greatly appreciate any feedback.

    Also, how do you guys center vertically when you have .main-container { position: absolute }

    I was able to center horizontally because I had a set width for the container, but my height was auto.

    Tried display flex and align-items but it didn't work.

    Responsive Order Summary Component

    2
    Ben Andrew Merritt•345
    @b-a-merrittPosted almost 3 years ago

    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
    0
  • Ben Andrew Merritt•345
    @b-a-merrittPosted almost 3 years ago

    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
    0
  • Lourdes González•150
    @Lourdes84Submitted almost 3 years ago

    Hello World! Here is my Calculator app challenge, if you like it give me like and any improvement advice will be well received! Thank you so much!

    Calculator app

    2
    Ben Andrew Merritt•345
    @b-a-merrittPosted almost 3 years ago

    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
    0
  • Rachel Mozzetta•135
    @Mozzarella-chzSubmitted about 3 years ago

    This code could certainly be cleaner. Is it best practice to be extremely (what feels like over the top) specific when writing in CSS ALL the class selectors? For example

    .container .card .main-paragraph { color: var(--stat-white); margin: 20px 50px 20px 50px; line-height: 1.7; align-items: center; }

    within in my code there is only one .main-paragraph so do i really need to precede it with .container .card?

    HTML, CSS

    1
    Ben Andrew Merritt•345
    @b-a-merrittPosted about 3 years ago

    Hey Rachel!

    I'll answer your question broadly, because I think there are circumstances when it would be necessary to use that many selectors. I think a deeper understanding of how CSS is parsed would benefit you.

    Browsers construct a CSS Object Model (like the DOM) starting with the selectors on the right and then working left. Single selectors will be nearest the root while more specific selectors will be further down the branches. For example, on line 77 of your stylesheet, the selector .container .card h1 span is parsed like this:

    1. The browser finds all span elements
    2. Of those, the browser creates a branch that also have h1 selectors
    3. Of those, the browser creates another branch for those that have the card class
    4. Finally, another branch is made for those that have the container class

    I'm sure you've guessed it by now -- the best practice is to limit how many times the browser has to do this. When using selectors, use as many as are needed to grab just that one (or those ones). In your project, you have only one span on the entire page. You could have just said

        span {
      --var(S-violet)
    }

    If chaining selectors, consider having the more specific on the right and the least specific on the left. That way, the browser doesn't have to gather up all the <p> elements before finding the one #identifier.

    Note: Don't look at my work for an example on how best to do this! I'm the epitome of hypocrisy.

    Additionally, you used a div with the class container right below main. Perhaps consider just using main? Semantic HTML elements like main or section can be selected just as easily, if not easier. You could delete that div and change all CSS selectors to main instead.

    Anyway, the result looks good! Good job

    Marked as helpful
    2