Amr mohamed
@AmrAbdelgwaadAll comments
- @ELMudyrSubmitted 8 months ago@AmrAbdelgwaadPosted 8 months ago
Hey, ELMudyr this should not be your first challenge you miss a lot of foundational things. I recommend using this to get started and I don't recommend using tailwind at this point, get a solid CSS foundation and then learn it.
0 - @lahanhelithSubmitted 9 months ago
Hey there Frontend Community!
I finished another solution today but I'm not truly proud of it yet since it was a struggle to get the CSS to look like the design.
I mainly struggled with getting the cards positions to change according to the size of the screen which created layout issues in the solution.
After a while of brute forcing and adding margin values here and there i managed to make it look okay on screens but I'm not truly proud of that part. I did lookup things but almost none of the resources helped with my situation.
I believe I'm going through the tutorial hell phase since I really couldn't understand what went wrong so I plan on going for simpler projects and try improve my CSS fundamentals and come back to this project.
Any resources that might help are appreciated which also includes Feedback!
@AmrAbdelgwaadPosted 9 months agoHey Lahan
I did this project before and I did the mobile version only because it depends heavily on position: absolute: I've done a GURU challenge and the HTML and CSS of it were way easier than this challenge, you are already out of tutorial hell continue building projects and you will get better, I have a couple of comments for you in this project:
-
You shouldn't use IDs to style your elements click here to know more
-
Media query should be in ems or rems and do the mobile version first PX, EM or REM Media Queries?
keep it up and continue building projects cheersπ
Marked as helpful0 -
- @GGSWEngineerSubmitted about 1 year ago
Hello everyone!
Always love going back to build simple things to solidify fundamentals.
No specific questions, but feedback is always welcomed!
@AmrAbdelgwaadPosted about 1 year agoHey Gerardo, Congratulations on completing this challenge π
your solution looks fantastic in the design comparison but there are a few things to take care of :
- 100vw on the body is redundant to center that card with less code remove the width and height on body and main and set the min-height to 100vh on main to make your code look like this:
main { min-height: 100vh; display: flex; justify-content: center; align-items: center; }
color: var(--Grey, #7d889e);
you are using custom properties wrong know more about how to use it here- it will be beneficial if you use a modern CSS reset like this one
I hope I am helpful keep it up π
0 - @ChewingOnCodeSubmitted about 1 year agoWhat are you most proud of, and what would you do differently next time?
I'm most proud that I got it almost 100% exactly like the review without the Figma file
What challenges did you encounter, and how did you overcome them?None really
What specific areas of your project would you like help with?None really at this time
@AmrAbdelgwaadPosted about 1 year agoHey Joseph, Congratulations on completing your first project π
I like how you used the <main> tag in your first project keep using them however, there are a few things to take into consideration:
-
You have an extra closing tag in the HTML line 28, you should take care of these as they will cause you huge problems in larger projects.
-
<h2 class="card-title"> Improve your front-end skills by building projects </h2>
this should be <h1> Learn more -
It will be really useful if you use a modern CSS reset like this one.
-
Font sizes should be in ems or rems, not px.
-
It's better for performance to use the fonts in <link> in the head tag and don't use import.
-
I am wondering why you are using multiple CSS properties to define border-radius it's widely supported see this.
-
We use margins to add spaces between elements think of flex and grid as a bigger thing to define a layout
I hope I am helpful and happy coding π
Marked as helpful1 -
- @rodorregoSubmitted about 1 year ago
Personally, I found the project quite simple. This was a quick code that I made in a few hours, so the responsive section has some details to improve so that it can adapt to many more devices.
@AmrAbdelgwaadPosted about 1 year agoHey Romina, Congratulations on completing your first project π
your solution looks nice but there are a couple of things to take into consideration:
-
you shouldn't use divs and use landmarks, wrap your page with <main> tag and the credits should be outside the main tag learn more.
-
<h2> Scan the QR code to visit Frontend Mentor and take your coding skills to the next level </h2>
This should be a <p> element not a heading. -
font sizes should be in ems or rems, not px
-
it will be really useful if you use a modern CSS reset like this one
-
to center a div in the middle of the screen you can use this on its container
.container { min-height: 100vh; /*using grid*/ display: grid; place-items: center; /*using flex*/ display: flex; justify-content: center; align-items: center; }
-
you should avoid using these units vb,vh until it's necessary
-
this project doesn't require media query at all
I hope I don't sound discouraging and happy coding π
Marked as helpful1 -