Order summary - responsive website

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, great work on this one. The layout in general looks fine to me, but when opening up dev tools at the bottom, the wavy-background-image suddenly changes size. The site is almost responsive because at some point like 380px width, the layout is being hidden by the screen, creating a horizontal scroll. On layout like 350px, the text is almost squished and the bottom section on the pricing, the music-icon is overlapping its right-side element.

  Diego Serrano already gave feedback on this one, just going to add some suggestions as well:

  • It would be great to have a base styling of this:

  html {
    box-sizing: border-boxl
    font-size: 100%;
  }
  
  *,
  *::before,
  *::after {
    box-sizing: inherit
  }
  

  This way, handling an element specially its size will be easier because of the box-sizing

  • Instead of using background-size: contain use background-size: 100% so that it will fill up all the spaces properly.
  • body tag does not need font-size: 16px since by default it uses that value.
  • Avoid using height: 100vh on a large container like the .box as this makes the element's height capped based on the viewport/screen's height. Instead use min-height: 100vh so that the element will expand if it needs to.
  • To make the layout more adaptive, in general, instead of using width use max-width to limit a size of an element. This way, even if the size gets lower than the size on the max-width the layout will respond on it since it doesn't have a fixed width. For example, use max-width: 450px on the .box-grid and use width: 100% on the img inside it. You will see the different if you change the screen-size via dev tools.
  • The vector/illustration is only decorative on this one. Decorative images should be hidden for screen-reader at all times by using alt="" and aria-hidden="true" to the img tag or only aria-hidden="true" if the image is using svg.
  • Only use descriptive alt on images that are meaningful and adds content to the site otherwise hide the image for screen-reader users.
  • The music-icon is only decorative as well so it will be better if it was hidden as well.
  • Also, when using img tag, you don't need to add words that relates to "graphic" such as "icon" and others, since img is already an image so no need to describe it as one.
  • The annual-plan text could have used a heading tag like h2 since it gives information on what the section would contain, hence the pricing of the plan.
  • You can just use a tag directly or use div to contain and instead of using p tag.
  • You don't need to use role="navigation" on the 2-buttons since those aren't really navigational links. Since you used button on this one, I assumed you though this component as a form where button is used. So maybe it will be better if you use button as well on the change but, only if you will wrap the layout inside of a form.
  • Lastly, just addressing the layout issue on like 350px size below:>

  Aside from those, great job again on this one.

  Marked as helpful

  0

  ---

  

  Michael•150

  @mikkoolPosted almost 3 years ago

  @pikamart First of all, I am so thankful for this comment. Just so you know that the first time I joined this community you are my inspiration. Learning html, css and javascript for almost two months now. I am really having fun and I don't know how I end up to your feed but I have seen your works and I said wow. You're amazing and finding out that you too are filipino. Makes a win win for me :) Thank you for this helpful comment. I always wanted to ask you to be my mentor but I am scared to ask. Lol. But this, this comment! Super! Super made me happy.

  Thank you so much! Really, really appreciate it :)

  1

  

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  @mikkool Glad that you find the feedback useful and there are only few Filipinos that I encountered as well ( but we'll just stick to using english so no excluding:> ) including you.

  Thank you as well for my works, but do not take notes of other solutions that I made since I haven't refactored those old ones, I've been refactoring others but can't find a dedicated time for it.

  Also, I'll give you a follow now so that I could see your other submission as well and I could review it as well. Goodluck for every challenges^^

  Marked as helpful

  0

  

  Michael•150

  @mikkoolPosted almost 3 years ago

  @pikamart Thank you so much :) I really appreciate it. I am really new at this :) Not sure if I am doing the right thing. Lol. Thank you!

  1
• 

  P

  Diego Serrano•180

  @dserranoiPosted almost 3 years ago

  Hey @mikkool - cool page. 1 comment if you don't mind. 1.- Your repository is not found. .

  Marked as helpful

  0

  ---

  

  Michael•150

  @mikkoolPosted almost 3 years ago

  @dserranoi Thank you for checking out, I don't know why it says repository not found. I linked the right one. Just in case here: https://github.com/mikkool/order-summary-component-main

  Thank you so much!

  0

  

  P

  Diego Serrano•180

  @dserranoiPosted almost 3 years ago

  @mikkool Still not able to see it, did you set it up as private?

  Marked as helpful

  0

  

  Michael•150

  @mikkoolPosted almost 3 years ago

  @dserranoi Oh. My bad, yes it was in private. It's public now :) Thank you!

  0

  

  P

  Diego Serrano•180

  @dserranoiPosted almost 3 years ago

  @mikkool awesome i love your code, nice!

  Marked as helpful

  0

  

  Michael•150

  @mikkoolPosted almost 3 years ago

  @dserranoi Thank you, hope that helps, I am learning as well. Good luck to our journey :)

  1