Skip to content
OverviewSolutions
13
Comments
11
Zubair Adham
@atmahana

All comments

  • Zubair Adham•400
    @atmahana

    Hi there, congrats on starting the journey on this field. For each summary category I would suggest to use flexbox for layouting. Here's how:

    <div class="reaction" 
  style="
          display: flex; 
          justify-content: space-between; 
          align-items: center
   ">
    <div style="display: flex;">
        <img src="URL">
        <p>Reaction</p>  
    </div>
    <div>
        <div class="test-score"> 80 </div>
        <div class="test-percent"> / 100 </div>
    </div>
</div>

    I divide the reaction children into 2 main elements and using display: flex and justify-content: space-between we would get a horizontally align elements with a space in-between. You can learn more about flexbox here.

    Also, the icons is not showing up because you don't have it in the project repository. For typical scenario, we would create a folder called public or assets for storing images of any kind.

    - /root <- project root directory 
   - /assets
       - /images <- store the images here 
   - index.html

    Then you can use the image like this:

     <img src="/assets/images/some-icon.svg">

    If you have any other questions, feel free to ask. I would be glad to help :)

  • Zubair Adham•400
    @atmahana

    Hi there. I noticed that the container does not have the same height as the image hence why the extra portion you see when hovering on the image. The workaround I would try to fix this is as following:

    .container {
    max-width: 250px;
    aspect-ratio: 1 / 1;
    overflow: hidden;
}

.child {
    width: 100%
}

    • max-width: 250px; : This property sets the maximum width of the .container element to 250 pixels. This means that the container will not exceed this width, even if the content inside it is wider.

    • aspect-ratio: 1 / 1; : This CSS property sets the aspect ratio of the .container. An aspect ratio of 1/1 means that the container will have a square aspect ratio. This is achieved by making the height of the container equal to its width, which maintains a square shape.

    • overflow: hidden; : This property hides any content that overflows the boundaries of the container. In this case, since the container is set to a fixed width and a square aspect ratio, if the content inside it is larger, it will be clipped and not visible.

    • width: 100%; : This property sets the width of the .child element to 100% of its parent container's width. In this case, since the .container has a maximum width of 250 pixels, the child element will take up the full width of the container.

    I hope this clears some things. Cheers mate.

    Marked as helpful
  • Zubair Adham•400
    @atmahana

    Hi there. Good job on finishing the project.

    I'd suggest you to use the mobile-first approach. You can find this topic being discussed on Google and YouTube. Example below 👇

    /* Write your mobile styling here */

@media (min-width: 640px) {
  /* Write your desktop styling here */
}

    Also, since you initially specified a fixed width for the card, it won't adapt well to smaller screens. To make it responsive, consider replacing the fixed width with a max-width and set the width to 100%.

    .card {
  max-width: 500px;
  width: 100%;
}

    This approach ensures that the content adapts to different screen sizes and devices while providing a maximum width to prevent it from becoming excessively wide on larger screens. Hope this is helpful.

    Marked as helpful
  • Zubair Adham•400
    @atmahana

    Hello there. The border property will always take up space. Since you have defined the fixed width for the img element, border will take up space within the element's dimension.

    I would suggest using outline to fix the issue since outline is rendered outside the element's content and won't affect the layout.

    You can learn more about these two property here: border and outline

    I do also noticed that you are using position: absolute on the notification-picture class but you already have these on the parent element

    display: flex;
justify-content: space-between;

    If you want to adjust the position of the image, you can just use margin or padding instead of absolute positioning.

    Marked as helpful
  • Zubair Adham•400
    @atmahana

    Hello there. To set different transition behaviour for different states, you can set its own transition property. See the following 👇

    button:hover {
    background-color: hsla(224, 30%, 27%, 0.5);
    transition: background-color .4s ease-in-out;
}

button:active {
    background-color: red;
    transition: background-color .1s ease-in-out;
}
  • Zubair Adham•400
    @atmahana

    Hi there.

    To reduce the repetition I would suggest the following:

    Iterate through all the tip selection

    Create a same class so you you can select all of them using JavaScript

    <input class="tip-percent" id="five" type="button" value="0.05"> // change tip value from using "%" to this 
<input class="tip-percent" id="ten" type="button" value="0.1">
// the rest of the tip selection input

    const tipsPercent = document.querySelectorAll(".tip-percent");

tipsPercent.forEach((tip) => {

  const tipValue = tip.value; // get the tip value from the input
  
  tip.addEventListener('click', () => {
  
  // the calculation goes here 
  
  // you can use the value dynamically here 
  let calculate = (billInput * tipValue)
  })
})

    If you want to further simplify you code you can implement encapsulations. You can find the topic on Google or YouTube. Hope this helps!

    Marked as helpful
  • Zubair Adham•400
    @atmahana

    Hey there. That is a good progress.

    You are using flex which is a good start for a responsive layout. The reason of why the cards are not placed in the center because you kind of reset the flex state of the parent element with display: block on mobile screen.

    @media(max-width:700px){
    .main{
      display:block;
    }
}

    I believe you want to make the cards direction vertical instead of horizontal. You can achieve it by:

    @media(max-width:700px){
    .main{
      flex-direction: column
    }
}

    With flex-direction you can change the direction of the flex items in the flex container either to row or column. If you want to know more about flexbox, you can read it here. Kevin Powell also covered up this topic in his video

    Marked as helpful
  • Zubair Adham•400
    @atmahana

    Hi there. Well done on completing the challenge!

    I have done the same challenge and I would suggest the following 👇

    index.html

    <div class="card__image">
     <img class="image-main" src=IMG_URL alt="Woman illustration">
     <img class="image-box" src=IMG_URL alt="Box illustration">
</div>

    styles.css

    .card__image {
     position: relative;
}

.image-main, image-box {
     position: absolute;
}

    and use any of the top, bottom, left, right property to adjust the final position of the image.

    Read more about absolute positioning here and on the docs.

    I do also noticed that your solution is not responsive. If you are having trouble in making a responsive layout, you can watch Kevin Powell videos on the topic. He is great at explaining stuffs through video.

    Keep up the good work 💪

    Marked as helpful
  • Zubair Adham•400
    @atmahana

    Hi there. Good job on finishing the challenge!

    For the SVGs, I would suggest you to use the img tag for better code readability but it is depends on the context of the project.

    For this particular challenge, using the inline SVG seems to be no issue. But for the background, I would suggest to use background-image: url(YOUR_SVG_PATH) in CSS since we can style the background using other background related properties such as background-size, background-repeat and more.

    And other things to improve your solution is by

    Reducing unnecessary HTML elements

    div > button > a > span

    While this structure is valid and can work, you might consider simplifying it to div > span. This could lead to a more straightforward and efficient structure, making your code easier to read and maintain.

    Using semantic HTML elements and CSS class names

    • The pricing element's class can be changed from btn to pricing-plan or any related name
    • Element for 'Change' can be a link (<a>) tag and the span element that wraps it seem unnecessary, maybe removing it is a good idea
    • btn2 class can be named as payment-btn and it can be in the button element and you can just write the text instead of having h2 inside the button element
    • footer class can be changed to cancel-btn and change it to button element instead of general div, you also do not need to wrap the text with h4

    Read more about semantic HTML here and semantic CSS class names here

    Removing the absolute position property in CSS

    Remove the position: absolute and instead use the display: flex on the parent elements to lay out the child elements. You can learn more about absolute positioning and when should you use it here.

    Keep up the great work 💪

    Marked as helpful
  • Zubair Adham•400
    @atmahana

    Hello there. Good job on finishing the challenge!

    I took a look at it and it seems that the quote is overflow in the parent container. The solution for this is by adding overflow: hidden in the .daniel class in CSS.

    If you want to learn more about overflow, you can read it here

    Keep up the great work!

  • Zubair Adham•400
    @atmahana

    Hello there. Good job on finishing the challenge! 🥳

    You said you had a problem with media queries and it is understandable when you start learning to create responsive web layout.

    My suggestion is to use mobile-first approach. It's a good practice to start developing the layout and stuffs for mobile devices first.

    For example from your code, instead of writing the CSS like this:

    .container {
  width: 500px;
  height: 380px;
  border-radius: 10px;
  display: flex;
  background-color: white;
  overflow: hidden;
}

.div_1 {
  width: 50%;
  float: left;
  display: flex;
  background-image: url("/images/image-product-desktop.jpg");
  background-size: cover;
}

@media only screen and (max-width: 600px) {
  .container {
    width: 100%;
    height: 700px;
    display: flex;
    flex-direction: column;
    margin: 50px 12px;
  }
  .container .div_1 {
    background-image: url("/images/image-product-mobile.jpg");
    background-size: cover;
    background-repeat: no-repeat;
    background-position: center;
    width: 100%;
    height: 300px;
  }
}

    You can write for mobile devices first:

    .container {
  width: 100%;
  height: 700px;
  display: flex;
  flex-direction: column;
  margin: 50px 12px;
  border-radius: 10px;
  background-color: white;
  overflow: hidden;
}
.div_1 {
  background-image: url("/images/image-product-mobile.jpg");
  background-size: cover;
  background-repeat: no-repeat;
  background-position: center;
  width: 100%;
  height: 300px;
}

@media (min-width: 640px) {
  .container {
    width: 500px;
    height: 380px;
    flex-direction: row;
  }

  .div_1 {
    width: 50%;
    float: left;
    background-image: url("/images/image-product-desktop.jpg");
  }
}

    And now the code look less cluttered on the media queries.

    My other suggestions:

    • Use semantic class names. Instead of using div_1, create more meaningful class name like image-container to describe what is the element for. Read more here.
    • Use <img> element for the product image with alt attribute. Why? because it can improve the accessibility. Read more here.

    Overall you are doing great! Keep up the amazing work! 👏