@pikapikamart
Posted
Hey, great work on this one. Layout in desktop is good, the mobile is good as well. However, your breakpoint for mobile layout is too early. Adjust those because your desktop layout could be shown more before going into mobile layout.
darryn already said a great feedback, i'm just going to add few as well:
- Instead of using
margin
to center the content on your.card
selector, you might want to use:
main {
align-items: center;
display: flex;
justify-content: center;
min-height: 100vh;
padding: add some to the top and bottom
}
Because as you may see, if you inspect your layout in dev tools, hover on the body
or main
, it doesn't really capture the whole layout, because margin
doesn't accumulate to an element's dimension, unlike padding
. The above stylings also will always make sure that the content is centered properly.
- The
h3
tag could be anh1
element on this. For every webpage, there should at least be 1h1
. Also, avoid skipping a heading tag level. If you usedh3
, make sure that there is ah1
and there are/ish2
. Do not skip a level. - Those 3 website information could use a
ul
element, since those are list of data about the company right. If there are elements on your layout, that seems a list,ul
element is the right pick. - The
alt
value of theimg
should have a more descriptive value. Also, avoid adding words that relates to "graphic" like "icon, logo, image, picture" do not add them as a value in thealt
attribute. Assistive tech will handle those for you. - Lastly, changing the breakpoint will be really awesome.
Still, you did a good job in here.
Marked as helpful