Skip to content
OverviewSolutions
31
Comments
7

Philip Droubi

@Philip-DroubiDamascus, Syria585 points

Hi 👋 I'm 23 years old programmer who enjoys creating responsive web designs and back-end restfull APIs. I am a Software Engineering graduated from the Faculty of Information Technology Engineering, Damascus University.

I’m currently learning...

React.js

Latest solutions

View more solutions

Latest comments

  • Philip Droubi•585
    @Philip-Droubi

    Congratulation on completing this challenge! 🎉

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

    Keep coding!👨🏻‍💻

  • Philip Droubi•585
    @Philip-Droubi

    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😀

  • Philip Droubi•585
    @Philip-Droubi

    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
  • Philip Droubi•585
    @Philip-Droubi

    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
  • Philip Droubi•585
    @Philip-Droubi

    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
  • Philip Droubi•585
    @Philip-Droubi

    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
View more comments