Darkstar• 1,070
@DarkstarXDD
Posted
Looks nice. Some suggestions.
- Always use
rem
forfont-sizes
. That way if the user changes their default browser font-size, the text in your website will scale accordingly. This won't happen if you usedpx
for font-sizes. - Try to avoid having text inside
div
orspan
alone. They are meaningless elements used for grouping or styling purposes. Change that quote text to ap
element or better, for ablockquote
element. You can usespan
if you want to select some text that is already inside ap
. - There should be an anchor (
<a>
) element inside those list items, because if a user clicked on one of those social media names they should be taken into that site. Otherwise having those social media names there would be useless. - That horizontal padding on the list items is huge. Is there a need for much padding? Whenever you are using that big amount of padding, specially in a small component like this, it should be a sign you are doing something wrong.
- The card shouldn't have a
min-width
. It should have amax-width
if needed. Because of that massive padding and themin-width
on the card, the card is not responsive in small screen sizes. Ideally a site should look good (responsive without any overflow issues) until around 320px screen sizes. - I would give a small padding to the
body
so that the card won't hit the edges of the screen.
Marked as helpful
2