Github Search HTML/CSS/TypeScript, ES6 Modules and PWA

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Layout in desktop looks great, it is responsive and the mobile layout looks great as well.

  Some suggestions would be:

  • Avoid using height: 100vh on a large container especially the body tag, this makes the element's height limited to the current remaining screen. Try to inspect the layout in dev tools at the bottom, scroll down and hover on the body tag, you will see that it doesn't captures the whole layout. Instead, use min-height: 100vh this takes full height and will expand if it needs to.
  • The theme-selection is not accessible at the moment since you are using div, when creating an interactive component make use of interactive elements. On that one, you could have used input type="radio" since it is a selection of choice, the input will be inside a fieldset along with an sr-only legend element that describes what the set of radio buttons are for. This way, it will be more accessible for all users. It can quite a tricky to implement but you have to, I don't have solution for this one, [but take a look on how my theme-selection is structured(https://gallant-albattani-35cdfb.netlify.app/) that is my todo-app solution, inspect the color theme.
  • The input needs to have an label associated with it, the label would have sr-only class. The label's text should describe what does the input does.
  • It's great that there is an error message, but at the moment it is not associated internally with the input. What you should do looks like this pseudocode:

  const id = id of the error-message;
  if ( input is wrong )
    input.setAttribute("aria-invalid", "true");
    input.setAttribute("aria-describedBy", id);
  else 
    input.removeAttribute("aria-invalid");
    input.removeAttribute("aria-describedBy");
  

  This way, users will know that there is an error from their input because of the aria-invalid and they will know as well what kind of error they had made because of the aria-describedBy attribute. You can take a look at my simple snippet on how to use those method

  • button needs to have type="submit", always be explicit what your button should be.
  • Also an addition, to make it even more accessible, you can make use of aria-live attribute to an element, maybe announce if the form submission is invalid, if there is a user or if there is no user.
  • Name of the person could use a heading tag since it is the primary text content of the app. Also avoid just wrapping a text inside a strong span div , use a meaningful element like a p tag.
  • I would use ul on the user__stats since those are "list" of information.
  • The alt for each of the icon usage on the bottom part should be left empty alt="" and added an extra aria-hidden="true" attribute on the img. Since it is a decoration only, you want to hide it.

  Aside from those, great work again on this one.

  1
• 

  FocusCookie•350

  @FocusCookiePosted about 3 years ago

  Nice job! 👍 You linked github repo is not working :(

  1

  ---

  

  Clément Creusat•1,655

  @ccreusatPosted about 3 years ago

  @FocusCookie Thanks! hahaha it was in private mode, I don't know why :) I changed the settings!

  0