CSS Flexbox, HTML Div, HTML Basic, Max Width, Hoverstate, Positioning

Community feedback

• 

  Elir-Mahad•100

  @Elir-MahadPosted almost 3 years ago

  Hey Yu i looked over your code !

  It looks clean and not clunky at all.

  In your website: https://selfreflective.github.io/NFT-preview-card-component/

  There seems to be a horizontal and vertical scroll.

  This probably because of the height and width in your body.

  I played around with the css and i made some changes to body, .ntf-card, .boxshadow

  Implement the below changes to the css classes and let me know what you think.

  I identified the css properties that i removed by putting them under 'removed css below' comment.

  I identified the css properties that i added by putting them under 'added css below' comment.

  body {
    text-align: center;
    background-color: hsl(217, 54%, 11%);
    margin: 0;
    font-family: Outfit;
    
  /* removed css below  */
  /* width: 1440px; */
  /*  height: 900px; */
    
  }
  
  .ntf-card {
    border-radius: 10px;
    background-color: hsl(216, 50%, 16%);
    width: 340px;
  /* remove css below  */
  /* position: relative; */
  /* margin: 200px auto; */
    
  /* Added css below   */
    display:block;
    margin:auto;
    margin-top:2rem;
  }
  
  .boxshadow {
    display: flex;
    height: 300px;
    width: 300px;
    justify-content: center;
    align-items: center;
    position: absolute;
    border-radius: 11px;
    display: flex;
    justify-content: center;
    background-color: hsl(178, 100%, 50%);
    
  /* removed css below  */
  /* margin-bottom: 47px; */
  /* bottom: 234px; */
    
  }
  

  Also it would be useful, when you are writing out your code, to avoid using position relative and absolute unless its absolutely necessary.

  Try using flexbox and margins to move around the page.

  Thoughts ?

  Marked as helpful

  1

  ---

  

  Yu Dan•30

  @SelfReflectivePosted almost 3 years ago

  @Elir-Mahad Thank you sooo so much !! I learned alot from your solution! This is amazing. I didnt even think that someone would actually take a look at my code :)

  But i didnt need to add display: block; on the .ntf-card class right? Since the default Value of divs is block :)

  0

  

  Elir-Mahad•100

  @Elir-MahadPosted almost 3 years ago

  @SelfReflective Hey ! I am glad that you found my response useful. I'm trying to improve my code review skills so i enjoy this !

  In regards to your question, you can comment out the display:block and see if anything changes, if nothing changes, then you can remove it.

  Usually when i want to centre something, i use

  display:block;
   margin:auto;
  
  

  Keep up the good work !

  Marked as helpful

  0

  

  Yu Dan•30

  @SelfReflectivePosted almost 3 years ago

  Thanks for the answer @Mahad Mohamood! Jeah i have tried it after reading through your solution and it didnt change anything bad :) so i left it out!

  I understood every single piece your described! So keep up the great reviews!

  1

  

  Elir-Mahad•100

  @Elir-MahadPosted almost 3 years ago

  @SelfReflective nice thanks for the follow i followed you back . Keep up the good work !!

  0

  

  Yu Dan•30

  @SelfReflectivePosted almost 3 years ago

  @Elir-Mahad No worries! Looking forward exchanging knowledge with you Mohamood! Have a great Weekend

  1