Skip to content
OverviewSolutions
15
Comments
18
Mohammed Abbas
@Mohammedabbas7

All comments

  • Mohammed Abbas•390
    @Mohammedabbas7Posted 10 months ago

    Hello

    Congrats on completing this challenge🎉🎉🎉🎉.

    Nice solution and also like what you did with the attribution.

    I noticed a few things in your solution you need to adjust:

    • First instead of adding height: 100vh to the body element because this causes some overflow issue so add min-height: 100vh to make the responsive.
    • Second instead of adding height: 60% to the .content element just add gap property gap: 1rem.
    • you did add a height property to .container element that is not good practice, so don't add specific height to elements unless it's so important to add it.

    I hope my advice is useful.

    Happy Coding :)

    0
  • Mohammed Abbas•390
    @Mohammedabbas7Posted 10 months ago

    Hello

    congrats on completing this challenge 🎉🎉🎉🎉.

    nice solution by the way but I have a few adjustments for you:

    • instead of using the flex-wrap: warp property on the .root element to make the layout for the mobile version to make the site responsive for different screen sizes you can use media queries like this:
    @media(max-width: 768px) {
    .root {
        flex-direction: column;
    }
}
    • also I noticed that you did not add the hover effect to the button but you did add the focus effect, you can add this code to add hover:
    button:focus,
button:hover {
    background-color: transparent;
    color: white;
 }
    • also you can add the transition properly on the button to make the transition smooth use this property transition: all .3s;.

    I hope you find these adjustments useful.

    Happy Coding :)

    Marked as helpful
    0
  • Mohammed Abbas•390
    @Mohammedabbas7Posted 11 months ago

    Congrats on completing this challenge 🎉🎉🎉🎉

    Nice solution.

    I have some advice for you

    • the hover effects on the img-overlay instead of using display: none; it's better to use opacity: 0; on the img-overly and also instead of using display: block; on the main_img-box:hover .img-overlay rule you can use opacity: 1;
    • also you can add transition property on the img-overly to make the background-color and the icon have some transition when you hover over it, add this transtion: opacity 0.5s;.

    That is all I have, I hope you find it useful.

    Happy Coding :)

    Marked as helpful
    1
  • Mohammed Abbas•390
    @Mohammedabbas7Posted over 1 year ago

    Congratulation on completing this challenge, well done 🎉🎉🎉.

    First, regarding your question, it's okay to use position absolute property when it needs it but in this scenario, you did not need it.

    Second, I guessed that you used position absolute to center the card in the middle of the screen this better way to center the div is by using flexbox, here is:

    HTML

    • create a div with a class of container inside the body element.
    • inside that container create a div with the class of card to hold the content of the card.
    <body>
    <div class="container">
        <div class="card">
        <div>
    <div>
</body>

    CSS

    • add this style to the container to center the card.
    .container {
    min-height: 100vh;
    display: flex;
    align-items: center;
    justify-content: center;
}

    You can test this code and tell me if it works for you instead of using the position absolute property.

    If you don't know about flexbox watch this video by Kevin Powell.

    Happy coding :)

    Marked as helpful
    1
  • Mohammed Abbas•390
    @Mohammedabbas7Posted over 1 year ago

    Congratulation on completing this challenge, well done 🎉🎉🎉. I have a few suggestions for you:

    • instead of using the main element to center the card you could create div with class of container inside the main to center the card, and inside the container create another div with the class of card to hold the column like this:

    HTML

    <main>
    <div class="container">
       <div class="card">
       </div>
    </div>
</main>

    CSS

    .container {
display: flex;
align-item: center;
justify-content: center;
min-height: 100vh;
}
    • I noticed that you gave the .car-type class height property. it's better to let the content define the height of its parent instead of give in it a specific height.

    I hope what I gave you is helpful.

    Happy coding :)

    0
  • Simon•20
    @simonmatt89Submitted almost 2 years ago

    This is my 2nd project that I have completed with Front End Mentor. This time, I hope I have improved my use of sementic HTML code. I do wonder if my CSS code could be more efficient? I'm looking forward to checking out other's solutions so I can compare my code.

    I also find it easier to create a mobile site than I did previously. I was able to create this project much faster than the last which makes me happy!

    3 column preview card component

    2
    Mohammed Abbas•390
    @Mohammedabbas7Posted almost 2 years ago

    Congratulation on completing the second project 🎉. I have a few adjustments that I wanna show it to you.

    • replace the <div> tag with <footer> tag on the attribution section because it has semantic meaning.
    • you forgot the hover state on the Learn More buttons. you can add these properties to the button element
    button {
display: inline-block;
    background-color: "color that you choose";
    color: #333;
    padding: .8rem 1.5rem;
    border: 1px solid "color that you choose";
    border-radius: 2rem;
    transition: all .2s;
}

    for the hover state you can add these properties:

    button:hover {
    background-color: transparent;
    color: "color that you choose";
}

    well done on this challenge and happy coding on the next challenge 👍.

    Marked as helpful
    0
  • Lea•160
    @HatchinoSubmitted about 2 years ago

    I'm currently stuck on a challenge, in the meantime I tried this one which is simpler. Feel free to leave any suggustions !

    Responsive card section CSS/HTML

    2
    Mohammed Abbas•390
    @Mohammedabbas7Posted about 2 years ago

    hello, nice solution. I just wondering why you used internal CSS instead of external CSS.

    0
  • Mohammed Abbas•390
    @Mohammedabbas7Posted about 2 years ago

    congratulation on completing the challenge. I just have a single comment on your solution you just forget to add a hover and focus state on the button. you can add this code to change the background color when you hover with the mouse or use the keyboard to navigate and also you can add a transition property on the button to make the transition smooth.

    button {
transition: background-color .5s;
}

button: hover,
button: focus {
cursor: pointer;
background-color: hsl(159, 63%, 16%);
}
    0
  • Mohammed Abbas•390
    @Mohammedabbas7Posted over 2 years ago

    Hello, congratulation on completing the challenge.

    • you just need add hover effect to the button by add this sytle to your section button rule, and remove border: none;
    section button {
border: 1px solid #fff;
}

section button:hover {
background-color: transparent;
color: #fff;
}
    1
  • Mohammed Abbas•390
    @Mohammedabbas7Posted over 2 years ago

    Hello, nice work by the way. just you have a little problem with your background-image URL, you typed the URL wrong.

    you need to be like this .card-image { background-color: url(../images/illustration-hero.svg; }.

    Keep up the good work :)

    Marked as helpful
    1
  • McMuffin1022•30
    @McMuffin1022Submitted over 2 years ago

    Hi, I make it in pure html /css. If you have any advice, im interested to know it!

    Thanks for your Time, McMuffin1022

    NFT preview card component

    1
    Mohammed Abbas•390
    @Mohammedabbas7Posted over 2 years ago

    Congratulations on completing the challenge.

    This is just a little advice:

    • you just need to add hover effect to the image, title and creation name.

    If you don't know how to make it just tell me.

    Happy coding:)

    0
  • Mohammed Abbas•390
    @Mohammedabbas7Posted over 2 years ago

    Congratulations for completing the challenge.

    I just have a little feedback for you, you need to add .card-image div { transition: opacity .3s; } for the hover state to work.

    Happy coding :)

    0