I think I should have made a grid for the summary content, that way I could have positioned every item in a more precise and clean way. I have to embed the last two scores in a separate div, just to make them justfied on the right side. What was the best way to achieve the correct position?
Problem 2
The responsive part was tricky. I had to add media queries, instead of relying on Flexbox only. The result side was always too big on mobile. What is the easiest way to do that?
I really like your solution; it's a near-perfect replication of the design!
I think it's perfectly okay to use flex in this project, but grid would be good too. I advise you to look into the gap attribute. You can set it on the container that contains the flex items, and it adds spacing between the items evenly, so you don't have to use margins.
It's okay to use media queries; they're intended for use in responsiveness. You could use grid instead of flex on your .card class and set the grid-template-columns to 1fr 1fr and on smaller screens (from 480px) to grid-template-rows: 1fr 1fr. If you're not familiar with grid, I think you should check out this mini-game: Grid garden. It's a fun way to practice grid. For flex, there's also a game called Flex froggy. I think you should check and play both of them. Another great source (I would argue that it's even the best) is Josh W Comeau's Interactive guide to flexbox
You use a lot of margins where padding would be better, for example in your result div you use a lot of top margin and bottom margin to make some space between your items and the container, but it can be easily done with the padding and you need to use it only once on your container, check some tutorials on it when to use which!
Please keep in mind that these are only suggestions and your project looks really good and thats what is important in the beginning, I think you did great and keep up the good work.
The hover for the image was difficult for me but I managed to complete it and as for the responsive I did not find what the expected result was, so I decided to leave it as it is and the size that I selected for the desktop is quite good and comfortable for phones. Other than that, I liked this challenge
I really like your solution, especially the part where you added the hover effect on the image. I love it!
I would like to give you some suggestions:
You could set your body's height to min-height: 100vh instead of height: 100vh. This way, if the container needs to be bigger, it can. I know that it may not be relevant in this project, but I read somewhere that we should avoid using fixed heights whenever we can because of responsiveness. So, I think it is good to practice.
As I mentioned, I really like your approach to the hover effect. However, if you have the time, you should look into the ::before and ::after pseudo-classes, which are a great way to achieve the same effect without adding extra HTML to your index page.
In the beginning, you started to add comments in the root section. I think it would be more consistent if you added comments to the whole CSS. It does not need to be very detailed, just mention which part of the design starts where. This makes it a lot easier to read for others and for you too if you plan to revisit the project later.
Anyway, keep in mind that these are just suggestions, and your project is great just the way it is.
I'm absolutely amazed by how perfect this is! I never would've thought it was possible without having access to the Figma files, and I really love it. Congratulations on such a fantastic job! Can I ask how much time it took you to complete?
Also, in a previous comment, I noticed that you mentioned using the Pixel Perfect extension. I was wondering, what is your approach when using this tool? Do you start by eyeballing the layout and then use the extension to fine-tune it?
And one more question if you don't mind, do you work as a professional web developer?
Thank you very much in advance, you really motivated me!
It was difficult to change the img src (background-image url) according to the media query and screen resolution (mobile / desktop img) to make the page responsiv. I think my solution is not nice at all.
You can use the <picture> semantic HTML tags in your html folder. If you use the srcset attribute inside the <picture> tags and use the media attribute you can set the min or max width at which the <img> src will be swapped to the srcset image! i find it really easy to use and use all the time when i can.
Your code seems fine to me, try to use comments so it is easier to browse for other and it is a good practice.
Remove duplicate font-family declarations: You have defined the 'Montserrat' font-family twice in the 'body' selector. You can remove one of them to avoid redundancy.
Use consistent measurement units: In the 'container img' selector, you have used 'px' units to define the width and height of the image, but you have used '%' units to define the width and height of the container. It's better to use the same units consistently throughout the code.
Add units to the margin-top property: In the 'body' selector, you have set the margin-top property to 50, but you have not specified the unit of measurement. It's better to add a unit such as 'px' or 'em' to avoid confusion.
Increase font size in attribution: The font size of 7px in the 'attribution' selector may be too small and difficult to read. Consider increasing it to improve readability.
Use more meaningful class names: The class names 'container', 'contents', and 'attribution' are not very descriptive and may make the code harder to understand and maintain. Consider using more meaningful names that describe the purpose of each element.
HTML:
Add alt text to images: You have used an image tag to display the QR code image, which is good. However, it's important to provide an alt text for the image to improve accessibility. The alt text describes the image for users who cannot see the image or who use assistive technologies like screen readers.
Use semantic HTML: Your code already uses some semantic HTML tags like 'head', 'body', 'div', and 'h3'. To improve the semantic structure of your HTML, you can consider using more descriptive tags like 'header', 'main', 'section', 'footer', etc. for different parts of your web page.
Add comments: Adding comments to your code can make it easier to understand and maintain. You can add comments to explain the purpose of different sections or elements of your HTML.
Furthermore your container is not centered vertically, i would suggest to do that because your project will look 100% better. You should watch tutorias about Flex and Grid, they make placing items to their corresponding places much much easier. i can suggest you 2 tutorial sites:
Flexbox - https://flexboxfroggy.com/
Grid - https://cssgridgarden.com/
Learn about them and then solve these mini-games and it will be much easier to play with positioning!
Once again congratulations to your project, happy coding! :)
Great solution a near perfect replication of the design, love it!
here are my suggestions:
CSS:
Consider adding comments to your code, adding comments to your CSS code can help make it more understandable and maintainable, especially if you are working on a larger project. Consider adding comments to explain the purpose of each section of code or any notable features. I know thet it is not that big project but it is good to practice ;)
Try to use more descriptive class names. While the class names used in the code are
functional, they could be more descriptive. Consider using class names that are more specific to the content they are styling, like card-container instead of container. Again it is good to practice and will repay you greatly later on.
HTML:
Add** alt text** to images. The <img> tag includes the alt attribute, it should be filled with a descriptive text that describes the image content. This is useful for accessibility. If i remember correctly 20-30% percent of the internet users are disabled so it is very important to watch out for accesibility.
Close <h6> tags: The <h6> tag for "Challenge provided by" is not closed properly, which can cause issues with the layout of the page. Make sure all HTML tags are closed properly.
Try to maintain the hierarchy of the Heading tags, the web should always contain a H1 and after the H1 comes the H2 and so on.
I would like to mention that you centered your card correctly, there are alternatives for it though.
Me myself i prefer to use display:grid and then place-items: center. it automatically centers the items both horizontally and vertically.
If you use flex you can use align-items:center combined with justify-content:center. One is centering it horizontally the other vertically
I know i mentioned a lot of things here but none of them are a serious problem just a suggestion! :)
I really love your take on this project, your site is a near perfect replication of the design.
I am a beginner myself so take my comment with a grain of salt
At a lower vertical resolution (375x640 for example) there is no space on top of the container and under the container, sadly i dont know the solution for that, i have the same problem in my projects. Maybe you want to look into that.
Maybe you should consider using comments in your CSS file to separate sections of the code so it is easier for others to browse!
Once again i really love your project and happy coding! :)
I really love your take on this project in fact I love it much more than the original design.
I am just a beginner so i cant really give you any kind of constructive criticism about your code, but i would like to let you know that as i tried your live preview and used the built in autofill function in chrome for the inputs the background color of the input changes to #e8f0fe and hides its corresponding label.
Keep up the great work!
P.S.: sorry if my comment is hard to understand i tried my best trying to explain it! :)
<div class="text">
<h1>Improve your front-end skills by building projects</h1>
<p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>
</div>
I checked/tried your code keep practicing and keep up the good work, congratulation to you for working on this project!
Here are my suggestions:
As mentioned in Vanza Setias comment if you use max-width your .attribution container instantly becomes more responsive.
i would suggest that in the .image-div class you delete the margin-top and margin, instead use padding (1rem for example) This way you can make a nice white lane around the image.
Also in the same class you can remove the width and height because it will resize automatically according to its container size
Please take my advice lightly, i am a beginner too and my advice is according to the best of my knowledge, i might be wrong.
