3 Column Preview Card

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great job on this one. Layout looks good in desktop, it is responsive and the mobile state looks great as well.

  kens_visuals already gave great feedbacks on this one, just going to add some suggestions as well:

  • Do not set a height: 100% on the html or body tag as they are relative to the viewport which limits their height. Use min-height: 100vh instead to allow the element to expand if needed.
  • Those car icons are just decoration. Decorative image must be hidden at all times by using alt="" and extra aria-hidden="true" attribute on the img tag.
  • Avoid using multiple h1 on a page, use only at least 1 per page so change those into other heading tags.
  • Also, since there are no text-content that are visible that could be h1, you will make the h1 screen-reader only text. Meaning this will be hidden for sighted users and only be visible for screen-reader users, search about sr-only stylings and see how it is used. The h1 text should describe what is the main content is all about, this h1 would be placed as the first text-content inside the main element.Have a look at Grace's solution on this one inspect the layout and see the usage of the h1 as well the stylings applied to it, copy those since you will use that a lot.
  • learn more should be a a tag, since it will likely to be a link rather than button in an actual website.

  Aside from those, great job again on this one.

  Marked as helpful

  0

  ---

  

  iduaine12•140

  @iduaine12Posted about 3 years ago

  Hi @pikamart,

  thank you for the suggestion on adding min-height: 100vh I read up on this but can you also provide me with a source of reference just to help me better get a grasp on this. I also appreciate the piece around aria-hidden="true" i wasn't too sure where to put this and make sense why I should use a tag over the button tag 🙌 Much appreciated for the feedback given

  0

  

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  @iduaine12 Hey, well I don't really have a resource about that one:>

  But, just for you to test out and see how it really works, what I want you to do is that, on this same solution of yours, go into dev tools. Open the dev tools at the bottom and stretch the dev tools height maybe about nearly half of the page or lesser.

  To see the difference, first bring back the height: 100vh on the body tag, as you do this, scroll the layout until you are on the bottom, then hover on the body tag. You will notice that it doesn't capture the whole layout because it is limited to the current-remaining-viewport. Then try to remove the height and use the min-height: 100vh and you will see the difference^^

  Marked as helpful

  0
• 

  P

  Ken•4,915

  @kens-visualsPosted about 3 years ago

  Hey @iduaine12 👋🏻

  I have a couple of suggestions on the project, just to make it more accessible and closer to the design

  • First, the car icons should have aria-hidden="true", because they are for decoration. You can read more about aria-hidden here.
  • Also, I suggest adding some border radius to the cards.
  • One last thing, I recommend adding transition: all 0.2s; to the button and the links, this will make :hover smoother. You'll see what I mean when you add it 🙃

  I hope this was helpful 👨🏻‍💻 Well done, nice job on responsiveness. Cheers 👾

  Marked as helpful

  0

  ---

  

  iduaine12•140

  @iduaine12Posted about 3 years ago

  Hi @kens-visuals,

  Once again this was very insightful, I wasn't aware of aria-hidden="true" thank you for leaving the source of reference for me to read up on. Also thanks for reminding me about adding transition I forgot to implement this. I will make the suggested changes and re-upload 👌🏾

  0

  

  P

  Ken•4,915

  @kens-visualsPosted about 3 years ago

  @iduaine12 no problem 👌🏻

  Marked as helpful

  0