@srikargunnam
Posted
Hi Alexandro Munera,
First I would like to congratulate you for completing this challenge 🎉
I would like to give few suggestions based on my observations.
-
It is must to use h1 tag (level one heading) as per web accessibility standards, you can swap h2 with h1, and my advice is to use a header tag instead of a div tag to wrap h1 and p tags.
-
Your card looks stretched, that is because you used width as 90%, so there is no max-width, Instead of using width as 90% use min(90%, "the max width the content fits good"), so that your content looks good even or much wider screens.
-
Breakpoint for mobile screens mentioned in the style guide is 375px, but used a breakpoint at 768px, it would be better if you can work on that, i can understand that the content looks like weird when it reached 768px, for this you could as a media query for tablet assuming it is given in this project, it will give you a new opportunity to work on your own ideas to make the content suit tablet view until it reach 375px.
-
I have seen that you have used rem units at some places and px units at some places, it is a good practice to stick to one type of units, i would suggest to go with rem entirely.
This link "https://www.sitepoint.com/understanding-and-using-rem-units-in-css" will help you get a better understanding of rem units, and how you can easily use a trick to make 10px = 1rem, this will help you easy to calculate and use rem units, but keep in mind that this trick will not work for media queries which is not mentioned in the above link.
hope this helps 😉, please mark as helpful if it does.
Marked as helpful
@AlexandroMunera
Posted
@srikargunnam Thank you so much, exelent that trick to work with 10px = 1rem and the function min().
@srikargunnam
Posted
Hi @AlexandroMunera, I forgot to mention that the trick with 10px = 1rem works only for font-size, sorry if i misguided you by not mentioning this.
Happy coding 😉