Vite App with styled components, Redux, and React-Dropdown component

Community feedback

• 

  Raymart Pamplona•16,040

  @pikapikamart

  Posted over 3 years ago

  Hey, awesome work on this one. The desktop layout looks great, but on my end, I only get a 3 country card per row. I am using a 1366x768 monitor, also you can make the country cards be justified at center so that when another country card needs to be wrapped in another row, the remaining will be centered something like instead of being left on the left-side like when you go to 106px. The mobile state looks great though.

  Some other suggestions on the site would be:

  • I am getting some logs so maybe checking if you have left some console.log on your code.
  • Always have a main element to wrap the main content of the site. For this use main tag on the .homepage selector.
  • The theme toggle works but the markup could be improved on that one. When you are building out a theme toggle in general, it will be better to use radio-buttons since a theme toggle is a selection and radio-buttons are intended for those. The radio-buttons will be placed insidea fieldset along with a legend tag that will describe what is the purpose of those input tags. You can have a look at my solution on this same challenge. Inspect the markup on how it is made and let me know if you have queries about this one.
  • Do not remove the outline styling. If you did, always include a visual-indicator on the :focus-visible for those interactive elements like the button a tag and others.
  • Also, you don't use span inside an svg. Another one, the svg for the theme toggle is just a decorative svg so adding an aria-hidden="true" attribute on it would be really great.
  • The search-icon on the search-bar as well is decorative so hide it using the method above.
  • Your search-bar input right now currently lacks associated label to it or an aria-label to which will define the purpose of the input element. Always include it so that user will know what they need to give on each input. It could be something like aria-label="country name".
  • Also, when you are typing out the country name, since you are adding like a :hover and bringing up a clear-button, it would be really great to just place the button directly on the markup so that user could toggle it even without using mouse. Making inclusive components should be prioritize when building it.
  • For the filter-bar, currently it is only limited for mouse clicks since you are not using an interactive element on it. Remember that interactive components needs to use interactive elements. For this one, you could use select tag, though it can't be styled. Another approach would be to use role="listbox", you can see that on my solution since I implemented that one.
  • For each of the country card, the a tag should not be nesting all of those components since it will be an invalid markup. You could use the a tag to wrap the img and maybe just use the a tag's ::after to occupy the full height and width of the single-country-card so that the whole can be toggled.
  • Those 3 information about the country card could use a ul tag since those are "list" of informations.

  VISITING A COUNTRY (on your app) :

  • Don't wrap an a tag inside of a button or the other way around. Use only a tag on the go back link.
  • The country name could be an h1 because remember that every page of a site needs to have an h1, so for this, use the h1 on the name since it is all about the country on the single page.
  • Again, those 8 information could use a ul tag since they are "list" of information. Also, it would be really nice to add some paddings on those or gap because right now, every text are almost touching each other.
  • For the border ul tag, using any other element except li tag as the direct child of ul is invalid. Change the p tag on that one or maybe just use like:

  div.border__holder
    h2.Border
    ul
      >li
  

  A markup of something like that would be fine.

  • Lastly, change each border-countries to using only a tag. a tag inside a button or vice-versa just adds an extra tab when navigating.

  Aside from those, great job again on this one.

  01

  Marked as helpful