@DylandeBruijn
Posted
@ownedbyanonymous
Hiya! 👋
Congratulations on your solution, it looks very close to the design! I can tell you put a lot of effort into it.
Things I like about your solution 🎉
- Use of semantic HTML elements
Things you could improve ✍️
-
I suggest adding a bit of
padding
to yourbody
element so the card has some space around it on smaller viewports. -
You could add a
min-height: 100vh
to yourbody
element so it takes up the full height of the viewport while still being able to grow when the content inside it grows. -
Try experimenting with CSS variables, they help you make your CSS values more reusable across your code.
-
I suggest using clear descriptive CSS classes like
.card
,.card-title
and.card-description
. -
You don't need
width: 100vw
on yourbody
.body
is ablock
element which take up the full width of their parent by default. -
Try making your solution more responsive.
-
I suggest not setting a fixed
height
andwidth
on elements. If the content in these elements grows larger you'll run into overflow issues. Try keeping theheight
atauto
whichblock
elements have by default. -
I suggest not using the
small
element and replacing it by a heading or paragraph element. -
I suggest replacing the
address
element by adiv
. Theaddress
element is for addresses.
I hope you find my feedback valuable, and I would appreciate it greatly if you could mark my comment as helpful if it was! 🌟
Let me know if you have more questions and I'll do my best to answer them. 🙋♂️
Happy coding! 😎
Marked as helpful
@ownedbyanonymous
Posted
Hie @DylandeBruijn 👋🏾 thank you so much for your feedback, it was really detailed and helpful. May you please kindly assist me with a bit of details on the first point. I get do get what you are saying, on smaller devices the card touches the right margin of which some padding and for the card to remain centered without having to scroll. I have an on pull-request on GitHub please feel free to leave some comments.