@asma-zahraSubmitted 5 months ago
P
Arnaud Lahaut
@Arnotts33All comments
- @Arnotts33Posted 3 months ago
Hey Asma!
Congrats on completing this challenge :)
Your solution looks very good, with a good use of semantic HTML and accessibility attributes.
Your code is very well structured and easy to read and understand.
I don't see what else I could say, as I am fairly new to web dev :)
Well done and keep it on!
0 - @Arnotts33Submitted 3 months agoWhat are you most proud of, and what would you do differently next time?
My first time doing a 'complex' JS challenge by myself. Quite happy but might refactor my code at some point.
I really need to start using SCSS. But is it widely used in the pro world and is it considered good practice?
What challenges did you encounter, and how did you overcome them?Main challenges:
-
Responsive part with switching the image on the right on desktop layout: I used and for the image. Use of flex grid.
-
JS email validation: Took me a while to make the JS file working... forgot to put novalidate in the HTML.
Any feedback or review are welcome!
Thanks!
@Arnotts33Posted 3 months agoHey Alex!
Thanks for another feedback, always appreciated!
I took the novalidate from the form course just before this challenge. Thing is when I get rid of this and the action attribute, it's not working. I don't get the error message.
Thanks for your advice about SCSS :)
1 -
- @khalagaiSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
Creating the floating social links element.
What challenges did you encounter, and how did you overcome them?Creating the floating social links element. I used absolute position to manipulate it.
What specific areas of your project would you like help with?I couldn't set the arrow part of the floating element fixed directly over the share image when resizing the screen. I would appreciate help with that.
@Arnotts33Posted 4 months agoHi!
Well done on finishing this challenge!
Your HTML structure is good and readable. Maybe try to use semantic HTML next time to improve readability and accessibility.
A few hints on how you could get closer to the preview:
- Drawer image: You could try to modify image width and/or its object-fit property.
- Elements spacing: Your elements look too big and it creates too much space between them. I think a good practice is to set their margin to 0 when setting CSS.
Hope that helps!
Marked as helpful1 - @ralphvirtucioSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
What I am most proud of is authoring the semantic HTML of this challenge and implementing the hero layout for the desktop design. I would do differently the hero layout for the desktop design I think it is doable with flexbox.
What challenges did you encounter, and how did you overcome them?The challenge I encountered was building the hero layout for the desktop. I overcame this by rethinking and rewriting my HTML structure.
What specific areas of your project would you like help with?These are the areas that you can help me with:
- Hero Desktop Layout Design I used grid
- Minimizing the usage of the max-width, I think
- Fixing the image in the footer, which is not showing up despite changing the path to a relative path.
- Improving the spacing of the page.
- Proper usage of clamp().
@Arnotts33Posted 4 months agoHi @ralphvirtucio!
Very nice job on this challenge! This one was quite hard for me...
Very good use of semantic HTML and easy to read code, well structured.
Well done on the responsiveness as well :)
1 - @elekviktor32Submitted 4 months ago@Arnotts33Posted 4 months ago
Hi @elekviktor32!
Nice work on this solution!
I like the way you used grid in there on different screen sizes.
Your code is really well structured and easy to read :)
0 - @Gilbert-KoomSubmitted 4 months ago@Arnotts33Posted 4 months ago
Hi!
Good start on this challenge. The layout is looking good on desktop and mobile. I think you haven't finished the styling so I am not going to comment on it :)
As for the html, it is readable but have you ever tried using semantic html? Yours is well structured though I would have used semantic and wrap everything in a <main> tag.
Keep it up!
Marked as helpful0 - @acandaelSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
I'm proud of using the Picture element for this challenge.
What challenges did you encounter, and how did you overcome them?I had trouble getting the layout right for desktop. For some reason the Picture element wouldn't scale to 50%. The only way I could fix it was by setting the image element to 60% and the content div to 50%.
What specific areas of your project would you like help with?I could use some feedback on how to make the images responsive.
@Arnotts33Posted 4 months agoHi!
Very nice solution :)
It is nice that you started using semantic html and how you structured it. Maybe you can try to extend it to the whole html file (section, button, ...).
Things I see that you could add or change:
- Responsiveness
- The category in capital letters
- The original price centered with the discount price (you could use flexbox here)
Good job!
Marked as helpful0 - @MeemByProxySubmitted 5 months ago@Arnotts33Posted 4 months ago
Hi!
Nicely done with your recipe!
Your code is nicely structured. The only thing I would say is, is there a reason why you don't use semantic html?
The look of is good in terms of structure. Maybe you could add a bit of top and bottom margins to the card to give it more space and a bit of line-space as well for more readability :)
I think you forgot to do the responsive part.
Nice job!
0 - @maxkdavisSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
Continuing to leverage the power of CSS Flexbox.
What would I do differently next time?
- Explore CSS Grid
- Use CSS Variables as I repeat typing the color code in a few CSS rules
@Arnotts33Posted 4 months agoHi Max!
Your solution is spot on with the design!
Your code is clean and easy to read, I am trying to get mine as clear and readable as yours :)
For the buttons, I used <a> tags to put directly the link in the html, as these are supposed to be linked to websites. I don't know if that's important.
Good job on the responsiveness as well.
I agree with you on the use of CSS variables and rules to prevent repeating color code.
Great to see your work!
0 - @rvj1Submitted 5 months agoWhat are you most proud of, and what would you do differently next time?
I'm proud that I was able to complete this project faster than the previous project even though, I have small mistakes and I believe I will improve in the upcoming project.
@Arnotts33Posted 5 months agoHi there!
Your solution looks good but the image is not showing. You may have a problem with the path to your image in your html.
You also forgot to add some hover features (title and box-shadow)
As for the smaller screen sizes, I think you didn't add responsiveness with media queries in the CSS.
Other than that, I think your code is well structured and easy to read.
Marked as helpful1 - @zoumushiSubmitted 5 months ago@Arnotts33Posted 5 months ago
Hi there!
I can't see the responsive part (media queries) in the CSS I think you can get rid of the credits. The qr code card should be centered. Other than that it looks spot on. The code is clear and readable.
Keep it up :)
0
