Shubhankar Sengupta• 285
@Shubhankar-Sengupta
Submitted
Kindly provide feedback.
Harsh Jain
@thisisharshjain
All comments
- Harsh Jain• 475
@thisisharshjain
Posted
Hello Shubhankar-Sengupta, Nice work here. I'll quickly point out few issues in your solution:
- There is too much space between avatar image and their title, On screen width between 770-990 it looks little wierd
- You forgot to put bg color
- Giving top and bottom marigin on main container will look better
- You can get rid of those HTML validation error by using forward slash (/) in image path Except these small issues i think everythings looks good. great work keep grinding 😄💪🏻
Marked as helpful
0 - Amr Fouad• 90
@A-Fouad-Code
Submitted
Please check out my
codeand if you find anything that you would've done differently share your ideas with me..Thanks in advance.
Harsh Jain• 475@thisisharshjain
Posted
Heyy A-Fouad-Code, Good work on this one! Also, Explaining your approach on readme is a very good practice 😉 So, 1. Moible design looks little congested, Giving more whitespace would look better 2. On wide screens there's too much empty space, You can give max-width and height for them. Everything else looks fine. Great work keep grinding 😄💪🏻
Marked as helpful
0 - Ivelin Mitev• 140
@ivelinsm
Submitted
Hi!
Do you know any good tutorials on mobile-first design? I have never done that and I would like to try.
Because, as you can see, my code isn't very "responsive" and I used some pretty dumb ways to make it look a little bit better.
Harsh Jain• 475@thisisharshjain
Posted
Heyy ivelinsm, That's a pretty neat solution. There's a whole playlist on mobile-first build by Net ninja on youtube. Check that out! Also, a few tips to make your solution look better on mobile devices:
- You can decrease padding of #content so content inside have more space
- You can decrease overall font-size (like html { font-size: 50%; } ) Nice work buddy, keep grinding 💪🏻
Marked as helpful
1 - Pop Andrei• 120
@pccipri
Submitted
Hey there! If u have any advice regarding image positioning or if u think I could've done something different let me know.
Harsh Jain• 475@thisisharshjain
Posted
Heyyy pccipru, Nice work man! Making this in vanilla HTML CSS is another challenge and you did great. I found a few small issues:
- If i click all accordions then top and bottom one's are cut (You can either increase height of container accordingly or maybe wrap all accordions inside a div and set overflow-y to auto)
- Below 1200px width the image on left doesn't stays in center
- On mobile screens, give .text-container a little padding. That will look little better Everything else looks good to me 😄 Keep grinding 💪🏻
Marked as helpful
1 - Amr Fouad• 90
@A-Fouad-Code
Submitted
If you have any comments or ideas on how I can make my code better or the design look better please share your thoughts with me.
Harsh Jain• 475@thisisharshjain
Posted
Heyy @A-Fouad-Code, I'll point out a few things here,
- Main card isn't centered instead on top-left side
- Color overlay on image doesn't looks that good. maybe change the opacity a little
- Try to use semantic elements when possible
- Reducing the padding-top on .stats will look better
- It kinda breaks between 770-1199px Except these few small issues, You've done a good job! 😄 Kepp grinding 💪🏻
0 - AhmedAbdullah11• 40
@AhmedAbdullah11
Submitted
I need any review or notes to make my self better
Harsh Jain• 475@thisisharshjain
Posted
Heyy @AhmedAbdullah11, Congrats on completing your first challenge! 😄 Your card sticks to sides from 380-450px, decrease the max-width of your card and padding. Other than that everything else looks great! Keep grinding 💪🏻
0
