css html

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @rahunak ,

  Congratulation on completing this frontend mentor challenge.

  I have some suggestions regarding your solution:

  • First of all , you have used some extra <div>.Remove the commented code that may confuses other developers or even you in the future.

  • 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 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.

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

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

  • To use more semantic tags , you can use <ul> to wrap class="coin_description__count_img" and in each <li> there would be<img>and <p>.

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

  CSS

  First of all why display none for the smaller screens ?

  `@media (max-width: 374px) {
      body {
          display: none;
      }
  }
  

  • As you used flexbox properties to the body you can add min-height to the body to center the component on the middle of the page .

  • width: 335px;an explicit width is not a good way . consider using max-width to card instead and a little margin to the card .That will let it shrink a little when it needs to

  General points :

  • 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.

  • It's recommended to use em and rem units .Both em and rem are flexible, Using px won't allow the user to control the font size based on their needs.

  You might have a look at my solution , to see how I did the hover effect on the image , it might help.

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

  Marked as helpful

  0

  ---

  

  Евгений•50

  @rahunakPosted over 2 years ago

  @PhoenixDev22 Thanks for your advice, i correct my code. You are really smart and good human.Thanks you a lot.

  0
• 

  David Paulos•80

  @David-PaulosPosted over 2 years ago

  I have a very simple solution for that problem

  Have a look at my code

  On the HTML I have a div that contains two items, the cube image with a class of "three-D-cube" and an empty div with a class of "hover-eye".

  On the css I styled the image "three-D-cube" like this

  .three-D-cube { width:100%; height:100%; border-radius:10px; }

  .image-box { width: 232px; height: 232px; object-fit:contain; padding-block-start:14px; margin-block-end:15px; }

  On the css I styled the empty div "hover-eye" in order to sit on top of the "three-D-cube" and to be ready for the :hover state.

  .hover-eye { width: 232px; height: 218px; position: absolute; top: 14px; left: 14px; border-radius:10px; }

  Finally the "hover-eye" on hover state like this:

  .hover-eye:hover { cursor:pointer; background-color: hsla(178, 100%, 50%, 0.5); background-image: url(./images/icon-view.SVG); background-repeat: no-repeat; background-position:center; }

  Let me know your thoughts

  0
• 

  P

  Marcell Mihály•750

  @mihalymarcell86Posted over 2 years ago

  Hi ЕВГЕНИЙ,

  the problem with the eye seems to be that it's behind the image. When you hover over the image, its opacity changes to 0.4, but it still covers the image underneath. The eye needs to appear on top of the image. You will want to separate the eye from the cyan background. I used ::before and ::after pseudo-elements, but z-index should be useful in this case as well.

  Hope that helps!

  0