@jorditosa
Posted
Hi Melvin,
Excelent work! Just to say something: For anchor elements, it is good practice target "_blank" attribute.
Then, for html markup, probably I would split main element to use it only for "join our community" container, and other two being created as section elements...all as the main part of the webpage, not all is the dominant content.
Finally, probably you have used many arbitrary values...then, could be better developing directly with css or with preprocessor. For example, class p-[1.5625rem], that you repeat often, would be faster to create a variable with that value (:root), and then you do not need to write different times.
I repeat, that is an excellent work! Is just to give some extra opinion, not best, but mine! keep working
Marked as helpful
@MelvinAguilar
Posted
@jorditosa Hello! Thank you very much for the valuable feedback, all opinions are important.
I appreciate the time you took to review the solution and write the comment
Regarding the <main> element, <main> is a landmark and can be a direct child of the <body> tag. <section> is not a landmark, it is a semantic tag (cannot be a direct child of the body tag) so it should be inside the <main> tag, but since the challenge is a small component, I don't think it's necessary to use <section> and only used <div> to separate each part.
PD: I can't have something like this because it would generate the warning in the report: WARNING: 'All page content should be contained by landmarks':
<body>
<div>
<main> <h2>Join our community</h2> </main>
<section></section> /* or <div></div>*/
<section></section> /* or <div></div>*/
</div>
</body>
Using target _blank is to open the link in a new tab, and I think that the link doesn't redirect to any new information. If they were links to other pages like in the footer of this challenge I would use target blank, but in this case I just let it refresh the page. More info: G200: Opening new windows and tabs from a link only when necessary - WCAG 2.0
I totally agree that using too many arbitrary values is a bad practice, it's a punishment for trying to make it pixel perfect. Note: Fixed.
Once again, thank you very much for reviewing this solution.
