Frontend Mentor | Koen Oosterhuis's profile

All comments

@MfrekeeSubmitted over 1 year ago

Hey y'all, came back for the summer holiday a few days ago, and i thought i'd learn more things on front end (push myself from being a designer to being a developer) and still practice the ones I already know (that second part's easier now with front-end mentor). Anyway, you'd notice that what I did isn't exactly what's on the desktop design pic (thought I'd play around the color a bit🤷), and I've actully got a question, which is that, when i share the live site link to my phone, everything in the QR Code Component shifts to the right (except that text that's above it though), and the white background doesn't cover the code nor the picture anymore, it's not even there again. So I ask, is this how it's supposed to be on mobiles?🤔 Probably because it wasn't created as a mobile site or something?🤷‍♂️

QR code component desktop design

Koen Oosterhuis•80

@koeno100Posted over 1 year ago

Good first try on this challenge! You've got the visual layout right and I get what you're trying to do. There are a few serious problems to address here, which luckily will clean up your design significantly when fixed:

What is the resolution of the screen you created this on? I'm on a screen that's 1920px wide and the solution looks very weirdly stretched on my screen (see the screenshot in the design comparison at the top of this page). To keep the width under control, I recommend you open the example image in a photo editor like paint or photoshop and measure the width of the main panel. Then, when you have found the width of the card, you can specify using the width property in CSS.
The reason your text shifts to the right is because of your margin: 51px 400px; line in the .vid selector. Since the first change I mentioned will keep the width under control, it's not necessary anymore to have such margins. If you want to center the card, regardless of the screen resolution, use margin: 51px auto instead. This will automatically center the card horizontally.
Be aware how padding and margin affect elements differently. padding affects the space within the border, while margin affects the space outside of the border. This means that the padding-top: 8px property within your .pic selector will add 8 pixels to the top of the bouding box of the image within the border. Since the dimensions of the image are fixed, the image itself stays unaffected, but the top border will shift 8 pixels up (which is 8 px of invisible space since there's no background color or anything else within this element), thus the top corners of the image will look different than the bottom corners.
Did you have a look at the style-guide.md? There it specifies the used colours, fonts and font-sizes. For instance, the font-size of <p> should be 15px according to the style guide, and it has given you all the colours used in the design for the background etc.
Your HTML needs some adjustments when it comes to accessibility. Instead of <div>, try use appropriate semantic HTML tags: reference.

For example, <div class="small"> could perhaps be replaced by <header> since it's heading the page, while <div class="vid"> could be replaced by <main> as it's the main content of the page.
You used <strong> incorrectly. This element is intended for content that has importance or urgency within the bigger context. Instead, the text is a header, so it's more appropriate to use one of <h1> to <h6> headers.
Avoid using id unless it really is a unique element.
I would also recommend to use more specific class names. Classes like "small," "vid" and "pic" are easy to track in a small challenge like this, but once you get to making bigger projects, it's easy to lose overview if you're not being specific with your class names. I believe it's better to teach yourself these habits early on instead of having to readjust later on.
Block elements like <div> and <p> automatically start on a new line, so <br> is not necessary before or after these elements. Here is an overview of block-level and inline elements.

Instead of using px in your CSS, use rem instead. px is absolute and can make your page unreadable for some people. Browser have a setting to have the default font size changed, which is by default 16px. However, if someone needs default font size set to 32px to be able to understand the page, then the layout of the page won't change along when using px. rem adjusts based on whatever font size the browser or <html> tag is set to. Thus, if the user sets their default font size to 18px, then 1rem = 18px. However, when the user sets the font size to 32px, then 1rem = 32px. By making your designs proportionally in rem, this ensures that the whole design still works when the font size is changed. Assuming that you code everything with a browser default font size of 16px, you can convert all your px values to rem values by dividing them by 16. Someone here referred me to this article earlier if you want to know more about the different units!

It's a lot of information, but I think if you tackle these points first, you'll get a much cleaner code with more control over the design and a cleaner looking design! If you have any questions, feel free to ask! :)

Marked as helpful

Bakek-lang•80

@Bakek-langSubmitted over 1 year ago

QR code component

Koen Oosterhuis•80

@koeno100Posted over 1 year ago

Hi! This is a nice first try at this challenge. The design looks similar to the one given and you're quite close with the dimensions. The colours match as well and while the structure could be closer to the original, it looks good overall!

A few comments from my side:

You have restricted the width of the container to 1440px and 375px. The problem this creates is that this will only center the card when viewing this on a device that has a width of 1440px or 375px. For example, on my screen, which is 1920px wide, the card is much more to the left. I believe the challenge meant that 1440px and 375px were used as references for responsive design so you can compare it at those widths, but instead try to make the result responsive by having the card stay in the center of the page, regardless of the screen resolution. You can do this by getting rid of width: 1440px in the body selector and then using margin: 0 auto with some additional code (Check this for reference) or use a flexbox and justify/align the card to the center. (Using a 125px top/bottom margin only works in certain resolutions, but for it to be truly responsive, you want the card to be centered at all times.)
'Outfit' is not a default font, so you have to import it first from Google Fonts by downloading it from there, or simply adding @import url('https://fonts.googleapis.com/css2?family=Outfit'); to your CSS file.
Use semantic HTML! In this case you have a main section and a footer, so you can easily use the <main> and <footer> elements instead of divs.

Those are the main things I have to say so far. Have a look at these things and see if you can improve your design!

Marked as helpful