Skip to content
OverviewSolutions
35
Comments
7

atanasov36

@AtanasovCodeNorth Macedonia660 points

Studying and practicing to become a Frontend Developer in the near future :) I have a strong passion for creating websites from scratch and I love finding out and implementing new techniques of doing so!

Latest solutions

View more solutions

Latest comments

  • joelani•80
    @joelani
    What are you most proud of, and what would you do differently next time?

    All feedback is welcome, thank you in advance

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

    I think the only problem I faced was centering the card div, it took me a while but after some research, I was able to center it by changing the parent div to flexbox and adding the justify content and align content.

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

    I'm struggling a bit with media queries and could use some guidance.

    Responsive QR Code Component

    2
    atanasov36•660
    @AtanasovCode

    Hey, great solution. Some tips I would like to give you are centered around responsive design.

    Right now the width and height of the card are fixed which is bad practice when it comes to making sites responsive. So instead of proving a fixed height to your card, you should let the content inside determine it's height.

    So instead of doing this:

     width: 315px;
 max-width: 315px;
 height: 484px;

    You can instead use padding inside of the card, and apply margin and padding the the children elements inside of the card and this will make the height dynamic and based on the content inside.

    You said you struggled a bit with media queries but they are also very important when it comes to responsive design. You can think of them as breakpoints that get applied whenever a certain width is reached.

    For example you could add this bit of code to your solution that will make the card take up 90% of the screen width if the width of the screen is less than 648px:

    @media (max-width: 648px) {
    width: 90%;
    max-width: 90%;
}

    Good luck and happy learning :)

  • atanasov36•660
    @AtanasovCode

    Hello!

    From what I can tell at a quick glance, you have a few problems that you should fix.

    1. The responsive design of your solution is flawed, the @media query kicks in when the screen size is 375px but by then the card is not correctly in view because it is too large to fit in the screen, (Tested on Firefox). You should set the @media query to activate at a larger size, around 500 or so pixels. You should play around with it and see which max-width works best depending on your design.
    2. As far as I can see <div class="right-col"> and <div class="product-info"> wrap around the same elements, so you can easily remove one of the divs. (not really a problem but you wanted to know how to use less divs)
    3. In your @media, you are applying a bit too many styles for a simple project like this. You can make the project responsive just by changing a few properties to your main wrapper (the one that contains the card). You don't really need to apply styles to the title and such if you apply the correct styles to them outside the @media.

    These are just my suggestions, otherwise your design looks great.

  • atanasov36•660
    @AtanasovCode

    I opened your solution on Firefox and I don't know if it is because of the footer, but the footer and the card are mashed together and are at the bottom of the screen instead of the card being in the middle.

    If you want to keep the footer (you can delete it freely if you don't want it), may I suggest positioning it absolute so that it does not affect any other elements on the page.

    And looking at your solution in Firefox, I can see that you gave the card a fixed width. This is fine because you want the size of the card to stay the same but I would suggest adding a media query when the window reaches a certain size, and make the width of the card take up 90vw (or don't specify a width and just give it some margin). This way it is more responsive and it will take up more of the screen which can increase readability on smaller screens.

    Otherwise the design is great!

    Marked as helpful
  • atanasov36•660
    @AtanasovCode

    Hi, looks like you did a great job!

    From my observations I can see that you have a few styling issues:

    • Your card is not correctly centered, you can also try and fix the sizing of the card because it looks like yours is a bit larger then the original design.
    • The rating buttons, you can try and make their background into a perfect circle because now they are a bit thinner on top and look more oval shaped.
    • Add cursor: pointer when someone hovers over the submit button and the rating buttons.

    Hope some of my feedback helped, I did the exact same challenge and you can go see mine and give me some feedback too :)

    Marked as helpful
  • atanasov36•660
    @AtanasovCode

    Looks like you did a great job! One thing I did notice is the border-radius. You applied this property to the image but you can see the background so I would suggest you apply the border-radius property to the actual parent of the image and just use overflow: hidden.

    I hope my feedback helped :)

    Marked as helpful
  • atanasov36•660
    @AtanasovCode

    Amazing job Etienne!

    One thing I did notice is when clicking on "more" to display the additional information, the button becomes inverted so when all the info is shown, the buttons displays "more" and when all the info is hidden the button displays "less"

    Marked as helpful
View more comments