Product Preview Card

Community feedback

• 

  T

  Chamu•13,340

  @ChamuMutezvaPosted 4 months ago

  Hi meredithincode

  1. The challenge has 2 images for the image-product , one to be displayed on small devices and another to be used for desktop devices . One of the recommended method to use is the picture element - mdn doc. You have listed both images at the moment as img elements and then used CSS to interchange the display of the images based on the screen sizes - this is not a recommended approach and some of the reasons are that resources that the user will not use will be downloaded. For example , if I am on mobile both the desktop and mobile images will be downloaded even though the mobile image will be displayed. Hence that is a waste of data for the user as well an increased loading time for the application . The picture element will only allow the required image to be downloaded.
  2. Heading elements should follow a sequential order - you can not have an h1 element after the other heading elements (h2, h3 etc) - that is the order should be that the first heading element should be an h1 element and that can only be followed by an h2 element . Then an h2 element can be followed by an h3 etc. Read the following heading elements structure - mdn for more clarity. But since this is just a component , you can do away with the h1 element and just have either an h2 or h3 element
  3. It is important to always use semantic elements everytime , if the element is a button , then just use a button. Using a div for a button as in this case <div class="purchase-btn"> has not added benefit but rather it has a drwaback. The button in this case is not accessible to assistive technology users
  4. Your CSS should always include a modern reset style sheet so that the site can be normalised between different browsers.
  5. style using a class instead of an id , that will keep your specificity low and also the id is meant for js manipulation
  6. when styling using mobile first , your inital styles will target the small devices and then use the min-width in your media query - you won't need the max-width

  0
• 

  MikDra1•7,060

  @MikDra1Posted 4 months ago

  Nice one 😀,

  My way of centering of card is right here, maybe you can try this:

  .container {
  display: grid;
  min-height: 100vh;
  place-items: center;
  }
  

  Hope you found this comment helpful 💗

  Good job and keep going 😁😊😉

  0
• 

  James.•340

  @JimztechPosted 4 months ago

  Nice work, but for your mobile view, your button should have more padding for the left and right. Just a suggestion. But great work Chief.

  0
• 

  geomydas•1,460

  @geomydasPosted 4 months ago

  I dont think that should happen no matter the browser. Maybe check if your Opera version is very old and does not support some of the latest CSS properties or there is a browser extension that is interfering wth it

  0