NFT-preview-solution-using-CSS-Flexbox

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello Pedro Castaneda,

  Great work! I have some suggestions regarding your solution:

  HTML

  • Use the <main> landmark to wrap the body content. HTML5 landmark elements are used to improve navigation

  • Since there's a :hover state on the image and means it's interactive, So there should be an interactive element around them. 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 use <a>.

  • Also use <a> to wrap Jules Wyvern and Equilibrium #3429

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

  • The alternate text should not be hyphenated, it should be human readable.

  • 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-view, icon-ethereum, icon-clock ).

  • If you wish to draw a horizontal line, you should do so using appropriate CSS. Remove this

          <div class="main-divider">
            <div class="divider-container"></div>
          </div>
  

  , you can use border-top: to the avatar's part

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

  • You can use unordered list<ul> to wrapclass="main-icons". In each <li> should be <img> and ``<p>`, then you can align them centrally.

  • To use more semantic tags , you may use <figure> and <figcaption> for the avatar's part.

  CSS:

  • Consider using rem and em as they are flexible units rather that absolute units px, better using rem for font size.

  • If your web content font sizes are set in absolute units, such as pixels, the user will not be able to re-size the text or control the font size based on their needs. Relative units “stretch” according to the screen size and/or user’s preferred font size, and work on a large range of devices.

  • With headings, a screen reader can read the page’s “table of contents” as implied by the heading structure (like the above list), and the user can jump directly to the desired section. By breaking a page up with headings, a user can scan the page faster and more effectively.

  • Last , there are some extra div’s.

  Aside these, Your solution is good. Hopefully this feedback helps.

  Marked as helpful

  0

  ---

  

  P

  Dante Castaneda•80

  @PedroCastaneda2000Posted over 2 years ago

  @PhoenixDev22

  Hi PhoenixDev22 ,

  Regarding Feedback:

  • Implemented the <main> landmark
  • Used interactive elements w/ hover state
  • Rewrote the alt proprieties
  • Added aria-hidden = "true" to icons
  • Used underorder list w/ icons
  • Used border-top to produce the horizontal line
  • Used rem instead of px to specify size

  Thank you PhoenixDev22 for the feedback! 😄

  1

  

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  @pgc0004

  You are welcome. Just one thing :

  • According to web standards you aren't allowed to put block elements into inline elements. As h1 is a block element and a is an inline element the correct way is: <h1><a href="#"> Equilibrium #3429</a></h1>

  After this, you can generate another report for your solution.

  0