Philip Droubi

Latest comments

• 

  Omar Zaghlouleh•150

  @OmarZaghloulehSubmitted over 1 year ago

  

  Product preview card component - Solution

  #animation#fresh

  1

  50
  

  Philip Droubi•585

  @Philip-DroubiPosted over 1 year ago

  Congratulation on completing this challenge! 🎉

  The solution looks really good, you did a really great job as first challenge.

  Keep coding!👨🏻‍💻

  2
• 

  rkrawczyszyn•10

  @rkrawczyszynSubmitted over 1 year ago

  

  interactive-rating-component - React App with SCSS styles

  #react#sass/scss

  2

  30
  

  Philip Droubi•585

  @Philip-DroubiPosted over 1 year ago

  Congratulation on completing this challenge! 🎉

  This is a small tip you can do to improve your solution.

  • For the page body just add this:

      min-height: 100vh;
      display: flex;
      align-items: center;
      justify-content: center;
  

  Doing This will center the rating component for all screens.

  -In background div remove the margin cause now we don't need it:

  .background {
      /* margin-left: 50px; */
      /* margin-top: 50px; */
  }
  

  • Give the .container some more max-width, let's say 380px or 400px would be great.

  • Finally, using js remove the old component when moving to the second screen by setting the display to none, or removing the element by .remove() js function and replace it with new one or by changing the inner-html ...etc, In fact, you have many ways to do this.

  Hope you find this helpful😀

  0
• 

  Eni•180

  @EnidaShehuSubmitted almost 2 years ago

  

  NFT Preview Card Component - Solution

  1

  120
  

  Philip Droubi•585

  @Philip-DroubiPosted almost 2 years ago

  Congratulation on completing this challenge.

  This is a small tip you can do to improve your solution.

  For the page body just add this.

  background-color: hsl(217, 54%, 11%);
  min-height: 100vh;
  display: flex;
  align-items: center;
  justify-content: center;
  flex-direction: column;
  

  That will center page elements for all screens.

  Hope this helps you :)

  Marked as helpful

  0
• 

  Muyiwa•250

  @muyiwa99Submitted almost 2 years ago

  

  Rating Component page

  #sass/scss

  1

  60
  

  Philip Droubi•585

  @Philip-DroubiPosted almost 2 years ago

  Congratulation on completing this challenge.

  Here are some tips you can do to improve your solution.

  • First for the page body it's better to replace the margin with padding .WHY? To not get that annoying page scroll.

  body{
  /*margin:1.5rem*/ 
  padding: 1.5rem;
  }
  

  • For the main-container doing this will make it look better on all screens.

  body .main-container, body .thank-you {
  width: min(90%, 390px);
  min-width: 200px;                  
  }
  

  • For the h1 inside the main-container I think adding some margin will make it look better.

  body .main-container h1 {
  margin: 18px 0 10px;               
  }
  

  • Finally, don't forget to add validation when clicking on the submit button, as I can click it without setting a value.

  Hope this helps you :)

  Marked as helpful

  3
• 

  Eric Jan Estiva•230

  @oceanotrashSubmitted almost 2 years ago

  

  Notification page with flex and js

  1

  30
  

  Philip Droubi•585

  @Philip-DroubiPosted almost 2 years ago

  Congratulation on completing this challenge.

  Here are some tips you can do to improve your solution.

  First for the page body it's better to add this code.

  body {
      font-family: "Plus Jakarta Sans", sans-serif;
      background-color: hsl(210, 60%, 98%);
      font-size: 16px;
      color: hsl(219, 12%, 42%);
      min-height: 100vh; /* To take the full page height on all screens*/
      display: flex;     /* To center the main element */
      align-items: center;
      justify-content: center;
  }
  

  For main or .container you need to delete the margin margin: 80px auto; also delete it from the 600px media query margin: 20px auto;

  Now for the .wrapper I think by doing this it will look better :

  .wrapper {
      background-color: white;
      padding: 40px 30px 0;
      margin: 0 auto;
      max-width: 800px;       /*So that the element does not appear too wide on the screen */
      width: 92%;             /*To fit small screens*/
      max-height: 90vh;       /* To be shorter than the height of the screen */ 
      overflow-y: scroll;     /* To show all elements in the wrapper */ 
      border-radius: 15px;
      box-shadow: 0px 0px 26px -13px rgba(0, 0, 0, 0.26);
  }
  

  Also delete margin: 10px; from .wrapper 600px media query.

  If you don't like the default scrollbar, I recommend this :

  .wrapper::-webkit-scrollbar {
      width: 8px;
      background: lightgray;
      border-radius: 10px;
  }
  
  .wrapper::-webkit-scrollbar-thumb {
      background: gray;
      border-radius: 8px;
  }
  

  Hope this helps you :)

  Marked as helpful

  0
• 

  Volodymyr Ostapyshyn•490

  @ostapyshynSubmitted almost 2 years ago

  

  GitHub user search app

  #react#typescript#vite#sass/scss

  1

  81
  

  Philip Droubi•585

  @Philip-DroubiPosted almost 2 years ago

  Congratulation on completing this challenge.

  Here are some tips you can do to improve your solution flexibility.

  First for the App I suggest you do this:

  .App{
    height: fit-content;
    min-height: 100vh;
    padding: 28px;
  }
  

  Doing this will make the background cover the page instead off 100vh only.

  For smaller screens, you can add this so that the link is no wider than the user card

  ._link_ds8a2_385{
      word-break: break-word;
  }
  

  Hope this helps you :)

  Marked as helpful

  1