All comments

• 

  Soltane rezaiguia• 10

  @soltane1414

  Submitted over 1 year ago

  

  interactive-rating-component-main

  1

  30

  

  Ricky Hewitt• 90

  @rickyhewitt

  Posted over 1 year ago

  Unfortunately it seems like it is broken in both desktop & mobile (screenshot)

  The star icon can be fixed by updating the path to:

  ./images/icon-star.svg
  

  The width of the card in the mobile version will need to be increased:

  margin: 5% auto 0 auto;
  width: 320px;
  

  While the margin between the buttons (.b1, .b2, .b3, .b4, .b5) on desktop will need to be decreased slightly to fit them all on one line as in the original design (or decrease button padding slightly).

  As the CSS for the .b1, .b2, .b3, .b4, .b5 is being duplicated, you only need one class for these.

  The submission leads to a 404, so unfortunately that also needs to be fixed.

  0
• 

  Gabriel Rodrigues• 190

  @ogabrielrodrigues

  Submitted over 1 year ago

  

  Frontend Mentor - Interactive card details form

  1

  20

  

  Ricky Hewitt• 90

  @rickyhewitt

  Posted over 1 year ago

  Great work!

  One thing I would note is that the desktop design seems to be incorrect, it is perhaps only using the mobile one.

  The other thing I noticed is that you could avoid duplication in your javascript for eventhandlers.

  For example, your existing code:

  document.querySelector("#name").addEventListener("keyup", checkErrorsOnPress);
  document.querySelector("#number").addEventListener("keyup", checkErrorsOnPress);
  document.querySelector("#mm").addEventListener("keyup", checkErrorsOnPress);
  document.querySelector("#yy").addEventListener("keyup", checkErrorsOnPress);
  document.querySelector("#cvc").addEventListener("keyup", checkErrorsOnPress);
  

  Could be replaced with either event delegation or a loop.

  const elements = document.getElementsByTagName("input");
  for (let i = 0; i < elements.length; i++) {
  elements[i].addEventListener("keyup", checkErrorsOnPress);
  }
  

  0
• 

  ~Estiven Mayhuay• 310

  @EstivenMayhuay

  Submitted over 1 year ago

  

  Responsive Landing Page CSS

  1

  20

  

  Ricky Hewitt• 90

  @rickyhewitt

  Posted over 1 year ago

  The header in the mobile version appears to be missing.

  I would also recommend against using position: absolute, but instead using CSS grid or flexbox.

  0