Responsive and functional E-commerce page

Community feedback

• 

  P

  Jo89 😈•510

  @AhmadYousif89

  Posted over 2 years ago

  Hi Gabrial 👋

  Your solution looks beautiful 👍 but I just noticed the following issues with your app and maybe fixing these issues will make your app even more appealing to the users :

  • I noticed that the backdrop in the lightbox is not the highest element on the DOM in term of z-index (usually the backdrop on any modal should have the highest z-index) and therefor user can click and interact with element beneath it and of course this is not good behavior.

  • because of the first issue I can open the cart and then open the lightbox and see that the cart is actually visible and popping through the backdrop.

  • would be a good user experience if I can open the cart and close it with just clicking away (anywhere on the page)

  • try adding a line-through the original price indicating a discount has happened to it and that it now has a discounted price (this is just a minor visual change) but it adds to the whole user experience (IMO)

  • would be good if I can close the side menu by clicking on the backdrop (and again I see the same problem as mentioned in the first issue)

  you have a nice day 👍

  110

  Marked as helpful
• 

  S MD suleman•3,510

  @sulemaan7070

  Posted over 2 years ago

  hey 😄Gabriel Gusso, congratulations on completing the challenge... here are a few tips to make your site better.

  1.Firstly the logo it seems you have used regular text...you can find the logo here frontendmentor-ecommerce/src/assets/svg/logo.svg

  2.It seems you are using 100% of the width for the nav.. to make it design accurate you use 75% to 80% of the width and margin auto on the right and left..

  3.The button seems to be missing the box-shadow which would make the button cooler..

  Everything else is great!! happy coding💯😄👍🏻

  11

  Marked as helpful
• 

  Abdul Khaliq 🚀•72,380

  @0xabdulkhaliq

  Posted over 2 years ago

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

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

  HTML 🏷️:

  • This solution generates accessibility error reports due to non-semantic markup, which lack landmark for a webpage

  ---

  • So fix it by wrapping the whole content inside the semantic element <main> in your index.html file to improve accessibility and organization of your page.

  ---

  • What is meant by landmark ?, They used to define major sections of your page instead of relying on generic elements like <div> or <span>

  ---

  • They convey the structure of your page. For example, the <main> element should include all content directly related to the page's main idea, so there should only be one per page

  ---

  HEADINGS ⚠️:

  • And, This solution had also generated another accessibility error report due to skipping heading levels

  ---

  • We want to avoid skipping heading levels, make sure to start with <h1> and working your way down the heading levels (<h2>, <h3>, etc.) helps ensure that our document has a clear and consistent hierarchy. Read more 📚

  ---

  • Because skipping heading levels is a poor practice from the perspective of information design, whether we are talking about web pages, books, journal articles, or about anything else. You can not only confuse screen readers but all readers when you don't follow a consistent, logical pattern with your heading structure.

  ---

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

  Happy coding!

  11

  Marked as helpful