@kaamiik
Posted
Hi. Congrats for doing this challenge. I noticed some points I wanna mention here:
-
For each project Use only one repo on GitHub. I see lots of folder in your repo that is wrong I think.
-
You can check most of the pages and see they have a structure like this:
<body>
<header>...<header>
<main>...<main>
<footer>...<footer>
</body>
Based on your design you may have header
and footer
or not. But You should
have a main
tag inside your page. So after your body always wrap all of your code
inside a main
tag. In this design you do not need header
and footer
.
-
Elements that have hover effect are interactive. So because you have hover effects for your
h1
then It needs to bea
orbutton
. Now you have too choose betweena
andbutton
. If the element take you to a new page It should be ana
tag and If do an action like submit a form or add to cart then It should be abutton
. In your challenge you haveh1
and inside theh1
you have to wrap it into a interactive element too. -
Heading levels should increase one by one. Here in your code after
h2
should beh3
. -
You do not need to have
<br />
tag in yourp
tag. It's more clean and robust to let the browser based on screen size, container max-width, font-size and other factors wrap the text or not. You only needmax-width
for your container because it prevents elements from stretching beyond a certain point, keeping them visually appealing across different screen sizes. It ensures your design remains adaptive and doesn't get too wide on larger screens. -
Use
min-height: 100vh;
instead ofheight:100vh;
.height: 100vh
strictly limits the height to the viewport size, potentially causing overflow issues if the content is larger than the viewport. On the other hand,min-height: 100vh
allows your element to grow in height if the content exceeds the viewport size. -
Try to use a proper CSS reset at the start of your CSS style. Andy Bell and Josh Comeau both have a good one. You can simply search on the internet to find them.
-
Your
font-size
andmax-width
should be inrem
unit notpx
. You can read this article about it and why you should not usepx
as a font-size.
Marked as helpful
@mahenajT
Posted
Hi @kaamiik,
Thank you so much for your thoughtful feedback! I truly appreciate the time you took to highlight areas for improvement.
I will streamline my GitHub repository to use only one for each project and ensure that my code is wrapped in a <main> tag. I now understand that the h1 tag should be an interactive element, and I will be more mindful of maintaining proper heading levels.
Additionally, I will avoid using <br /> tags, switch to min-height: 100vh;
, and use rem units for font sizes. I will also consider adding a CSS reset.
Thanks again for all your tips! I’m excited to apply them in my upcoming projects!
@kaamiik
Posted
Your welcome @mahenajT I strongly suggest that first refactor this project and test it and after you did this completely then go for the next project. Refactoring your project is a key factor in each challenge you start and finish.
Marked as helpful
@mahenajT
Posted
Thank you, @kaamiik! I appreciate your advice. You’re right—refactoring is essential. I will focus on refining this project before moving on. Thanks again for your encouragement and guidance!