Skip to content
OverviewSolutions
11
Comments
6
Chukwuka Eze
@ezechuka

All comments

  • Chukwuka Eze•310
    @ezechukaPosted about 2 years ago

    Nice work James on finishing this project. Having observed your code you can make the icon display by this few changes:

    • In your index.html you should change it from a div to an img: <img class="card__image--icon" src="./images/icon-view.svg" alt="preview">

    • Go to your style.css and set the display to none by default and block on hover.

    .card__image--container .card__image--icon {
  display: none;
}

    .card__image--container:hover .card__image--icon {
  display: block;
  position: absolute !important;
  top: 50%;
  left: 50%;
  transform: translate(-50%, -50%);
}
    1
  • Chukwuka Eze•310
    @ezechukaPosted over 2 years ago

    Well done chief! Your CSS looks neat. However, I do have some suggestion. On the mobile menu, the links doesn't show because the text-color is white.

    You can change it from this:

    .nav-links-container li {
        color: var(--dark-blue);
        font-weight: 500;
    }

    to this:

    .nav-links-container li a {
        color: var(--dark-blue);
        font-weight: 500;
    }

    And it should work.

    Also nice job chief :)

    Marked as helpful
    1
  • Skippy•110
    @skippysworldSubmitted over 2 years ago

    After some time when I was learning JavaScript I came back to next challenge.

    I'd say I'm pretty satisfied with the JS part, but not at all with CSS. It works, but I feel it could have been written much better, easier with less code.

    What do you think?

    Interactive Rating Component

    1
    Chukwuka Eze•310
    @ezechukaPosted over 2 years ago

    I checked your CSS code and it's very neat, I like it. If you want to write less and easier CSS I suggest learning/using SASS (that is if you don't know about it before), it's a css preprocessor with cool features like nesting, functions, inheritance.

    For example: this code snippet from your style.css:

    .container-thanks h2 {
    padding-top: 1rem;
  }

  .container-thanks p {
    margin-bottom: 1rem;
    text-align: center;
  }

    can be written or nested as:

    $top-padding: 1rem;
.container-thanks {
      h2 {
          padding-top: $top-padding;
     }
     p {
        margin-bottom: $top-padding;
        text-align: center;
     }
}
    Marked as helpful
    1
  • Chukwuka Eze•310
    @ezechukaPosted over 2 years ago

    You did a great job, although I've one suggestion, on mobile when the mobile menu is opened you should disable scrolling of the page.

    Marked as helpful
    0
  • Chukwuka Eze•310
    @ezechukaPosted over 2 years ago

    You did a really good job bro! You can watch Traversy videos on CSS Flexbox and CSS Grid to get better understanding of CSS layouts

    Marked as helpful
    1
  • Chukwuka Eze•310
    @ezechukaPosted over 2 years ago

    This is really impressive bro! Also you've 5 HTML issues. To fix it you can use this validator site ==> https://validator.w3.org/#validate_by_input

    Paste your html code in it and it shows the issues.

    Also nice job. I think I'll do this for my next challenge

    0