Melad• 110
@meladcodes
Submitted
Any feedback will be appreciated!
Jennifer
@Jennifer1919
All comments
- Jennifer• 220
@Jennifer1919
Posted
Hi! First of all good job! I'll suggest you just change up the style of the heading so that it matches the template.Look at the font weight maybe. I don't know if you are into javascript but maybe consider adding an alert message if someone types in an invalid email adress
Marked as helpful
0 - Ayush Verma• 80
@ayushv45
Submitted
I have used div for stats part(10k+ companies ......) can anyone give me a good alternative code. More Suggestions to improve are most welcome.
Jennifer• 220@Jennifer1919
Posted
Hi! I just completed this challenge and I used a ul list of the stats part. I just got rid of the list style with list-style:none. To display it as a table for larger devices I use display:flex and flex-direction: rows. Hope it helps! PS: can you explain you got the purple overlay?
Marked as helpful
0 @CarlosZaragozaBeato
Submitted
Jennifer• 220@Jennifer1919
Posted
Hi! Here is an advice if you want to get rid of the 2nd accessibility issue. Use HTML tag like <main> instead of <div class="card"> or <section> instead of <div class="card-body">. It is more meaning full for the browser. Good job btw!
Marked as helpful
1- Melad• 110
@meladcodes
Submitted
I feel like I wrote too much CSS for this component, any feedback will be appreciated. Thank you!
Jennifer• 220@Jennifer1919
Posted
Hi! Here is an advice if you want to reduce the amount of line in you css file:
- instead of writing border-top-right-radius: 10px; border-top-left-radius: 10px; you can just write border-radius: 10px 10px 0px 0px; and it will do the same thing
- you can do the same thing for margin-right: 30px; margin-left: 8px; just set it to margin: 0px 30px 0px 8px;
here are some links to help you: https://www.w3schools.com/css/css_margin.asp https://www.w3schools.com/css/css_border_width.asp
Marked as helpful
1
