Sneakers E-commerce Page (HTML, SCSS, and Vanilla JS)

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

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

  Just some suggestions besides others feedback on this one:

  • Your markup placement on the header is somehow confusing. You put first the cart before the nav which should not be. Make the nav first before the cart^^.
  • Also there are 2 extra button on the header which currently can confuse the user. Try tabbing on the desktop layout on the navbar, you will find it focusing on something. If you are trying to hide an element, use display: none or visibility: hidden.
  • Your cart button should have either aria-label attribute or screen-reader text inside it, giving information on what the button does like "shopping cart dropdown". It should be using also an aria-expanded` attribute.
  • svg inside the cart is just decorative so hide it using aria-hidden="true" attribute on the svg.
  • Your cart dropdown should be placed lower, since right now it blocks the cart-button when it is showing and it is hard to toggle it back.
  • The image as well for the user should be using either a button if it should act as a control for something like dropdown or a link a tag if it is a page for the user.
  • I would give an alt the hero-image since the section is all about the image so making it visible would be great.
  • The minus and add button should have the same as the cart-button about aria-label or screen-reader text.
  • Also there's a bug, when you click on the img it shows a modal but after closing it, a border remains on the screen.
  • I would use radio buttons for the selections of images below the sneaker and each img would be like a background-image for the label for each radio button.

  OPENING AN IMAGE

  • The close-button should as well use the method I mentioned above, it could use "close modal" as text-value.
  • Also try to inspect the layout in dev tools at the bottom while the image-modal is present. You will see that it doesn't really occupy the whole screen and it just leaves a bug on the site.
  • The next and previous buttons as well should be using the method.

  To be honest, this challenge is hard to make really accessible or just e-commerce in general since there are lots of things here and there but being aware of this kind is much better than just progressing without knowing. I reviewed a solution on this before and it was really great though I can't share it since it is hard to find that review that I made :>

  But still, great job again on this one.

  Marked as helpful

  1

  ---

  

  Danielle S.•140

  @dstrickl7Posted about 3 years ago

  @pikamart Thank you for your super in-depth feedback! I really appreciate it!

  A lot of the issues you pointed out are due to my ignorance of WAI-ARIA. I will read up on it and fix those issues. I'm currently looking into the button issue and it appears to be my mobile open and close buttons. This is also why my navbar is set up the way it is (I code mobile first). I'm working out why the mobile buttons are being registered even though I have them set to hidden. I've been using the same navbar technique since I first learned, so it might be time to work on a new way to make it flow better.

  I'm confused about the bug you're talking about. I don't see an outline on my screen when the modal closes. What screen size and browser is it appearing on? Or are you talking about the border around the image thumbnail?

  I like your idea about using radio buttons with a background image for the modal thumbnails. I never even considered something like that. Brilliant!

  0

  

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  @dstrickl7 Hey, glad that you find it useful^^

  About the bug, if you try clicking the image of the sneaker itself, not the 4 selection, the actual image itself, the one that is big. After that, there is this huge border that on the screen that's not being removed. I am using chrome, 1366x768 screen^^

  Marked as helpful

  1

  

  Danielle S.•140

  @dstrickl7Posted about 3 years ago

  @pikamart OH, thank you so much. That definitely will get fixed.

  1
• Account deleted

  This comment was deleted about 3 years ago

  1

  ---

  

  Danielle S.•140

  @dstrickl7Posted about 3 years ago

  @adrianerbod Thank you so much for your detailed and helpful feedback. I feel so silly for completely ignoring fixed positioning. It even fixed my too short overlays. I originally tried putting a border and using a lower opacity on the lightbox thumbnail images, but it made them darker (because my background is black). The design file wanted them to have a white tint when hovered.

  Thank you for telling me about the notification. I just now realized I never checked how it looked on screens >1440px. I'm curious why that bug appears after 1442px.

  I added the h2 in the articles to be in line with the accessibility requirements, but now I'm debating whether I should have even used articles.

  Thank you for your advice on interview prep too. I had interviews at 2 companies and after the 2nd and 1st interview, I never heard back and it really shook my confidence.

  If you don't mind, could you tell me what makes the project appear rushed? I would like to avoid doing it in the future.

  0