@Yazdun
Posted
Hello Lucy and well done on the challenge, Here are my suggestions :
-
Use Prettier extension on your IDE to format your code.
-
use code below instead of specifying default value for html element separately
*,
*::before,
*::after {
margin: 0;
padding: 0;
box-sizing: border-box;
}
- Html elements are block level by default ( thats one of the reason for approaching mobile first ), so you don't need š
display: flex;
justify-content: center;
align-items: center;
flex-direction: column;
on your main
tag, you get the column by default
-
give
min-height:100vh
instead ofheight:100%
to your body. -
right now, background circles are not responsive and layout breaks quite easily on smaller screens, so here is what I did on this challenge for background circle to make sure of their responsiveness. firstly add š
.c1 {
right: 50%;
bottom: 43%;
}
.c2 {
top: 50%;
left: 50%;
}
.c1,
.c2 {
position: fixed;
transition: 0.5s cubic-bezier(0.075, 0.82, 0.165, 0.6);
z-index: -1;
}
to your css and then add images to the markup š
<img
src="./assets/images/bg-pattern-top.svg"
alt=""
role="presentation"
class="c1"
/>
<img
src="./assets/images/bg-pattern-bottom.svg"
role="presentation"
alt=""
class="c2"
/>
and then remove background properties from body
. this way, background circles won't go bananas on smaller screens. keep in mind that's my approach and there are definitely better approaches you may come up with.
-
use
width:100%
and thenmax-width: ... px
instead of giving a static width to your card, so card's layout won't break on small devices, right now your card leaves side scroll on small screens. -
I don't recommend using static heights for elements unless it's really crucial.
-
for stats, I think you'll be better off with something like
<ul class="card-stats">
<li><span>80k</span> <span>Followers</span></li>
<li><span>803k</span> <span>Likes</span></li>
<li><span>1.4k</span> <span>Photos</span></li>
</ul>
instead of using so many divs.
- give some paddings to
body
so content won't occupy whole screen on mobile devices.
ā Also I opened a pull request to your repository which will fix these issues
I hope this was helpful
Marked as helpful
@lmac-1
Posted
@Yazdun Hi Yazdun, thank you so much for this feedback, it was SO useful and I learnt a lot! I'm so sorry but I've only had time to finish going through it all properly today.
I haven't accepted your pull request because I wanted to combine your advice with Mike's. I just made some changes this week on GitHub and hopefully, you can see an improvement :) open to any further feedback!
Thanks again!
https://github.com/lmac-1/profile-card-component