P
I feel Struggle• 190
@Kein-Internet
Submitted
P
Margaux
@margaux-works
All comments
Hi there,
your solution looks great! Very close to the design which was provided, good job :)
I checked your code, and found a few potential optimisations:
- you could use <header> for your titles for a better semantic structure.
- your card are mixed with <section> and <div>. You could have a better consistency by using section for all cards elements for example.
- there is not alt attributes for your images
- you could put all your reset rules in a separate css file (i.e reset.css) so that it gives more clarity on what is your personal style and what are the reset rules.
I hope this helps! Keep up the good work :)
Marked as helpful
0- ICode88• 280
@ICode88
Submitted
Hey Icode 88,
your solution looks great! I can tell you put a lot of effort into it, it looks super close to the design :)
Honestly, I don't have much to say, except a few minor things I noticed that could be optimised:
- The picture element styles (display: flex) are unnecessary since it's just a container for the images. The child img will take the full width naturally due to the parent main being a flex container.
- In the media query, I would recommend setting up a min-width on .info to ensure that the content doesn’t shrink too much on smaller screens.
Keep up the good work!
0 - Dimitri V• 370
@Dubidimi
Submitted
What are you most proud of, and what would you do differently next time?
ok
What challenges did you encounter, and how did you overcome them?
ok
What specific areas of your project would you like help with?
ok
Hi Dimitri,
congratulations in submitting this solution, good job!
I had a look into your solution and noticed a couple of things that could be optimised.
- your HTML follows a clear and standard structure, which is really good!
- You also used semantic headings which is great for accessibility and SEO.
- you have linked twice the preconnect. You could remove one and only keep one.
- your image is missing an alt text which is important for accessibility and SEO.
- there is an excessive use of <br> tags for spacing, which is generally discouraged. CSS is usually a better solution to handle spacing.
- the table does not follow exactly the design that was provided to us. I can see you also created div, but here I would recommend to use the table tag. You can add the missing border by adding a border bottom on the td.
- your page currently looks nice on desktop, but is not responsive and adjusting well on smaller screens. I would recommend implementing media queries for this.
I hope this is helpful!
Keep up the good work :)
0 - Maryam Garba• 70
@masha-a-m
Submitted
What are you most proud of, and what would you do differently next time?
I would use grid so that i can understand it.
What challenges did you encounter, and how did you overcome them?
I couldnt center the profile card, and the buttons kept being too large on different mobile screens
Hi Maryam,
Congratulations on submitting this solution! It looks fantastic and very close to the design. Well done! 😊
You have a very clear HTML structure, and I appreciate your use of semantic HTML elements. You also ensured your design adapts to different screens by using media queries. Great job on these aspects!
Here are a few suggestions for further optimization:
-
Use Relative CSS Units: Instead of using pixels, try using relative units like rem and em to make your layout more adaptable to different screen sizes. This approach improves the responsiveness of your design.
-
Semantic Links: Instead of using buttons, consider placing your links within an unordered list to enhance the semantic structure of your code. Additionally, make sure to add actual hyperlinks to these items for better functionality.
-
Flexbox Gap Property: You can use the Flexbox gap property to set the spacing between your elements more efficiently. This property simplifies the spacing management without the need for margins.
-
Hover Animation: To make the transition of background color and text color smoother, you can add a hover animation using CSS transitions. The ease timing function can help achieve a smoother effect.
I hope you find my feedback helpful!
Keep up the great work and happy coding! 🌈
Cheers, Margaux
Marked as helpful
1 -
Hi Grymbo,
Congrats on completing the challenge! ✅
Your solution is looking really close to the design, well done :)
I have noticed a few things that could be optimised:
- your HTML is well structured and organised. Keep in mind that you can use semantic HTML to make it more clear and readable. For example, you can use <main> to wrap your main content instead of <div>.
- in the design, the card has a border of 1px (solid). This is currently missing from your development.
- there is currently a blur applied to your shadow, while on the design the shadow does not have any
- in the hover state, I believe the only change is the color of the main title. Currently, there is an hover effect applied to your border shadow.
I hope this is helpful and keep up the great work!
0- Shubham Pratik• 40
@shubhamr10
Submitted
Hello!
good job in posting your first solution :)
What I noticed:
- you correctly implemented the border radius for the card, but a border radius is missing on your image (see original design provided)
- in order to use semantic html, you could use <main> instead of a simple <div> for the card
- for your text, consider using headings such as <h1> for your main title, so that you are structuring your content effectively.
The last 2 comments would not optimised how your page looks but would be helpful for SEO and accessibility purposes.
Hope this helps!
Marked as helpful
0
