Not Found
Not Found
Cannot read properties of null (reading 'code')
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Cannot read properties of null (reading 'code')
Not Found
Not Found
Not Found
Cannot read properties of null (reading 'code')
Not Found

All comments

  • phemmyils• 80

    @phemmyils

    Submitted

    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.

    atanasov36• 580

    @AtanasovCode

    Posted

    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 :)

    0
  • atanasov36• 580

    @AtanasovCode

    Posted

    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.

    0
  • Alan-H-H• 60

    @Alan-H-H

    Submitted

    I had no issues with this one. Only with the footer section that frontendmentor gives you, maybe because it wasn´t in the design.

    atanasov36• 580

    @AtanasovCode

    Posted

    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

    0
  • Camille• 130

    @fyrfli

    Submitted

    One of the things that has had me stuck for a couple of days is that I read that you need to removeEventListener() after processing is complete but I haven't found any solid explanations on the different ways to do that yet. That is something I need to research some more before I do any more in-depth functionality in JS. I am well aware of pages that end up going on and on and on and hanging the browser and want to write code solid enough to avoid that kind of overhead.

    Any ideas/thoughts/suggestions?

    atanasov36• 580

    @AtanasovCode

    Posted

    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

    0
  • atanasov36• 580

    @AtanasovCode

    Posted

    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

    0
  • Etienne L.• 95

    @etnlmy

    Submitted

    Hello Everybody

    This was a fun challenge and it took me longer than expected, as usual, lol. I went for a Vanilla JS/HTML solution.

    I ran into some issues when deploying, as one of the API (worldtimeapi) does not support HTTPS. Since the calls have to be made directly from the client, and therefore cannot be proxied, I used another API instead (TimezoneAPI.io).

    If you give it a try and notice some issues with the location and/or time data, please let me know.

    Any other more general feedback is greatly appreciated 😀

    Thanks!

    atanasov36• 580

    @AtanasovCode

    Posted

    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

    1
  • P
    Aaron Romanick• 270

    @aaron-romanick

    Submitted

    This is my first Frontend Mentor project, but it was really fun!

    • This is my first time using CSS variables and themes. I feel like my variables are not very DRY and could be condensed more. Is there a better way to organize them? Also, I only really used variables for the themes, but if you have any suggestions about ways I could have used it in other areas of the CSS, that would be much appreciated.
    • As for the JavaScript, while I went with classes. I feel like some of my methods are still too long; if you could point out some ways to break them into smaller, reusable functions that would be great!
    • I bug tested this a lot, but through many code rewrites, I probably missed some things. Let me know if you find any bugs!

    I appreciate any and all feedback, including in regards to anything I didn't list above! Thank you :)

    atanasov36• 580

    @AtanasovCode

    Posted

    Great work here it looks great :)

    The transition from the themes is smooth and the themes look great too!

    0