3 Column Preview Card Component

Community feedback

• 

  P

  Ken•4,915

  @kens-visualsPosted about 3 years ago

  Hey @IshitaBisaria 👋🏻

  I have a quick tip for the icons. I suggest adding aria-hidden="true", because they're for decoration only. For example, <img src="images/icon-luxury.svg" alt="" aria-hidden="true" class="logo">. You can read more about aria-hidden here.

  I hope this was helpful 👨🏻‍💻 overall, you did a great job, just tweak media-query sizes a bit. That's it from me, cheers 👾

  Marked as helpful

  0

  ---

  

  Ishita Bisaria•40

  @IshitaBisariaPosted about 3 years ago

  Hey @kens-visuals , Thank you so much for the suggestion. I have updated the aria-hidden = "true" in the img tag in html. I have also updated the media-query size. Thank you so much :)

  0

  

  P

  Ken•4,915

  @kens-visualsPosted about 3 years ago

  @IshitaBisaria you're welcome 😇

  0
• 

  Fidel Lim•2,775

  @fidellimPosted about 3 years ago

  Hi Ishita,

  Great work finishing the project! Just some suggestions you can apply:

  • you can add cursor:pointer to the buttons so that the user knows that it can be clicked.
  • for your media query, it would be better to increase is as some of the component is hidden. you can set the media query to @media (min-width: 900px) instead.

  I hope it helps. :)

  Marked as helpful

  0

  ---

  

  Ishita Bisaria•40

  @IshitaBisariaPosted about 3 years ago

  Hey @fidellim , Thank you so much for the suggestions. I have updated the cursor as pointers. Regarding the media queries, it was actually given in the style guide that Mobile width is 375px. Thanks you so much :)

  1

  

  Fidel Lim•2,775

  @fidellimPosted about 3 years ago

  @IshitaBisaria No worries :) Regarding the mobile width, you can still adjust it to anything other than 375px. If you would want to make your site available to most devices, it would be great to be made responsive. But I would assume you already know that hehe. Good luck with your other projects :)

  Marked as helpful

  0

  

  Ishita Bisaria•40

  @IshitaBisariaPosted about 3 years ago

  @fidellim Yes, Sure :)

  1
• 

  Muhammad Fadll•320

  @fadelunPosted about 3 years ago

  great, no issues detected

  Marked as helpful

  0

  ---

  

  Ishita Bisaria•40

  @IshitaBisariaPosted about 3 years ago

  @fadelun :)

  0

  

  Muhammad Fadll•320

  @fadelunPosted about 3 years ago

  hi @IshitaBisaria ,i have seen your css, why you write @media query min-width twice?

  Marked as helpful

  0

  

  Ishita Bisaria•40

  @IshitaBisariaPosted about 3 years ago

  Hi @fadelun , U can write all the styles of Media Query in a single media query too! I too have merges the media queries.

  0