@ErayBarslan
Posted
Hey there, congrats on your solution! Design looks good, responsive and you have clean usage of CSS. Nothing much to add but some suggestions:
- If you wish to place the attribution to bottom of page without effecting your container's placement you can use:
.attribution {
...
position: absolute;
bottom: 0;
}
body {
...
justify-content: center;
}
/* In this case you can place your container to center */
- Background color is missing on body:
background-color: var(--Very-light-gray);
Although arguably white looks better so you might as well leave it as it is. - Instead using <div> on
.container
and.attribution
you can use semantic elements to make the page accessible:<main class="container">
&<footer class="attribution">
- You shouldn't leave
alt
empty. Screen readers skips empty alt. Since images on this challenge are for decorative usage, nothing wrong regarding accessibility. But your page might get lower SEO. Instead you can use like :<img src="./images/icon-sedans.svg" alt="sedan" aria-hidden="true"/>
- On production, when you dirct links with
target="_blank"
, addingrel="noopener noreferrer"
will make it secure to attacks. Aside these nothing I'd add, happy coding :)
Marked as helpful
@trandainien
Posted
@ErayBarslan thank you, i will keep them in mind <3 your comment is really helpful