@savaliyaalpeshSubmitted 3 months ago
Marco Di Matteo
@Marcod01All comments
- @Marcod01Posted about 1 month ago
Hello,
Overall good job on this project,
- Always make sure that the font families are correct, some of your elements have 'Times' font when it is supposed to be 'Manrope'
- Also consider having a reset javascript function when there is a resize of the screen, that makes sure that if the screen size changes that the functions will be reset properly.
0 - @AdriverionSubmitted about 1 month ago@Marcod01Posted about 1 month ago
This project is very well done overall. The use of semantic HTML is good and the code is well structured. Using <strong> instead of <b> would improve how the content is understood by screen readers.
The layout looks great on different screen sizes, and the responsive design works well. The CSS is well-organized, and the use of variables makes the code cleaner and easier to maintain. Overall, this project is great and looks identical to the design, nice job!
Marked as helpful0 - @BluordeSubmitted about 1 month agoWhat are you most proud of, and what would you do differently next time?
I learnt the use of CSS Grid. Probably further familiarize myself with the layout techniques I'm yet to use.
@Marcod01Posted about 1 month agoOverall this is a good job on this project.
Your supervisor heading needs a capital S, always make sure that spelling and grammar is correct. If it helps you can copy and paste the text from the starter files. Also your heading should stand out compared to the text. The headings of your cards should have had a higher weight and been darker, this improves user readability.
You could use some more padding and margin as well. Giving your elements and headings proper spacing could make your site look much cleaner and it makes it easier to read.
Marked as helpful0 - @zainy2401Submitted about 2 months ago@Marcod01Posted about 1 month ago
Hello,
Overall this is great and a very close solution to the design. I noticed you used a <div> for the product image and set the background image to: background-image:url('../images/image-product-mobile.jpg');
This works but the image looks distorted and uncentered. Instead use a <img src="images/image-product-desktop.jpg"> in the html and in the media query use this line
content: url('images/image-mobile-desktop.jpg'); this will actually change src of the <img> element.
Marked as helpful1 - @CrazyGreekGRSubmitted about 2 months agoWhat are you most proud of, and what would you do differently next time?
I am proud that I was more "bold" in trying stuff out until I find what was working. I did use ChatGPT for stuff I didn't know how to do, like how to color the dots in the prep time list and things like that
What challenges did you encounter, and how did you overcome them?As mentioned before, I solved some stuff by just googling how to do stuff and I did other stuff by asking ChatGPT.
What specific areas of your project would you like help with?Margins and positioning stuff inside a div and in general is kinda hard for me. For example, I had a kind of hard time positioning the image of the omelette in the div and the prep time div in the main div and so on and so forth. Any help is appreciated :)
@Marcod01Posted about 2 months agoThe HTML code uses some semantic elements like <main>, <h1>, <h2>, and <p>, which helps make the structure clearer. But, using more semantic tags like <section> or <article> for the different parts like ingredients or instructions would make it even better. It’s pretty accessible since it has alt text for the image, but more could be done, like improving contrast for readability or using aria-labels for screen readers. The layout should look fine on bigger screens, but the CSS doesn't seem to account for mobile screens. Adding media queries would help it adapt better to different screen sizes. The code is mostly well-structured and easy to read, but using an external stylesheet for all styles would make it more reusable. Overall, it seems close to the design, though I can’t see the original, so there may be small differences in spacing or fonts.
1 - @Makc240305Submitted about 2 months ago@Marcod01Posted about 2 months ago
The solution correctly uses semantic html with tags like <header>, <main>, and <section>, which help structure the content properly and improve accessibility. The image's alt attribute could be more descriptive to help users understand what should go there just in case the image doesn't display. The code is well-organized and easy to read, making it reusable for other projects. Overall, the solution closely matches the design.
Marked as helpful1 - @jambanixSubmitted 2 months agoWhat are you most proud of, and what would you do differently next time?
Happy to have completed it. Next time I will not try to overthink my approach
What challenges did you encounter, and how did you overcome them?Getting the fonts to resize properly with media queries when the screen shrinks
What specific areas of your project would you like help with?Any help on the best way to approach responsiveness would be appreciated
@Marcod01Posted about 2 months agoThis code doesn’t use semantic HTML. It could use more descriptive tags like <article> or <section>. Adding alt attributes to images is important as well. The design might not look good on all screen sizes, so testing and using CSS Grid or Flexbox could help. The code is easy to read and well-organized, but separating CSS into its own file and making sure it matches the design would make it easier to maintain and look more polished.
0 - @ardaacikgozSubmitted about 2 months agoWhat are you most proud of, and what would you do differently next time?
I have forgotten after not studying for a while, but after some time I understand what to do and which part to manipulate. Then I have revised the code but sometimes I had to ask to ChatGPT which is I am not proud of. I will try to check less ChatGPT and check the Internet more by manually in the next time.
What challenges did you encounter, and how did you overcome them?I didnt understand this display:flex display: float etc. when learning for the first time and still dont understand fully, that is why I used ChatGPT at some point.
What specific areas of your project would you like help with?Did I use divs correctly? Did I use CSS codes next to the body directly correctly? Should I have used these CSS codes by opening style?
@Marcod01Posted about 2 months agoYour use of the <div> element is pretty good for wrapping the QR code and text together. The class name attribution works, but you could try naming it something like qr-container to make it clearer what the <div> is for, especially if the project gets bigger.
As for the CSS you've put directly in the <body>, it does the job, but it’s better to move those styles to a separate CSS file or at least into the . This makes your code cleaner and easier to update later if you need to make changes. Overall, your combination of inline and embedded CSS works, but separating your styles from the HTML will help in the long run. You've done a good job centering everything with Flexbox, and the QR code with the text looks well-balanced.
In summary, your code is functional, but moving styles out of the body tag and organizing them better would be a nice improvement.
0
