Skip to content
OverviewSolutions
6
Comments
6
artemkotko14
@artemkotko14

All comments

  • artemkotko14•100
    @artemkotko14Posted 3 days ago

    Congrats, you have done a great job! I can only suggest a couple of changes to your code:

    1. Specify alt on your images. This text helps screen-reading tools describe images to visually impaired readers and allows search engines to better crawl and rank your website. So it's a good habbit to always include that.
    2. You are using 90rem for your media query. On a full size of my laptop screen I can still see a mobile design. I would suggest to use something between 40-60 rem. Hope this will help. Keep on coding!!!
    Marked as helpful
    0
  • artemkotko14•100
    @artemkotko14Posted 10 days ago

    Your desktop design looks great. However the image stays the same when switch to a small screen. To switch your image based on the screen size I would advice you to use picture element. Also, I would not suggest to use h1 for a small ribbon text above the name of the product. h1 must indicate the primary topic of your webpage. hgroup element with heading and paragraph Parfume for this would be much better practice. Hope this will be helpful for you. Keep on coding!

    Marked as helpful
    1
  • artemkotko14•100
    @artemkotko14Posted 27 days ago

    I have nothing to say, except very well done!!! Keep on doing your great job.

    0
  • artemkotko14•100
    @artemkotko14Posted about 1 month ago

    Great job on this challange! A few things I would suggest to consider:

    1. Use <a> or <button> instead of <li>. Think of it as a real website, if a user clicks one of these buttons, it takes them to another page. With <li> you are not gonna have possibility to implement such functionality.
    2. Your buttons don't have a focus state, so I can't move through your page without a mouse.
    3. Add cursor: pointer; to your hover state, to make it look the same as at the design.
    4. Also you used px for your font-size which is not a good practise. You can read aboit it in this article: Article
    0
  • artemkotko14•100
    @artemkotko14Posted about 1 month ago
    1. Get into the habit of including a full modern css reset at the start of the styles in every project. Andy Bell or Josh Comeau both have good ones you can look up and use.
    2. This solution currently overflows my screen at the top so there's some content cut off that I can't reach. This card mustn't have a height. That's important to understand that you shouldn't limit the height of elements that contain text. Let the browser do it's job and decide what height is needed based on the content and spacings inside. As soon as you limit the height the card has the potential to break, like if an author changes the amount of text in there or a user changes their default text size or translates the site into another language.
    3. The card shouldn't have a width either. Give it a max width in rem instead. That way, it can shrink narrower if it ever needs to like on a small screen.
    4. The card doesn't have hover or focus states, which was the main challange here. If you need help with that you can read more about it here:
    • focus state
    • [hover](https://www.w3schools.com/cssref/sel_hover.php
    1. Font size shouldn't ever be in pixels. Use rem instead.
    2. Your h1 shouldn't be centered. Check again at the original design.
    3. I don't see font-weight and font-size properties on most of your elements.
    4. Your button is missing a border-radius
    0
  • Jennifer•70
    @x3blondieSubmitted about 2 months ago
    What are you most proud of, and what would you do differently next time?

    Centering divs

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

    How can i better document in github

    qr-code-component

    2
    artemkotko14•100
    @artemkotko14Posted about 2 months ago

    Great job!!!

    I would suggest to put your <div class="attribution"> at the bottom of the page. You can do so with { position: fixed; bottom: 0; } or some other ways that you prefer. Don't forget in CSS file we use /* */ for comments. Also try to experiment with sizes as your container looks bigger then it is on an original design. But overall I like how your code is well-structured and readable!

    0