Raymart Pamplona• 16,140
@pikapikamart
Posted
Hey, really nice work on this one. The overall layout looks great on this one.
Here are some suggestions for the site:
- Right now, there is an error on the console. It looks like the font-family that you are importing. Might want to check that one out.
- The
body
tag could omit thefont-size: 16px
since the default is using a 16px. - Avoid using
height: 100vh
on a large container like themain
as this makes the element's height capped based on the viewport/screen's height. Instead usemin-height: 100vh
so that the element will expand if it needs to. - Currently, if you zoom out on your screen, the layout's size/height gets small. For this one, I find the
width: 30%
usage on the.card
causes this because as the size screen gets big, the layout responds to its size and gets bigger as well, creating more space inside the card and letting the content expand as well and just occupying lesser rows. For this one, you could use arem
unit on themax-width
and notwidth
:
max-width: # your choice;
width: could use 100% since it is a flexbox
- The music-icon on this is only acting as decoration. Decorative images are just images that doesn't contribute to the overall content of the site. They should be hidden for screen-reader at all times by using
alt=""
andaria-hidden="true"
to theimg
tag or onlyaria-hidden="true"
if you are usingsvg
instead ofimg
tag. - The
annual plan
text could use a heading tag since it gives information about the section's content. The pricing below it could sit on another tag outside theannual plan
:
<div>
<h2>Annual Plan</h2>
<p>Pricing in here</p>
</div>
- Lastly, the
.attribution
could be removed from themain
tag since it doesn't really belong in there. The.attribution
could usefooter
so that it will be inside a landmark element.
Aside from those, great job again on this one.
Marked as helpful
0