Expense Chart Component (Chart-js + Tailwindcss)

Community feedback

• 

  Abdul Khaliq 🚀•72,580

  @0xabdulkhaliqPosted 6 months ago

  Hello there 👋. Congratulations on successfully completing the challenge! 🎉

  • I have a suggestion regarding your code that I believe will be of great interest to you.

  ---

  TAILWIND CLASSES 📐:

  • Your solution is well designed and matches the actual design image provided by FEM, Congrats for that. I'm here to address a small suggestion on min-h-[100vh] utility class.

  ---

  • You want min-height: 100vh right ? Then you need to use min-h-screen class instead of min-h-[100vh].

  ---

  • We don't need to create arbitrary tailwind utility class, because there's already a class for that.

  ---

  • Additionally, I want to mention the usage of h1 in your solution. It's not a good practice to use h1 for display balance.

  ---

  • An h1 heading provides an mportant navigation point for users of assistive technologies, allowing them to easily find the main content of the page.

  ---

  • So just replace the h1 with p or span, I would prefer to use span by nesting it's content with p and just style it like you need.

  ---

  • Example:

  <p class="text-sm">
     My balance
     <span class="font-[600] text-2xl block">$921.48</span>
  </p>
  

  ---

  • Now you want to add a level-one heading to improve accessibility, Don't worry you can add a visually hidden Heading adding a sr-only class to hide it from visual users (it will be useful for visually impaired users)

  ---

  • Example: <h1 class="sr-only">Expenses chart component</h1>

  ---

  .

  I hope you find this helpful 😄 Above all, the solution you submitted is great !

  Happy coding!

  Marked as helpful

  0

  ---

  

  I'mVeryZyla•500

  @RaulTindoganPosted 6 months ago

  @0xabdulkhalid Thank you for this! I always forgot the min-h-screen 😊. About the h1 tag I always find it hard to know what content I need to put inside the h1 tag just like here. I'll do this sr-only class next time when I don't know what content to put inside h1 tag.

  0
• 

  Korney Chervonenko•960

  @KorneyChervonenkoPosted 6 months ago

  Hello friend, There is a requirement in readme: “current day's bar highlighted in a different colour to the other bars”. How do you think “current day” is a some random day of week or it must be changed dynamically when page is loaded in browser? In other words “current day” is today ?

  1

  ---

  

  I'mVeryZyla•500

  @RaulTindoganPosted 6 months ago

  @KorneyChervonenko Yes! I think current day === today too so it will be dynamically changed based on what day is today. I didn't saw the readme haha, as you can see I followed the design and it is not changing base on the current day. For it, to changed the highlighted bar base on the current day. I think I will use Date methods and add some logical operations. Thanks!

  1