Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

3-Column Preview Card Component

Darkstarβ€’ 1,000

@DarkstarXDD

Desktop design screenshot for the 3-column preview card component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


Hello everyone! πŸ‘‹

I managed to finish another challenge. Any sort of feedback, including how to improve or optimize my code, is greatly appreciated.😊

Thank you πŸ™

Community feedback

Daniel πŸ›Έβ€’ 44,230

@danielmrz-dev

Posted

Hello @DarkstarXDD!

Your project looks great!

I noticed that you used a main tag and a div.container to wrap the main content. Unless you have a specific reason to do that, you could've used only the main tag.

It's not like this is a problem, it's just a suggestion for you to optimize it.

I hope it helps!

Marked as helpful

1

Darkstarβ€’ 1,000

@DarkstarXDD

Posted

@danielmrz-dev Thanks!

Nope that div actually had no purpose. I updated the code according to your feedback. Removed the div.container and modified main tag to be main.container. One less line of code now :)

Appreciate you taking time to go through my code. Thanks again.

2
Mouwvficβ€’ 210

@mouwaficbdr

Posted

Hey @DarkstarXDD

First, nice job on this project !

But you know, you can even leave the .container and directly use the main selector in the CSS to define the styles. In general, if the main tag is unique in your project and the style won't be reused elsewhere, directly targeting the tag can be a reasonable approach. However, if you plan to apply a similar style to other elements in the future, adding a class can make your code more flexible and maintainable.

Hope that helps somehow !

Marked as helpful

1
Darkstarβ€’ 1,000

@DarkstarXDD

Posted

@mouwaficbdr yeah, now that I think about it there was no reason to give the main tag a class. Could have applied the styles directly to the main tag since there is only one of them.

Thanks for the feedback. I appreciate it.

2
Mouwvficβ€’ 210

@mouwaficbdr

Posted

My pleasure @DarkstarXDD !

1
Daniel πŸ›Έβ€’ 44,230

@danielmrz-dev

Posted

I'm glad I could help 😊 @DarkstarXDD

1

@MelvinAguilar

Posted

Hello there πŸ‘‹. Good job on completing the challenge !

I have other suggestions about your code that might interest you.

HTML 🏷️:

  • You should use only one <h1> tag per page. The <h1> tag is the most important heading tag, This can confuse screen reader users and search engines. This challenge requires that Sedans, SUVs and Luxury are headings, but you can use the <h2> tag instead of the <h1> tag. You can read more about this here πŸ“˜.
  • Not all images should have alt text. Car icons are for decoration purposes only, so they can be hidden from screen-readers by leaving its alt attribute empty. You can read more about this here πŸ“˜.

I hope you find it useful! πŸ˜„ Above all, the solution you submitted is great!

Happy coding!

2

Darkstarβ€’ 1,000

@DarkstarXDD

Posted

@MelvinAguilar Thanks for your feedback!

I was bit confused what to use as a level one heading in this project, as it says there should be at least one h1 in a webpage. Source

I assume in this situation the best thing is to skip the h1 and start with h2.

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord