NFT preview card component using Flexbox, SASS and media queries

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Greeting Larrie,

  Congratulation on completing this frontend mentor challenge.

  I have some suggestions regarding your solution:

  • There should be two landmark components as children of the body element - a main (which will be the NFT card) and a footer (which will be the attribution).<Footer> should not be in the <main >. HTML5 landmark elements are used to improve navigation .

  • Anything with a hover style in design means it's interactive. you need to add an interactive element <a> around the image , Equilibrium #3429 and Jules Wyvern.

  • The <div> element is the most general purpose element. It has no special meaning. It’s purpose is to group content that is not semantically related. Because it is essentially meaningless to screen readers, it should be sparingly used. <p class="description">Our Equilibrium collection promotes balance and calm.</p>

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

  • 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

  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.

  • The icon-view doesn't really need to be in the html, you could do it with css. If you want it to stay in html it needs to be aria-hidden or role presentation with empty alt.

  • There are so many ways to add the hover effect on the image , The one I would use, using pseudo-elements to change the teal bg color to a hsla. Then opacity can be changed from 0 to 1 on the pseudo element on hover as there is no reason to have the extra clutter in the html.

  • No need for <div class="line"></div> , you simply can use border-top: rule to the class="avater" .

  • The avatar's alt should not be avatar. It's meaningless .You can use the creator's name Jules Wyvern. Read more how to write an alt text

  • Never use <span> or <div>alone for meaningful content , you can use <p> instead.

  • To use more semantic tags , you can use <ul> to wrap class="price-block" and in each <li> there would be<img>and <p>. (For the same reason stated before , never use <div> alone to wrap meaningful content)

  You can use <figure> and <figcaption > for the avatar's part.

  • font-size: 62.5%;this has huge accessibility implications for those of us with different font size or zoom requirements. It's recommended not change the html or root font size.

    • width:100vw; If you set a page width, choose 100% over 100vw to avoid surprise horizontal scrollbars

  • Consider using min-height: 100vh; instead of height: 100vh to the body allows the body to set a minimum height value based upon the full height of the viewport also allows the body to to grow taller if the content outgrows the visible page.

  General points :

  • Really important to keep css specificity as low/flat as possible. The best way to do styling is single class selectors.

  • Remember a css reset on every project. That will do things like set the images to display block and make all browsers display elements the same.

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

  Marked as helpful

  1

  ---

  

  Larrie•100

  @AnirogPosted over 2 years ago

  @PhoenixDev22 Hi, thanks so much for the feedback, it's very helpful.

  1