Order Summary Component - SCSS. Mobile-First. BEM. Semantic HTML.

Community feedback

• 

  maia•300

  @maiaflowPosted over 2 years ago

  hey Eric!

  i wanted to ask a question about how you determined the size to make your box. maybe this is getting too granular- ultimately mine looks right for the most part. but something i struggled with in this challenge was matching between the screenshot and my work. looking now, i think the screenshot still looks smaller than my submission, but i know my submission is smaller in height and width than the example- in the frontendmentor screenshot comparison, where yours looks perfect, mine looks off. if i remember correctly, making mine 450px made it too short, so i felt like my width worked well to maintain the proportions. but your proportions look good!

  i didn't notice this before, but when i went to write this, i was looking on my laptop screen vs my larger monitor, and on this screen i noticed that mine cuts off around 590px height and yours cuts off around 700px height, which is a little more than the max height of my browser on this screen with the bookmark bar on. is there a best practice for a height below which you can assume most browsers won't be? i don't know if this factored into your thinking about calculating the size of the box. i guess the best way to do it is to just define a max-height and let the height be what it is?

  aside from my long-winded question about box size, i wanted to call out how you did your hero image. i didn't think of just adding a border-top-[left/right]-radius to the image, i thought to get the rounded corners i would have to make it a background-image with padding, which caused me issues with my spacing below that i can see you don't have- if i change your max-width in inspect it stays spaced well, not the same for me lol! much cleaner how you did it. love it!

  also wanted to shout out your semantic naming for your sections (main for div.box-wrap, article for div.box) looks really nice. i'm going to try that for my next submission.

  thanks for teaching by doing! :D

  Marked as helpful

  0

  ---

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  I would recommend focusing on the code quality instead of trying to make your solution pixel-perfect. There's a great blog post by Josh Comeau about pixel-perfect dreams. So, I suggest trying to make your solution looks similar with the original.

  The card is getting cut off because the height of the body element is limited to 100vh of the user's screen height. So, I would recommend never setting a height on the body element. If you ever need to set a height then use min-height instead.

  For the border-radius, I would recommend setting an overflow: hidden to the card element. This way, you can remove the border radius properties from the image. As a result, the image will obey the border radius of the parent element.

  Hope this helps both of you, @ericsalvi and @maiaflow! 😁

  Marked as helpful

  2

  

  Eric Salvi•1,330

  @ericsalviPosted over 2 years ago

  @vanzasetia Thank you for the suggestions. For these components, I tend to add a height to the body when using grid to get things vertically centered. I may have actually used min-height in the past and have since forgotten that. But min-height is the way to go. Thank you for that.

  As for pixel perfection, I like to try and challenge myself personally to get the results as close to the original design as possible. Mine personally is not pixel-perfect if you look close enough with the slider.

  Lastly, setting the overflow: hidden on the container and removing the border-radius on the images does the trick and it is 1 less line of CSS. Sweet!

  2

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @ericsalvi You're welcome! 👍

  1
• 

  Yui•50

  @yteraiPosted over 2 years ago

  Great job, my sensei Eric! I love how you organize the files and I'll refer to your Sass for the next time I use Sass :)

  0