@ApplePieGiraffe
Posted
Hi, Hyeon Park! 👋
Good work on this challenge! 👍 Your solution looks pretty great! 👏
A few suggestions I have are,
- In the mobile view, setting the width of the card component to
100%and then adding a max-width to it to prevent it from becoming too wide when there's extra space around it. This is often better than setting the width of the element to a fixed value or percentage because it'll ensure that the element only takes up as much space as it should be allowed but also shrinks accordingly when the space around it shrinks. Currently, things get a little wonky when the width of the screen gets really, really small (which isn't a huge issue in this case, but this pattern can help you in future projects). - Making sure that the social media popup can be seen in the mobile view. Currently, it is only displayed in the desktop view.
- Making the social media icons links, not buttons (since when clicked, they should take the user to their respective social media site).
Hope you find this helpful. 😊
Keep coding (and happy coding, too)! 😁
@hkparkjs
Posted
@ApplePieGiraffe
I fixed mobile view issue and made social media icon to links after I saw your comment.
Thank you for your detailed advice! :) but I didn't understand your first comment, so Could you explain the comment more detail (example)?
@ApplePieGiraffe
Posted
@hkparkjs
Hey!
I just mean that instead of setting specific width for the card component like this,
.card {
width: 500px;
}
...you should set the width to be a percentage and then add a max-width to prevent the card component from becoming too wide, like this,
.card {
width: 100%;
max-width: 500px
}
That way, you should you notice that when you decrease the size of the screen, the width of the card decreases when necessary (instead of remaining a fixed width and causing horizontal scroll bars to appear).
Hope that makes sense!
@ApplePieGiraffe
Posted
Btw, I'd also suggest avoiding using px for setting the value of width in your styles. Instead, use a responsive unit such as em or rem so that users will be able to change the size of things in your site by changing the default font-size of their browser. It might also be worth setting the values for other properties such as margin or padding in those units so that your entire site will scale with the user's chosen default font-size. If you'd like to learn more about those units in CSS and how all of this works, check out this helpful video on the topic.
@hkparkjs
Posted
@ApplePieGiraffe
I understood your comment and it's very useful for me! I will use the way as you commented. Thank you very much :)
