Loopstudios landing page

Community feedback

• 

  Andrey•2,570

  @dar-juPosted about 1 month ago

  Hi Yacoub AlDweik!

  Great job!

  If you don't mind, I'll add a little bit from myself:

  • your close cross for the mobile menu didn't load, because of this there is no way to close menu
  • for the menu and social icons you use ul/li but for "Our creations" - div, I think you need ul there too
  • your keyboard accessibility works, that's cool! but for better readability there is should be aria-label for social icons
  • at the end of development it is necessary to do an overflow test for blocks and for text. For example, the "Our creations" block passes the test, everything is fine here. But the menu items and social network items - do not pass. Why is necessary to do test? Imagine that the customer wants to add new menu items, then they will start to go beyond the block boundaries. Here's how to fix it (the top menu for example): remove the margin-right from li, add the code for ul:

  flex-wrap: wrap;
  justify-content: flex-end;
  gap: calc(calc(var(--spacing)* 6)* calc(1 - var(--tw-space-x-reverse)));
  

  In this case, no matter how many menu items you add, the layout will always be stable.

  Everything else is great, good luck with your development!

  Marked as helpful

  0

  ---

  

  Yacoub AlDweik•2,850

  @YacoubDweikPosted about 1 month ago

  @dar-ju Yeah I got it! You have no idea how much I love these kind of feedback cuz this is what improves my skills, I can't thank you enough Andrey!

  0

  

  Yacoub AlDweik•2,850

  @YacoubDweikPosted about 1 month ago

  @dar-ju Talking about your last point, don't you think that flex-wrap: wrap would be enough to do that? also why all this complex calculations? instead of calc(var(--spacing)* 6): I can just say gap-6 || calc(1 - var(--tw-space-x-reverse)): I don't see any benefit of using this to handle the rtl and ltr changes, bcuz I can't see why we justify-content to the end, I mean center would be a better idea, what do you think?

  0

  

  Andrey•2,570

  @dar-juPosted about 1 month ago

  @YacoubDweik Of course, wrap and gap is enough. justify-content it's a matter of taste. Calculations I took from margins of your li ) I think it is calculations of tailwind

  Marked as helpful

  0
• 

  Karol•3,100

  @VillageR88Posted about 1 month ago

  You're ready for the job market!

  Just be sure to check the console for any errors.

  Marked as helpful

  0

  ---

  

  Yacoub AlDweik•2,850

  @YacoubDweikPosted about 1 month ago

  @VillageR88 Will do! Thanks for your supporting Karol!

  0