Skip to content
Submitted over 1 year ago

Second Challenge, using HTML and CSS only. Fixed some stuff

Feithers•180
@Feithers
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Product preview card component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

Hello! This is the second challenge I've done so far. This time around I think I got it pretty good. Maybe there's some stuff on the code that can be made in simpler ways, and that's probably the advice that I would like to know so I can learn for the next one. Thanks! *This is the update, I've fixed some stuff!

Community feedback

  • Melvin Aguilar 🧑🏻‍💻•61,140
    @MelvinAguilarPosted over 1 year ago

    Hello there 👋. Good job on completing the challenge !

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

    HTML 🏷️:

    • Avoid using uppercase text in your HTML because screen readers will read it letter by letter. You can use the text-transform property to transform the text to uppercase in CSS.

      The word "perfume" is written as separate letters, which does not convey the meaning that this text is a single cohesive unit of content. This can be confusing for users and for screen readers, as it can be difficult to understand the meaning of the text.

      Example:

      <p>Perfume</p>

p {
    text-transform: uppercase;
    letter-spacing: 0.3em;
}
    • The alt attribute should not contain the words "image", "photo", or "picture", because the image tag already conveys that information.

    • Not all images should have alt text. The cart-icon is a decorative image, it does not add any information to the page. You should use an empty alt attribute instead of a descriptive one. You can read more about this here 📘.

      If you want to learn more about the alt attribute, you can read this article. 📘.

    I hope you find it useful! 😄

    Happy coding!

    Marked as helpful
    0
  • Abdul Khaliq 🚀•72,560
    @0xabdulkhaliqPosted over 1 year ago

    Hello there 👋. Congratulations on successfully completing your first challenge! 🎉

    • I have other recommendations regarding your code that I believe will be of great interest to you.

    HTML 🏷️:

    • <html> element must have a lang attribute with valid value, so fix it by <html lang="en">

    CSS 🏷️:

    .main { 
    display: flex;
    justify-content: center;
    margin-top: 12rem;
}
    • Instead try this,
    .main { 
    display: grid;
    place-items: center;
    min-height: 100vh;
}
    • This css snippet will helps your component to be placed at center of the screen dynamically without any margin or padding

    I hope you find it useful! 😄 Above all, the solution you submitted is great!

    Happy coding!

    0
    Feithers•180
    @FeithersPosted over 1 year ago

    @0xAbdulKhalid What does the min-height property exactly do? And what measurement is vh? Thanks, I'll try all that!

    0
    Abdul Khaliq 🚀•72,560
    @0xabdulkhaliqPosted over 1 year ago

    @Feithers

    min-height property:

    • We want to use min-height: 100vh instead of height: 100vh.
    • Setting the height to 100vh may result in the component being cut off on smaller screens, such as a mobile phone in landscape orientation.
    • This why we prefer to use min-height instead of height

    vh property:

    • The full form of vh is viewport height.
    • It works like the percentage unit as well. Specifying 10vh is equivalent to occupying 10% of entire visible screen height.
    • If you look carefully, you can see that 50vh means 50%, which will cover half of the entire screen height.

    Hope this reply will be helpful for you

    Feel free to reach me if you have any queries

    Marked as helpful
    0

Please log in to post a comment

Log in with GitHub

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