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-main

arowstevβ€’ 120

@arowstev

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


All feedbacks are welcome.

Community feedback

@MelvinAguilar

Posted

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

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

  • 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 πŸ“˜.
  • The "<div class="learmore">" should be a button and not a div. Since the element has a hover effect and is interactive in the design, it's better to use a interactive element like links or buttons for accessibility. Also remember to add a cursor: pointer

I hope you find it useful! πŸ˜„

Happy coding!

1
Marcos Travagliniβ€’ 4,920

@Blackpachamame

Posted

Greetings @arowstev! There are a few things to fix:

  • The images are not visible, this is because you placed the paths wrong, from src="/images/icon-suvs.svg" they should be `src="./icon-suvs.svg"
  • The header is empty, it has no use, it is better to delete it
  • The <div class="mainbox"> should be <main class="mainbox">, the previous main just delete it, it serves no purpose
  • The <div class="attribution"> should be <footer class="attribution">, please leave it out of the main
  • To center the content do not use position: absolute, rather use flex:
body {
    min-height: 100vh;
    display: flex;
    justify-content: center;
    align-items: center;
    flex-direction: column;
    gap: 20px;
}

.mainbox {
    display: flex;
    min-height: 400px; /* This change adapts better to the screen change */
    max-width: 800px; /* This change adapts better to the screen change */
    /* position: absolute; */
    /* top: 50%; */
    /* left: 52%; */
    /* transform: translate(-50%, -50%); */
    border-radius: 5px 5px 5px 5px;
}

@media (max-width: 1250px){
  .mainbox {
    flex-direction: column;
    min-height: 400px; /* This change adapts better to the screen change */
    max-width: 800px; /* This change adapts better to the screen change */
  }

 .mainboxone {
    border-radius: 5px 5px 0 0;
    padding-bottom: 20px;
 }

 .mainboxtwo {
    padding-bottom: 20px;
 }

 .mainboxthree {
    border-radius: 0 0 5px 5px;
    padding-bottom: 20px;
 }
}
1
arowstevβ€’ 120

@arowstev

Posted

Thank you for the feedback. I will note all the corrections.

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

@danielmrz-dev

Posted

Hello @arowstev!

You did a very good job there!

I have just a couple of very simple suggestions for improvement:

  • First: Since the buttons Learn more are clickable element, it's nice to add cursor: pointer to them.

  • Second: Your icons are not showing because you set the incorrect path for them.

This is your code:

<img class="sedaniconimage" src="/images/icon-suvs.svg" alt="">

But since they are in the same folder than the HTML file, you have to remove /images/ from the path, like this:

<img class="sedaniconimage" src="icon-suvs.svg" alt="">

Advice: Keep the assets of the project (images, fonts) in a separate folder. It helps with the organization of the project.

I hope it helps!

Other than those details, you did a great job!

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