Skip to content
OverviewSolutions
5
Comments
3
Whythree
@Whythree

All comments

  • Whythree•90
    @WhythreePosted 3 months ago

    Overall, this is a great project with a solid foundation. There are a few areas where you can improve to make it even better.

    You use main and section, which is good.

    <main>
  <section class="profile">
    <div class="container">
    </div>
  </section>
</main>

    But the <div class="container"> is unnecessary. It serves no purpose, not even in your styling

    Same with

    <div>
        <img src="/assets/images/avatar-jessica.jpeg" alt="">
</div>

    You can style the image directly

    Then you use an h1, but in your css you target a h2. Probably a typo but it is why your heading is not aligned.

     <div>
        <button>Github</button>
        <button>Frontend Mentor</button>
        <button>Linkedln</button>
        <button>Twitter</button>
        <button>Instagram</button>
 </div>

    Better to use <a> tags here, since these buttons link to other webpages and don't trigger an action like submitting a form: Also since you are listing a bunch of these links, why not use an unordered list

    It could look like this

    <ul>
      <li>
        <a href="#">
          GitHub
        </a>
      </li>
      <li>
        <a href="#">
           Frontend Mentor
        </a>
      </li>
      <li>
        <a href="#">
          LinkedIn
        </a>
      </li>
      <li>
        <a href="#">
         Twitter 
        </a>
      </li>
      <li>
        <a href="#">
         Instagram
        </a>
      </li>
    </ul>

    Although you import the fonts at the start of your css, you don't use them anywhere. Try to use font-family and font-weight to more accurately reflect the design.

    Keep up the good work, and these adjustments will help polish your project further!

    Marked as helpful
    0
  • Whythree•90
    @WhythreePosted 4 months ago

    I like your solution, because of the use of semantic markup. Embedding the card in an a tag should be obvious, but I failed to do so.

    Considering the cases, like :visited shows, that you are very meticulous and strive for a good user experience.

    Really the only thing I have noticed are some slight deviations from the given designer. Your border-radius and the box-shadows seem to be to small.

    But overall a very good solution.

    Marked as helpful
    0
  • David Khuu•30
    @davidkhuuSubmitted 5 months ago
    What are you most proud of, and what would you do differently next time?

    Picking up Sass as quickly as I did. CSS is pretty straightforward to write but Sass helps take the tediousness out of it.

    Next time, maybe I'd try a different CSS pre-processor. I also relied heavily on the Figma file for exact measurements so next time maybe I'd try eyeballing it to challenge myself.

    What challenges did you encounter, and how did you overcome them?

    I forgot some basic CSS styles that I could usually fall back on in React. With raw HTML/CSS, I needed to search documentation online about the right properties I needed.

    What specific areas of your project would you like help with?

    Some Sass related questions:

    • Is Sass the biggest CSS pre-processor? How is its usage vs. Less in the real world?
    • Do *.css.map files need to be committed to git or could I have gitignored these file types?

    QR code component

    1
    Whythree•90
    @WhythreePosted 5 months ago

    Looks good. Cant give a definite answer but i decided against committing the .css.map file and saw no problems.

    I like your use of classes and variables. Makes your css reusable. You were very diligent even on a small project like this. This is something I can definitely take away from your project.

    Only thing i have noticed is your use of px units. Rem units are more accessible since they scale the conent based on users browser settings

    Good job overall

    Marked as helpful
    1