Base Apparel Page - scss and js

Community feedback

• 

  Kamasah-Dickson•5,590

  @Kamasah-DicksonPosted about 3 years ago

  Awesome but check there is something wrong with your media quaries.....You can choose to make a breakpoint at 768px min width.

  0
• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted almost 5 years ago

  Hey Nitya,

  Awesome work on this challenge, it's great to hear you had fun building it! I like the custom states that you've added. They look great.

  Just a quick not on some of the styles you've set on the html and body elements. You don't need width: 100vw on either as they're block-level elements and will stretch the full width of the browser by default. Also, I would remove the height: 100vh. It's not needed, as you've already got min-height: 100vh on both. In this situation, min-height is preferrable as you want to make sure the screen can scroll if the user had a small viewport compared with the content height.

  With regards to making it DRYer, you're already using mixins nicely and reusable classes, so you've done a great job. On larger projects, you could start looking at different file architectures, like ITCSS or SMACSS. They're both great ways of structuring your code.

  I've noticed you've been giving some amazing feedback to others on here lately. Thanks a lot for getting involved in the feedback side. Your contributions have been really valuable! 🙂

  1

  ---

  

  P

  Nitya Gulati•665

  @nityagulatiPosted almost 5 years ago

  @mattstuddert Thanks for the tips, Matt. I read up on ITCSS and SMACSS and I'm going to try and incorporate them in the next challenges and see which one I like better!

  I'm always happy to share and engage with the community here :)

  0

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted almost 5 years ago

  @nityagulati great! I look forward to seeing your next solutions 🙂

  0