Interactive rating card using Javascript

Community feedback

• 

  mubizzy•1,520

  @mubizzyPosted over 2 years ago

  Excellent job on this challenge! your report has a few issues though:

  1. wrap everything in your body in <main> or use semantics

  2.   it is a best practice to use both HTML 5 and ARIA landmarks to ensure all     content is contained within a navigational region.

  Hope it helps:)...don't forget to mark it as helpful 👍

  You can get more details here...click here

  Marked as helpful

  0

  ---

  

  CodingTube•510

  @CodinGitHubPosted over 2 years ago

  @mubizzy Thank you very much for your feedback. I'm going to google a litle bit about ARIA landmarks. Those are new for me. 😉

  0
• 

  Danilo Blas•6,300

  @Sdann26Posted over 2 years ago

  Hola CodingTube!

  Tu maquetación te ha quedado bastante limpia, me gusta. La funcionalidad esta bastante bien. En lo personal solo le cambiaría 4 cosas:

  a. Al <section class="rating-state"> le quitaría el margen, ya que no ayuda al centrado responsive ya que los margenes son estaticos, centra bien el horizontal pero en vertical no lo hace y por ejemplo en mi pantalla el componente se ve para marcar más abajo de lo que se debería, en cambio quitale los margenes y al body colocale además los siguientes atributos:

  { 
     display: flex;
     flex-direction: column;
     justify-content: center;
     align-items: center;
     min-height: 100vh;
  }
  

  b. Le cambiaría el color de la letra del footer ya que no contrasta bien, no digo que deba resaltar mucho pero el negro es muy opaco más bien utilizaría un color claro por ejemplo: color: rgba(149, 158, 172, 0.5);

  c. Agregale transiciones, a los botones, esto da un toque más profesional y es bastante amigable a la vista. A los elementos con las clases :hover, :active, etc. le puedes colocolar el atributo transition: transition: all 250ms;. Puedes leer la documentación para ver que valores le puedes dar.

  d. Coloca todo el <section> dentro de un <main>. Por temas de accesibilidad siempre debes tener 1 por página que englobe todo el contenido principal. Por cierto, además de hacer eso debes colocar un h1 tambien por página, puede agregarlo dentro del <main> y colocarlo con la clase sr-only que tengas los siguiente atributos con la finalidad de que este presente en el esquema de tu página pero no se pueda ver para que no altere el diseño:

  .sr-only {
    position: absolute;
    width: 1px;
    height: 1px;
    padding: 0;
    margin: -1px;
    overflow: hidden;
    clip: rect(0,0,0,0);
    border: 0;
  }
  

  Hay un tema más si revisas el reporte, sale que no te recomienda usar los atributos ARIA que has usado, capaz no son permitidos en el elemento que has usado, pero te recomiendo que los elimines ya que el propio reporte te lo recomienda, una ves elimines todos los errores del reporte, generá uno nuevo :D.

  Y solo eso buena suerte.

  0
• 

  lieneil•560

  @NJVSPosted over 2 years ago

  Try to use <input type="radio"> instead of <button>. With this you'll only need one event listener, instead of the numberContainer and check which button triggers the click event. GOOD LUCK <3

  0