Mobile first design using flexbox and scss

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Desktop layout looks nice and great to have that custom attribution below it! Mobile state looks fine, but your breakpoint is too early to show the mobile state. Right now, 1024px already shows the mobile state and it is too big for it, desktop layout could use more of those screen don't you think.

  Some other suggestions would be:

  • There is an error in the console, you might want to check that one out.
  • You don't really need to use article for each of the car since they are just normal contents of the site. Using div would be better on this one.
  • Those car icons are only decorative images. Decorative images should be hidden for screen-reader at all times by using alt="" and aria-hidden="true" to the img tag or only aria-hidden="true" if the image is using svg.
  • Only use descriptive alt on images that are meaningful and adds content to the site otherwise hide the image for screen-reader users.
  • Only have a single h1 on a site. It would be great to change those headings into something like h2.
  • Since there are no visible text that are suitable to be h1, the h1 would be a screen-reader only heading. Meaning it will be hidden visually but present for screen-reader users. On this, the h1 would have like sr-only class and the text-content should describe what is the main-content is all about. The h1 could be placed as the first text inside the main.Have a look at Grace's solution on this challenge inspect the layout and see the usage of h1 as well the stylings applied to it.
  • It would be better to use a tag rather than button for the learn more, because on a real site, that would be a page-link where the user can "learn" more about a specific car.
  • Lastly, just for the breakpoint to be adjusted :>

  Aside from those, great job again on this one.

  Marked as helpful

  0