React and Tailwind CSS

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Desktop layout looks nice but the layout in general needs to have bit more of contrast. Right now, each country card's color somehow for me makes it seem blend with the layout background-color. Maybe using the design provided where the layout's background is bit grey and each country card has white background with a box-shadow to give borders. Site is responsive as well and the mobile state looks fine as well.

  Some other suggestions would be:

  • Putting the header out of the main tag would be really great so that it will sit on its own row on the markup, since it is a primary landmark for users:

  <header />
  <main />
  

  • Using button for the theme toggle works but it is not the preferred markup. It should be done using 2 radio buttons which is inside a fieldset along with its legend element. Have a look at my solution on this one inspect the markup on the theme selections.
  • Also the svg being used on the theme-selections should be hidden since it is only a decorative svg so use aria-hidden="true" on it.
  • Your form-input right now currently lacks associated label to it or an aria-label to which will define the purpose of the input element.
  • Your outline for interactive elements lacks a color, right now you are using transparent for the outline.
  • You should not have used the a tag as the parent container that will nest all of those country information. What you could have done is something like, the a tag will only wrap the country-image, then use the a tag's ::after or ::before to have full height and width of the whole card so that the card will be clickable. This way, the markup will be valid.
  • For each of those country card information, those 3 items below each country name could have used ul since those are "list" of information and not using h4 to wrap the full text.

  VISITING A COUNTRY (on this :>)

  • Don't wrap an a tag within a button or vice-versa. For this, use only a tag since the go back would be a link. It is invalid to nest those two since there will be 2 focus on each item. Try using tab key on the go back.
  • Same as well for those information, use ul on them since they are "list" of items.
  • Again, for each border-countries, use a tag only and not a tag inside button.

  Aside from those, great job again on this one.

  Marked as helpful

  0

  ---

  

  Sandeep Gumaste•365

  @SandeepGumastePosted about 3 years ago

  @pikamart Thank you very much for the feedback. Will get to fixing these now. :) i guess I need to start watching tutorials on accessibility and semantic html.

  1