Godsfavour03• 20
@Godsfavour03
Submitted
Izzat Razab
@izzatrazab
All comments
- Izzat Razab• 70
@izzatrazab
Posted
Nice work on completing the challenge. 👍
I have some suggestions on your code that may help you.
- Set height of html and body to 100%, why ? this will make centering any contents much easier and will avoid extra white space at the bottom of the page or when zooming (though this does not happens in your case). To do that:
html{ height: 100%;✅ } body { min-height: 100%;✅ background-color: hsl(30, 38%, 92%); }- To center the card: Instead of using margin which is not responsive and akward, I would suggest using
display:flex;. Now that we already set height of html and body element to 100% before, there will be no awkward slightly offset position because they are covering you viewport 100% (in desktop view, your card is a little bit off to the top):
body { min-height: 100%; display: flex;✅ justify-content: center;✅ align-items: center;✅ background-color: hsl(30, 38%, 92%); } .pricecard { width: 90%; /* margin: 2em auto 2em auto; */❌ background-color: hsl(0, 0%, 100%); border-radius: 20px; max-width: 600px; } @media (min-width:600px) { .pricecard { display: flex; max-width: 500px; /* margin-top: 10em; */❌ }- Landmark main element: I see that you did not use main element. This is actually bad practice as using landmark helps screen reader users easily navigate your website. But why? Look here briefly: HTML5 landmark elements are used to improve navigation. Try evaluate your deployed website here. Though you don't have to stress out too much on this because it will make sense to you the more you practice. I would do this instead:
<main class="pricecard">✅ <div class="pricecard">❌ ... </div>❌ </main>✅I downloaded your code and tried all the above locally and it works fine. If you found this helpfull, please upvote this comment as it will help me improve. That's all from me, adios.
0 - KEYAJANI• 20
@KEYAJANI
Submitted
Hey, My name is Karrar Esbeyajani, and I'm very grateful to you guys, I'm new at programming I finished HTML&CSS two days ago, I didn't find it difficult to do the project it was fun
Thanks, a lot. Karrar E.
Izzat Razab• 70@izzatrazab
Posted
Hi, firstly I am not really qualify to comment people's work as I am a beginner myself. But I will try and give suggestion that may benefits you.
Your user interface in general is good but there are two things that can be fix:
- It seems that your desktop view has lots of extra space below the product card. If you can remove it, it will better.
- In your mobile view, your product card is not centered properly, it is slightly to the left.
I want to read your codes but it seems that I can't access it, therefore I can't comment or try to fix on them. It says page not found.
0 - Monsij Bayan• 70
@monsij-07
Submitted
Izzat Razab• 70@izzatrazab
Posted
Hi, firstly I am not really qualified to comment on people's work as I am a beginner myself. But I will try to give suggestions that may benefit you. My comments requres images, therefore I included a google doc link here and my comments are in there.
Please reply if you I was wrong in any way. I hope this solution helps you.
Marked as helpful
0 - Ikapocc• 170
@Ikapocc
Submitted
I struggle a bit with the IMG, there are some parts that I'm not too convinced about, like the use of the border radius. I know what the problem is, it's the use of the min (width), but I wanted to try this function. so if there is another way to solve this, any feedback would be nice.
Izzat Razab• 70@izzatrazab
Posted
Hi, firstly I am not really qualified to comment on people's work as I am a beginner myself. But I will try to give suggestions that may benefit you. I have read your code carefully but if I am still wrong in any way, do reply here. My comments require images, therefore I use google doc instead:
1 - Kuzitaa• 130
@Kuzitaa
Submitted
Hello everyone!
This is my first time using tailwind.
I have seen that the node_modules folder should not go to the github repository, however it is my first time trying something beyond basic html, css and js, and I was not able to configure it.
I appreciate all kinds of comments!
Izzat Razab• 70@izzatrazab
Posted
Hi regarding the node_modules, you can do the following
- Add "node_modules" in .gitignore file.
- Delete node_modules file
- Commit changes (to remove node_modules file in repo)
- Run "npm i" again in your workspace (to reinstall all dependecies)
Next time you commit, it will ignore node_modules file
Marked as helpful
1 - James Helms• 120
@jhelms82
Submitted
Izzat Razab• 70@izzatrazab
Posted
Hi, firstly I am not really qualify to comment people's work as I am a beginner myself. Plus, your portfolio page prove that you already have a lot of experience. But I will try and give suggestion that may benefits you.
-
Colour Scheme: I can see that you did not follow the color scheme suggested in the challenge. But it doesn't really matter much. What matters is the contrast (especially the fonts) of the content you want to show to the users. It's hard to read your page. One trick to realize how good or bad your contrast is to read your page while blinking your eyes fast (Basically you restrict your eyes from focusing). You will find it is hard to read your page properly while blinking fast.
-
Gradient: you're not using any gradient suggested in the solution. Gradient is really hard to do manually. Thus, use gradient generator. My favourite would be: https://cssgradient.io/.
-
h element: I can see that you're using h4 for "Your Result" and "Summary". I bet you use h4 because of its size right? Actually this is not good practice for SEO (but it is not a major flaw, so do not worry too much). Watch this: https://www.youtube.com/watch?v=cOmehxAU_4s at 6:37 web elements
-
CSS: I can't really comment on your css because you also use bootstrap, I am not that experience in using bootstrap. But do use flexbox, it will make your life easier.
That is all that I got for you. Hope these helps.
1 -
