Insure Landing with flexbox, css grid

Community feedback

• 

  Adarsh Pratap•5,515

  @adarshcodesPosted over 4 years ago

  Hi! @Sarah_Hassan, you and your friends did a great job on this project👍. As you say you've been allotted the nav and header section, everything works fine there, you did a fine use of Z-index property and also the Responsiveness works great. Overall everything is great🙂. You can solve the HTML Issues raised.

  2

  ---

  

  Sarah Hassan•145

  @sarah27hPosted over 4 years ago

  Hi @adarshcodes, thanks for your feedback, I will try to solve the HTML issue.

  0
• 

  Diarrah•3,418

  @DiarrahPosted over 4 years ago

  This comment was deleted over 4 years ago

  1

  ---

  

  Sarah Hassan•145

  @sarah27hPosted over 4 years ago

  Hi @Diarrah, thanks. I put the image first so I can get it first then the intro text for small screens. And for large screens, I used row-reverse to get image on the right and text on the left.

  Which pattern did you mean the left one or the right one?

  0

  

  Diarrah•3,418

  @DiarrahPosted over 4 years ago

  @sarah27h I was able to get the left svg on perfectly just not the right portion.

  0
• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 4 years ago

  Hey Sarah, great work on this project! It's awesome to see that you collaborated with others on this one.

  One small heads up about the .main__how section (I know this wasn't your job, but you could relay the feedback to your teammates 🙂). height: 40vh; is being used to set the height of this element. This is what's throwing off the screenshot, but it's also not a very good practice to use. The vh unit should only be used in very specific circumstances, as it can lead to unintended layout issues. As an example, open your project, keep the screen at desktop size but then reduce the height of the browser. You'll start to see that section shrink and the text will start to overflow. Instead, using combinations of margin and padding to space elements inside and avoid setting an explicit height on the parent. This way the height of the parent is dictated by the height of the content inside it, which is a good thing.

  Also, a lot of IDs have been used for the CSS selectors. I would recommend avoiding IDs as CSS selectors. They have high specificity and can't be reused on the page, so they're not good for the purpose of styling. Instead, I'd recommend sticking to class, attribute, pseudo, and type selectors. Using these will help keep your CSS more maintainable.

  I hope that all makes sense and I've explained OK. Keep up the great work! 🙂

  1

  ---

  

  Sarah Hassan•145

  @sarah27hPosted over 4 years ago

  Hey @mattstuddert, thanks so much for this very useful feedback, I will relay the feedback to my teammates.

  I have a question about using IDs, I used them to connect those sections with the navbar links. Is there another alternative to that?

  What if I have 3 close buttons for 3 divs and I need to select them in js and when clicked on button 1 close div 1 and so on.

  <button class="fas option-close-btn btn fa-chevron-circle-up"></button>
  
  

  select them using

  this.closeOption = document.getElementsByClassName('option-close-btn')[0];
  this.closeOption = document.getElementsByClassName('option-close-btn')[1];
  this.closeOption = document.getElementsByClassName('option-close-btn')[2];
  

  Is true or I need to use ID for every one or using one class?

  0

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 4 years ago

  @sarah27h you're welcome! For singling out an element and selecting in JavaScript IDs are absolutely fine and are actually very useful. It's just using IDs as selectors in your CSS which isn't a recommended practice.

  1

  

  Sarah Hassan•145

  @sarah27hPosted over 4 years ago

  @mattstuddert thanks so much

  You mean for example to add a class for styling purposes and use IDs in js

  <nav id="main-nav"  class="main-nav hide">
          <ul>
            .
            .
          </ul>
  </nav>
  
  .main-nav {
   .
   .
  }
  

  0

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 4 years ago

  @sarah27h yeah, exactly. That’s the way I’d recommend doing it 👍

  1