Hello Everybody! 👋
This is my first solution for Frontend Mentor, so any feedback or suggestions on how I can improve my HTML and CSS are much appreciated!
Specific feedback I would love to hear:
Hi Kegan,
Fantastic first solution!
Your HTML is just about perfect I would say. The only thing I would recommend is to include height and width attributes on the img element. This tells the browser how much space to give the image even before it loads, which helps to prevent layout shift. You can see this in action if you throttle your network speed in dev tools and reload the page.
If you wanted to include the attribution section you have commented out, I'd also put that into a footer element for semantic purposes.
In your CSS I like that you've included comments to explain your decision-making. For your body tag you've used the place-content shorthand. This saves one line of space but, unlike justify-content and align-items, it is not supported in pre-chromium versions of Edge and not at all in IE, so you may want to avoid using it if those old browsers are still prevalent among your users.
You probably didn't need the grid on your qr-card__content. All you're gaining from it is the gap property, which in this case could be a single margin on either the h1 or p tag instead. I can understand why you'd use grid here though, as if you add more content to the card it would keep things simple.
Your page overflows on widths lower than 337px (with default font sizes) because you haven't set a max-width for the image. You can calculate what that needs to be using the viewport width and your --card-padding variable.
My only other comment would be that rather than base your card size off the image width, could you base the image width off the desired card size instead? Either way works fine, it just feels like the image width variable is a bit arbitrary, while a targeted total size for the card seems more logical to me.
All in all, a very professional job and much better than mine :P
Keep it up!
This is my very first challenge on frontend mentor and I can't wait to start a new one :)
I've had a lot of fun doing it. It helped me improve my knowledge of CSS grid as well as getting better on the responsive part.
The most difficult element for me of this challenge was the navbar and more particularly the js code related to it: I'd be happy to learn if there's a better way to do it.
Thank you and have a nice day
Hi Anne,
Great job for your first project here! I've just completed this one too and also found the navbar to be quite a challenge.
I think your approach with the JavaScript is basically the same as what I have done. My main feedback there would be to use IDs for specific elements you want to add JS handlers to, rather than classes.
You also appear to have written some JS to toggle classes on mouseover events for some elements. This wasn't working reliably when I was previewing your page (the classes were added but were not removed correctly). Could a better result be achieved in pure CSS with a hover selector, like you've done for the nav menu items and the 'Read More' button?
As far as non-JS feedback goes, take a look at the Accessibility Report the site has given you here. You've correctly nested your nav inside a header tag. Consider doing similar with a main tag for your page content and a footer tag for the attribution section.
Your .container div has a width set to a fixed value of 1300px by default. That means at screen sizes between 760px and 1300px the content overflows to the right. Changing the 'width' property to 'max-width' would fix this.
Your .list-card-1,-2, and -3 elements could share a single 'list-card' class instead. The grid-column properties you've given each one individually are redundant since they all take up the next available grid cell anyway.
My final input: In a number of places you have used pixel units for CSS values, particularly padding and margin. I would encourage you to try using more dynamic units like 'em', which are more responsive when users adjust their zoom settings for accessibility purposes.
If you get a chance, please check out my solution and give me some feedback as well. I'd really appreciate it!
