Skip to content
OverviewSolutions
8
Comments
15

Alberto Ledesma

@ledesmxMéxico370 points

Hi! I'm Alberto. I love translating beautiful graphic designs into web pages. I'm passionate about web development, especially responsive web design, and animation.

I’m currently learning...

🔹 TypeScript 🔹 Node.js 🔹 Next.js 🔹 Design 🔹 Svelte 🔹 Godot

Latest solutions

View more solutions

Latest comments

  • Alberto Ledesma•370
    @ledesmx

    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
  • Alberto Ledesma•370
    @ledesmx

    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
  • Alberto Ledesma•370
    @ledesmx

    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
  • Alberto Ledesma•370
    @ledesmx

    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.

  • Alberto Ledesma•370
    @ledesmx

    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
  • Alberto Ledesma•370
    @ledesmx

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