@SvetlanaStoycheva
Submitted
Looking to hire developers?
@b-a-merritt
@SvetlanaStoycheva
Submitted
@b-a-merritt
Posted
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
@Julr09
Submitted
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.
@b-a-merritt
Posted
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!
@Andreeesc
Submitted
I can't get the region filter to work properly, can someone help me find the solution?
@b-a-merritt
Posted
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
@NomiDomi
Submitted
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.
@b-a-merritt
Posted
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
@marialena31
Submitted
@b-a-merritt
Posted
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
@Lourdes84
Submitted
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!
@b-a-merritt
Posted
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
@Mozzarella-chz
Submitted
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?
@b-a-merritt
Posted
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:
span
elementsh1
selectorscard
classcontainer
classI'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