Skip to content
OverviewSolutions
23
Comments
10
David Gichuru
@dxiDavid

All comments

  • David Gichuru•360
    @dxiDavid

    Fixed it

  • David Gichuru•360
    @dxiDavid

    I'm not sure why the screenshot makes it look so stretched out 🤔. The live view looks just fine.

  • David Gichuru•360
    @dxiDavid

    Hello @Julian-Carlosama 👋🏾

    Congrats on completing the challenge 🎉

    I have a few suggestions on how you can improve your code:

    1. HTML📃

    • Make sure your content is wrapped inside a <main> landmark tag. If you want to learn more about semantic HTML tags you can find that here.
    • Make sure that your images have alt text for screen readers which will improve accessibility for your future websites. Try to have descriptive text so that the person knows exactly what the image is.

    2. CSS📄

    • Avoid setting fixed values for dimensions like width and height. Instead set a max-width and max-height for your elements
    • Avoid using absolute units like px for anything other than border-radius or box-shadow. Use responsive units like em and rem which will make your projects more responsive.
    • To center your card you can try the following methods
    1. Using grid
    body{
     min-height: 100vh;
     display: grid;
     place-items: center;
}
    1. Using flexbox
    body{
     min-height: 100vh;
     display: flex;
     flex-direction: column; 
     justify-content: center;
     align-items: center;
}

    Another useful tip is to build a habit of setting custom properties for the styles found in the style guide and any other styles you might re-use.

    You might also want to set up utility classes for centering things to avoid repeating yourself throughout the code.

    Your solution looks great

    I hope you found this helpful 😁

    Happy Coding 🥂

    Marked as helpful
  • David Gichuru•360
    @dxiDavid

    Hello @WiaterKa 👋🏾

    Congrats on completing the challenge.

    To solve your centering problem, try wrapping everything in a wrapper div or main container and center it using flexbox or grid.

    body {
  background-color: hsl(210, 46%, 95%);
  margin: auto;
  display: flex;
  justify-content: center;
  align-items: center;
  min-height: 100vh;
  width: 1100px;
  height: 572px;
}

    For the above code you've written, declare a min-height first then use flexbox. Please do not give the body a fixed height or width after. The auto margin is also unnecessary.

    One last thing, avoid using pixels for dimensions, font sizes, or spacing. You're better off using em or rem.

    I hope you found this helpful, happy coding🥂

    Marked as helpful
  • David Gichuru•360
    @dxiDavid

    Hello @JavierRiv97 👋🏾

    The exact font sizes are for pro members otherwise, use your knowledge of CSS and best judgment to get as close as possible to the design. Don't worry though, It's not that hard.

    Also, ensure that your projects meet accessibility requirements by adding alt text to your images for screen readers.

    Make sure that your Body content is wrapped in landmarks like <main> or <article>. You can find out more on semantic HTML here

    Happy coding🥂

  • David Gichuru•360
    @dxiDavid

    Hello there @Akiraz14 👋🏾

    I have a suggestion that will help you write better code

    • when setting custom properties, try giving them descriptive names like --primary-font-color instead of --color-1st. It will make your CSS easier to read and understand. Also, try to set custom properties for styles such as font-size, font-weight, (basically things you see in the style guide)border-radius since they will most likely be used on more than one occasion.
    • Limit the use of pixels to things like box-shadow and border-radius, otherwise use relative units.
    • only look for references when you are absolutely stuck and cannot find a way out. don't waste time doubting yourself. If you have a working solution then good. come back and refactor later

    The attention to detail in your solution is impressive💪🏾

    Keep Going, happy coding🥂

    Marked as helpful
  • David Gichuru•360
    @dxiDavid

    Hello @adarsaparasar

    I have a few suggestions on how to improve the solution

    • First contain everything inside the <main> </main> tags to improve accessibility. After that, put your container div then wrap the main elements, which are the picture and text, in divs of their own then use flexbox to position them.

    • The markup you've written just has one container and that makes things unnecessarily difficult to control. Take the time to really think about the structure of your content which will make styling it easy.

    • Avoid using absolute values like pixels for margins, padding, and font size and use relative values like em and rem which improves responsiveness and accessibility

    • Use max-width and max-height instead of width and height for a more responsive design. Also, avoid redeclaring the same values in a media query or active state if nothing is changing, it just makes your code messy and hard to read. Only declare what is changing.

    • Last but not least try as much as possible to stick to the colors in the design

    I hope you find this helpful, happy coding 🥂

  • David Gichuru•360
    @dxiDavid

    Hey @Iron-Mark

    Congrats on completing the challenge 👏🏾 The solution looks really good.

    Here's how you could make a few improvements:

    • Make sure the title or main heading is contained in the <h1> </h1> tags to get rid of those warnings and improve accessibility then adjust the font size to fit the design.

    • As for the overlay, since there's no fancy animation going on in this situation, just put the image in a div, set the background color to that overlay color, and then slightly reduce the opacity of the image.

    <div class="ctn-img">
      <img src="./images/image-header-desktop.jpg" alt="people in an office">
</div>

    .ctn-img {
       background-color: hsla(277, 100%, 24.5%, 0.57);
}
.ctn-image > img{
       filter:brightness(40%)
       filter:opacity(40%);
}

    You could also play around with other filters like brightness and saturation to get closer to the design. This will save you several lines of CSS code and markup. Just make sure the image fits in the div and you should be good.

    Hope you find this helpful 🙂

    Happy coding

    Marked as helpful
  • David Gichuru•360
    @dxiDavid

    I'm not sure why I have those warnings

      <div class="reviews">
           <div class="stars">
                <ul>
                    <li>
                      <img src="./images/icon-star.svg" alt="star">
                    </li>
                    <li>
                      <img src="./images/icon-star.svg" alt="star">
                    </li>
                    <li>
                      <img src="./images/icon-star.svg" alt="star">
                    </li>
                    <li>
                      <img src="./images/icon-star.svg" alt="star">
                    </li>
                    <li>
                      <img src="./images/icon-star.svg" alt="star">
                    </li>
                </ul>
           </div>
   </div>
  • David Gichuru•360
    @dxiDavid

    Hi, Ezequiel Córdova Sotomayor! 👋

    This solution is awesome, congrats 🎉

    I have a few suggestions for you to improve the code

    You might want to set the breakpoint a little higher than 490px to allow

      .track__snippets {
    flex-direction: column;
  }

    to stack items in that section on top of each other starting at a larger screen size (somewhere around 900px)

    You could also try to put the sections inside <main> to avoid the warnings

    <main>
    <section class="track">
    <section class="access">
    <section class="workflow">
    <section class="clipboard">
</main>

    I hope you find this helpful 😁

    Marked as helpful