@pikapikamart
Posted
Hey, awesome work on this one. Layout in general looks great.
Other already gave their feedbacks on this which is nice, just going to add some suggestions as well:
- Always have a
main
element to wrap the main content of your page. On this one, the.container
should be using themain
instead ofdiv
. - On the
.container
instead of usingwidth: 22rem
usemax-width: 22rem
so that it will adapt to a small screen-width, because by usingwidth
you are setting a fixed size. footer
should be placed on its own row in the markup and not inside the.container
which is supposed to be a main. A typical structure of a site looks like:
<header />
<main />
<footer />
This way, all element that has content are inside their respective landmark elements.
- Vector image should be hidden since it is decorative only. Decorative image must be hidden at all times by using
alt=""
and extraaria-hidden="true"
attribute on theimg
tag. - Also when using
alt
attribute, avoid using words that relates to "graphic" such as "illustration" and others. Animg
is already an image/graphic so no need to describe it as one. - Music-icon should be hidden so use the method I mentioned above.
- Lastly, just maybe making the layout a bit bigger like the original and the interactive elements be bigger as well.
Aside from those, great work again on this one.
Marked as helpful
@mikeyxx
Posted
@pikamart Thank you so much! Most of these are new to me especially about hiding decorative images. I will definitely lock them down and remember to follow these specifications going forward in my subsequent projects. I have made all the edits you suggested and I think my output looks so much like the design now.