Skip to content
OverviewSolutions
8
Comments
15
Alberto Ledesma
@ledesmx

All comments

  • Alberto Ledesma•370
    @ledesmxPosted 10 months ago

    Hi @aziz712 👋

    Great job on your solution!

    Here are some recommendations for you:

    • There is a better approach to try to center the card. You can use Flexbox along with justify-content: center; and align-items: center; to center anything. For this approach is necessary to set the height to 100% of the viewport, I suggest using min-height: 100vh; to achieve this, like so:
    body {
  display: flex;
  justify-content: center;
  align-items: center;
  min-height: 100vh; /*Add this*/
}
    • Make sure to remove the margin-top of the .box.
    • Lastly, I suggest adding a box-shadow to the .box to add a subtle pretty shadow surrounding the card.

    In summary, I suggest adding this code:

    body {
  min-height: 100vh;
}
.box {
  box-shadow: 0 5px 15px 10px rgba(0, 0, 0, .1);
}

    And removing this one:

    .box {
  margin-top: 10%;
}
@media only screen and (max-width: 375px) {
  body {
    margin-top: 110px;
    display: flex;
    flex-direction: row;
  }
}

    I hope this helps a little.

    Well done for the rest.

    Marked as helpful
    1
  • Juan Martín Rivas•190
    @JuanMartinRivasSubmitted 11 months ago

    ** Hi there, I am Juan Martín and this is my solution for the Interactive Rating Component.

    I had a rough time turning the buttons into perfect circles and I'm not sure if the result is the best possible. If anyone could answer this doubt for me it would be of great help.

    Also, I'm not really sure if the names I assign to my classes is the best, If anyone could guide me on this I'd be most grateful.

    Any other feedback is also welcome. Specially if it's constructive and specific.

    Interactive Rating Component

    1
    Alberto Ledesma•370
    @ledesmxPosted 11 months ago

    Hi Juan Martín Rivas 👋

    Great job on your solution!

    The rating component works pretty well.

    To make perfect circles there is a better approach. Here's what I would do:

    • First of all the padding will not help us in this case, so remove it.
    • Then fix the height and the width of the element to make a square. I suggest using the aspect-ratio property together with height/width. No problem if you use height and width.
    main .card .buttons-list .list-item > .rating-text {
//
  height: 3rem;
  aspect-ratio: 1/1;
}
    • This along with the border-radius: 50%; will make a perfect circle. But the numbers will not be centered. Lastly, to center the content you can use either flex or grid. I recommend using grid along with the place-content property.
    main .card .buttons-list .list-item > .rating-text {
  display: grid;
  place-content: center;
}

    In summary, I suggest adding this code:

    main .card .buttons-list .list-item > .rating-text {
  height: 3rem;
  aspect-ratio: 1/1;
  display: grid;
  place-content: center;
}

    And removing this:

    main .card .buttons-list .list-item > .rating-text {
  padding: .75em 1.25em;
  display: block;
}

    Regarding to your question about the classes' names I let you these links about BEM and SUIT CSS that I believe will be of great interest to you:

    https://en.bem.info/methodology/

    https://suitcss.github.io/

    I hope this helps a little.

    Well done for the rest.

    Marked as helpful
    2
  • Alberto Ledesma•370
    @ledesmxPosted about 1 year ago

    Hi @ashkan-zs 👋

    Great job on your solution!

    The rating component works pretty good.

    Here are some recommendations for you:

    • There is a better approach to change the width of the component based on the screen's width. Instead of using a media query, I suggest using width and max-width properties together. You can usemax-width to prevent the component from streching too much. See the example below.
    .Card_card__7Eml9 {
  width: 90%; 
  max-width: 420px; /*This will prevent from streching out more than 420px*/
}
    • I also suggest using transitions in your buttons to add a smooth transition while the background-color and the color change. Check out the MDN Web Docs to see how it works: https://developer.mozilla.org/en-US/docs/Web/CSS/transition
    .Ratings_rate-btn__bqd2Z {
  transition: background-color .2s, color .2s;
}
.RateForm_card__btn__wHc2p {
  transition: background-color .2s, color .2s;
}

    In summary I suggest adding this code:

    .Card_card__7Eml9 {
  width: 90%;
  max-width: 420px;
}
.Ratings_rate-btn__bqd2Z {
  transition: background-color .2s, color .2s;
}
.RateForm_card__btn__wHc2p {
  transition: background-color .2s, color .2s;
}

    And removing this:

    .Card_card__7Eml9 {
  width: 30%;
}
@media (max-width: 768px) {
  .Card_card__7Eml9 {
    width: 90%;
  }
}

    I hope this helps a little.

    Well done for the rest.

    Marked as helpful
    1
  • Alberto Ledesma•370
    @ledesmxPosted about 1 year ago

    Hi @akojha556 👋

    Great job on your solution!

    Here are some recommendations for you:

    • I suggest using flexbox and min-height to center your content. See the example below.
    .container {
  display: flex;
  flex-direction: column;
  align-items: center;
  font-family: "Roboto", sans-serif;
  text-align: center;
  padding: 5rem;

  /*Add the code below*/
  justify-content: center; /*This will center your content vertically*/
  min-height: 100vh; /*This sets the minimum height of the container to 100% of the viewport's height*/
}
    • Now a vertical overflow will appear. Remove the margin in the body to remove that overflow.
    body {
  background-color: #d5e1ef;

  /*Add the code below*/
  margin: 0;
}
    • Regardless your question. I suggest checking out the BEM's naming convention: https://getbem.com/

    I hope this helps a little.

    Well done for the rest.

    0
  • Alberto Ledesma•370
    @ledesmxPosted about 1 year ago

    Hi @AkramGalib 👋

    Great job on your solution!

    Here are some recommendations for you:

    • Everything looks quite well except for a vertical overflow. You can remove the margin of the body to make the overflow desappear.
    body {
  margin: 0;
}
    • For your next projects I suggest removing all the margin automatically added by the browser. Use the * selector to select everything and remove the margin with margin: 0;. This gives you more control over whether or not you want add margin on each element separately. You can do the same with padding, border and box-sizing. Look the example below:
    * {
    margin: 0;
    padding: 0;
    border: 0;
    box-sizing: border-box;
}

    I hope this helps a little.

    Well done for the rest.

    Marked as helpful
    0
  • Alberto Ledesma•370
    @ledesmxPosted about 1 year ago

    Hi Christian Ribeiro 👋

    Great job on your solution!

    Here are some recommendations for you:

    • I suggest using Flex or Grid to center the card. Check out the MDN Web Docs to see how it works: https://developer.mozilla.org/en-US/docs/Learn/CSS/CSS_layout/Flexbox
    body {
  display: flex; /*Here you set this element as a Flex Container*/
  align-items: center; /*This center the content inside itself vertically*/
  justify-content: center; /*This center the content inside itself horizontally*/
  min-height: 100vh; /*This sets the body's minimum height as 100% of the viewport's height*/
}
/*Now the card's margin is no longer required*/
.card {
  margin: auto; /*Remove this*/
}
    • It is a good practice to remove all the added margin automatically. Use the * selector to select everything and remove the margin with margin: 0;. This gives you more control over whether or not you want add margin on each element separately.
    • It is also good practice to use percentage units (%) instead of viewport units (vh and vw). Only use vh and vw if you don't have other option.
    • By default the <div> element is a block element. It is no required to specify display: block; in the .card. I woud remove it as well.
    • Lastly, when you want to give to an element the width based on the width of something else (in this case de screen's width) it is a good approach to use the width property along with either max-width and min-width properties. See the following example:
    .card {
  width: 80%; /*This sets the card's width to 80% of the screen's width*/
  max-width: 350px; /*This will prevent the card from stretching out more than 350px*/
}

    In summary I would add this code:

    * {
    margin: 0;
}
body {
  display: flex;
  align-items: center;
  justify-content: center;
  min-height: 100vh;
}
.card {
  width: 80%;
  max-width: 350px;
  padding: 13px;
}
img {
  width: 100%;
}

    And remove this:

    .card {
  display: block;
  margin: auto;
  width: 30vw;
  height: 70vh;
}
img {
  width: 28vw;
  margin: 13px;
}

    I hope this helps a little.

    Well done for the rest.

    Marked as helpful
    0
  • Alberto Ledesma•370
    @ledesmxPosted about 1 year ago

    Hi Chloe O 👋

    Great job on your solution!

    Everything works quite well. I love the star as the site's icon and the message when you try to submit without selecting a rate.

    Here are some recommendations for you:

    • I suggest using Flex or Grid to center the content instead of setting the position absolute. Only use position: absolute; if you want an element positioned in a weird out-of-DOM flow position.

    I would add this code:

    body {
  min-height: 100vh; /*Sets the minimum height to 100% of the viewport's height*/
  display: flex;
  align-items: center; /*Centers the content vertically*/
  justify-content: center; /*Centers the content horizontally*/
}

    And remove this:

    body #container {
  position: absolute;
  top: 0;
  bottom: 0;
  left: 0;
  right: 0;
  margin: auto;
}
    • Lastly, I suggest adding a radial gradient instead of the linear gradient. It fits better to the design image. Check out the MDN Web Docs to see how it works: https://developer.mozilla.org/en-US/docs/Web/CSS/gradient/radial-gradient

    You can add this code:

    body #container {
  background: radial-gradient(circle at top, rgb(37, 45, 55) 8%, rgb(25, 29, 34) 95%);
}

    I hope this helps a little.

    Well done for the rest.

    1
  • Alberto Ledesma•370
    @ledesmxPosted about 1 year ago

    Hi Ana 👋

    Great job on your solution!

    Here are some recommendations for you:

    • I suggest using the min-height property instead of height. If we set height: 100vh; then the body will have 100vh height no matter what. Even if the content spans more than 100vh of viewport (this will result in the component being cut off on smaller screens.) Whereas with min-height it will continue growing.
    • Lastly, there is a margin in your body which is adding more space in addition to the 100vh. I only suggest removing it.

    Code to remove:

    body {
  height: calc(100vh - 1px);
  margin: 1.25rem;
}

    Code to add:

    body {
  min-height: 100vh;
}

    I hope this helps a little.

    Well done for the rest.

    Marked as helpful
    0
  • Alberto Ledesma•370
    @ledesmxPosted about 1 year ago

    Hi @RAHUL-19OOPS 👋

    Great job on your solution!

    Here are some recommendations for you:

    • I suggest adding a max-width property in the card to prevent stretching. This sets the maximum width on an element.
    • Also add the radial-gradient() function in the card's background property to achieve that gradient background. Check out the MDN Web Docs to see how it works: https://developer.mozilla.org/en-US/docs/Web/CSS/gradient/radial-gradient

    Code to add:

    .main-container, .thank-you {
  background: radial-gradient(ellipse at top, hsl(213, 20%, 22%), hsl(212, 28%, 12%));
  max-width: 400px; /*Here you add the width you want*/
}

    I hope this helps a little.

    Well done for the rest.

    Marked as helpful
    0
  • mcstarley1215•110
    @mcstarley1215Submitted about 1 year ago

    Still trying to figure out flexbox. Flexbox What is the best practice on mastering flexboxes?

    QR code component

    1
    Alberto Ledesma•370
    @ledesmxPosted about 1 year ago

    Hi @mcstarley1215 👋

    Great job on your solution!

    You almost have it centered. The body takes the height of the content inside itself like its own height. Meaning when you try to center its content vertically it doesn't work unless you explicitly set the body's height. Fortunately, it's pretty straightforward to do it.

    You only need to add the following code for the body tag:

    body {
  min-height: 100vh;
}

    It sets the minimum height of the body tag for the 100% of the viewport's height. Now your content will be centered vertically.

    Regarding to you question I suggest checking out this game about Flexbox: https://geddski.teachable.com/p/flexbox-zombies

    And practice a lot what you learn with projects like this.

    I hope this helps a little.

    Marked as helpful
    1
  • Alberto Ledesma•370
    @ledesmxPosted about 1 year ago

    Hi Yasmin 👋

    Great job on your solution!

    Here are some recommendations for you:

    • I found that there are two background images when the screen's width is shorter than 650px. Because the background images are rendered in two different elements (body and .container). I suggest removing one of them:
    @media (max-width: 650px) {
  .container {
    background-image: url(images/bg-intro-mobile.png); /*Remove this*/
  }
}
    • Whether you want to render one background or the other. You can add the media query for the body tag. Like this:
    body {
  background-image: url(images/bg-intro-desktop.png);
}
@media (max-width: 650px) { /*Add this media query*/
  body {
    background-image: url(images/bg-intro-mobile.png);
  }
}

    I hope this helps a little.

    Well done for the rest.

    0
  • Alberto Ledesma•370
    @ledesmxPosted about 1 year ago

    Hi Ahmed Farouk 👋

    Great job on your solution!

    In this challenge, there is a subtle shadow behind the card.

    I suggest using the box-shadow property to apply that shadow.

    You only need to add this code:

    #card {
  box-shadow: 0 15px 15px 5px rgba(0, 0, 0, 0.1);
}

    I also recommend you to check out the MDN Web Docs about the box-shadow: https://developer.mozilla.org/en-US/docs/Web/CSS/box-shadow

    Well done for the rest.

    Marked as helpful
    1