NFT Preview Card Component with HTML, CSS, and Flexbox

Community feedback

• 

  Elaine•11,400

  @elaineleungPosted over 2 years ago

  Hi Marta Jo, first off, you've done a great job completing this challenge! Not only does the solution look very close to the design, you've also done many things well (such as using flexbox). I also appreciate the way you've asked for feedback, and so, here are some of the thoughts I have that address the points you listed and also might give you some ideas on how to tackle future projects:

  1. Great job centering the component, which is one thing that many people actually struggle with. The one thing I'd change in the body selector is height: 100vh, which I would write as min-height: 100vh instead. To understand why, try resizing the browser's height and make it so that the window height is smaller than the contents. You may find that part of the content is cut off; this is because the body only has a fixed height of 100vh. When you use min-height: 100vh, the body selector would make sure its height is at least greater than 100vh, and therefore all the contents get shown.

  2. In your desktop view, your card has a max-width of 350px, and if I resize the window width and make it smaller, the component would suddenly grow huge at your breakpoint, and then become small again. That jarring jump in size is not really something you'd want. If you want the component to be at 350px even past the 600px breakpoint, I'd have max-width: 350px put in the .card selector already and not in the media query. It is OK to have width: 90% with max-width: 350px in the same selector; that's actually like writing width: min(90%, 350px).

  3. There's a bit of leftover code that got cut off somehow (not sure whether you can see the "> under the image), but yes, that happens at times!

  4. In your smallflex containers, I see that you've wrapped the containers with another set of containers; you can actually just have one container. Here's how I'd write the HTML and CSS:

     HTML:
     
     <div class="smallflex1">
        <div class="smallflex-item">
            <img src="./images/icon-ethereum.svg" class="card-icon" alt="Ethereum">
            <p class="price"> 0.041 ETH</p>
        </div>
        <div class="smallflex-item">                   
           <img src="./images/icon-clock.svg" class="card-icon" alt="Clock">
           <p class="time"> 3 days left</p>
        </div>
     </div>
     
     CSS:
     .smallflex-item {
         display: flex;
         align-items: center;  // for vertical alignment
     }
     .smallflex-item img {
         margin-right: 0.5rem;  // for the spacing between the icon and text
     }
     

  5. I think you need some more reset/normalize rules in your CSS; you have the box-sizing: border-box in the star selector, but right now there's still the default padding and margin in the browser. Without these rules, the padding and margins you set could be off, and for me, the first thing I do for any project is to copy and paste in my reset rules. Here are some basic ones to get you started:

     *, *::before, *::after {
       box-sizing: border-box;
     }
     * {
        margin: 0;
        padding: 0;
        font: inherit;
     } 
     html, body {
         height: 100%;
     }
     img, picture, video, canvas, svg {
        display: block;
        max-width: 100%;
     }
     

  As David says, it really is normal to spend lots of time trying to get something to look right, especially in the beginning when you're experimenting with what works and what doesn't. It's also normal to write out a whole bunch of CSS declarations, but you'll later discover that a lot of the code isn't necessary, and I learn a lot when I go back to my old solutions to clean up the code and learn from past mistakes. One thing that helps is, I like to comment out code and see whether something gets changed. If nothing is changed, chances are that line isn't necessary, and I'd keep it commented out instead of deleting it until the very end when I'm happy with all changes.

  Hope some of this can help you out, and I really think you're doing a great job already, so keep experimenting and keep going! 😊

  Marked as helpful

  1

  ---

  

  Marta Jo Telkamp•80

  @idlehands1969Posted over 2 years ago

  @elaineleung Wow, Elaine, that is some excellent feedback! Thank you so much for the advice, I will definitely be saving it for future use! Cheers!

  0
• 

  Lucas 👾•104,340

  @correlucasPosted over 2 years ago

  👾Hello Marta, congratulations for your new solution!

  Here's some tips for you:

  To make the card responsive you need to use max-width: 340px instead of width: 90% and a a better value for the border radius is border-radius: 12px

  .card {
      background: hsl(216, 50%, 16%);
      display: flex;
      flex-direction: column;
      border-radius: 12px;
      font-size: 18px;
      max-width: 340px;
      padding: 20px;
  }
  

  Your pricing section starts to break after 320pxa better way to save space inside the container and make the pricing section become two rows is by adding a media query, here;s the code for that:

  @media (max-width: 340px) {
  .smallflex1 {
      display: flex;
      flex-direction: column;
      margin: 0 auto 25px;
      text-align: center;
  }
  }
  

  👋 I hope this helps you and happy coding!

  Marked as helpful

  0

  ---

  

  Marta Jo Telkamp•80

  @idlehands1969Posted over 2 years ago

  @correlucas Thank you so much!

  1
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello Marta Jo Telkamp,

  Congratulation on completing this challenge. Excellent work! I see you have received some incredible feedback. I have few suggestions regarding your solution, if you don't mind:

  HTML

  • The most important part in this challengeinteractive elements. Since there's a :hover state on the image and means it's interactive, So there should be an interactive element around it. When you create a component that could be interacted with a user , always remember to include interactive elements like(button, textarea,input, ..) for this imagine what would happen when you click on the image, there are two possible ways:
    1: If clicking the image would show a popup where the user can see the full NFT, here you use <button>. 2:If clicking the image would navigate the user to another page to see the NFT, here you can use <a>.

  For the same reason, use <a> to wrap Equilibrium #3429 and Jules Wyvern.

  • Page should contain a level-one heading. In this challenge , as it’s not a whole page, you can have<h1> to wrap the Equilibrium #3429

  • The link wrapping the equilibrium image should either have Sr-only text, an aria-label or alt text that says where that link takes you.

  • For any decorative images, each img tag should have empty alt="" and add aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images in ( icon-ethereum, icon-clock ).

  • Profile images like that avatar are valuable content. The alternate text of the avatar’s image should not be avatar, it’s meaningless. You can use the creator's name Jules Wyvern. Read more how to write an alt text

  • look up a bit more about how and when to write alt text on images. Learn the differences with decorative/meaningless images vs important content

  • Never use <div> and <span>alone to wrap a meaningful content. Just keep in mind that you should usually use semantic HTML in place of the div tag unless none of them (the semantic tags) really match the content to group together. By adding semantic tags to your document, you provide additional information about the document, which aids in communication.

  • For middle part of the card class="smallflex1", you can use an unordered list <ul>, in each <li> there should be <img> and <p> for the reason stated before. That way you can align them centrally.

  • Adding rel="noopener" or rel="noreferrer" totarget="_blank"links. When you link to a page on another site using target=”_blank” attribute , you can expose your site to performance and security issues.

  There are so many ways to do the hover effect on the image, The one I would use is pseudo elements::before, ::after. You can use pseudo-elements to change the teal background color to hsla. Then the opacity can be changed from 0 to 1 on the pseudo element on the hover. Also using pseudo elements makes your HTML more cleaner as there's no need for extra clutter in the HTML.

  Overall, Your solution is good. Hopefully this feedback helps.

  Marked as helpful

  0

  ---

  

  Marta Jo Telkamp•80

  @idlehands1969Posted over 2 years ago

  @PhoenixDev22 Wow, so much information. I will definitely look into all of these excellent suggestions! Thank you!

  1

  

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  @idlehands1969 Glad to help. Happy coding!

  0
• 

  David•8,000

  @DavidMorgadePosted over 2 years ago

  Hello Marta, congrats on finishing the challenge, you did a pretty good job, keep it going!

  I would like to suggest you a little thing.

  I saw that you are using margin on your main and in your footer, you don't need to since you are already using flexbox to center your items on the body, if you want to get a bit of separation between the footer and the main, you can use gap instead of margin-top.

  For your question #2, it's normal that you waste a lot of time trying to get a solution, even more experience developers can take hours thinking of CSS solutions because they are not CSS experts, the experience will give you the improvements to make decision on your CSS faster and get your projects in no time!

  Hope my feedback helps you!

  Marked as helpful

  0

  ---

  

  Marta Jo Telkamp•80

  @idlehands1969Posted over 2 years ago

  @DavidMorgade Thank you so much for your kind suggestions. I will take them to heart. 😊 Cheers!

  1