Raymart Pamplona• 16,140
@pikapikamart
Posted
Hey, great work on this one. Layout in general looks great, responds well but for mobile view, the layout could adapt to small size.
I like pizza and for some suggestions on the site if you would like as well:
body
tag has noheight
because the main content isposition: absolute
which is not great. Avoid usingposition: absolute
on large element on your site since this makes the element out of the flow, you don't really need this stylings on the.card
:
position
left
right
transform
margin
top
On the body
tag just use:
align-items: center;
display: flex;
flex-direction: column;
justify-content: center;
min-height: 100vh;
- Always have a
main
element to wrap the main content of your page. On this one, the.card
should be using themain
instead ofdiv
. - Also, use
max-width: 350px
on the.card
instead ofwidth: 350px
so that it will adapt even in small screen's width. - Those decorative images on the site could have use an extra
aria-hidden="true"
attribute so that they will be totally hidden. - A page must have a single
h1
on a page. Since there are no text-content that are visible that could beh1
, you will make theh1
screen-reader only text. Theh1
text should describe what is the main content is all about and this would be placed as the first text-content inside themain
element. annual-plan
could use a heading tag since it gives overview on what the section will contain.
Aside from those, site looks great.
Marked as helpful
1
Alex Daniel• 650
@p-alex
Posted
Thanks for the suggestions @pikamart! I'll try to fix everything!
1