Design comparison
Community feedback
- @numan-iftikharPosted about 2 years ago
Hey dear, I've made a pull request to your code on github. I've fixed a lot of things. Do check it out and remember to read the pull request message :)
0 - @correlucasPosted about 2 years ago
πΎHi Mysooma Maqbool, congratulations for your first solution!π Welcome to the Frontend Mentor Coding Community!
Great solution and great start! By what I saw youβre on the right track. Iβve few suggestions to you that you can consider to add to your code:
1.Fix the alignment of the whole content using
flex
andmin-height
to manage the vertical alignment and make everything centered.First of all putmin-height: 100vh
to thebody
to make the body display 100% of the viewport height (this makes the container align to the height size that's now 100% of the screen height) size anddisplay: flex
eflex-direction: column
to align the child element (the container) vertically using the body as reference.body { min-height: 100vh; background-color: rgb(167, 189, 196); display: flex; align-items: center; justify-content: center; flex-direction: column; }
2.Add the
alt text
to allow screen readers to recognize that img. Adding alternative text to photos is first and foremost a principle of web accessibility. Visually impaired users using screen readers will be read an alt attribute to better understand an on-page image.3.Replace the
<div>
youβve used to wrap the card container with<main>
, that in this case is the best tag and shows that this is the main block of content of this page.4.Instead of using
<b>
for the heading, it's better you use<h1>
since this is the main heading for this page. Remember that any page needs ah1 heading
to display which is the more important text. Note that you need to increase the headings by one level, like h1, h2, h3 to show the titles hierarchy and you cannot have more than one h1 heading.5.Add the correct color for this background, the value is
background-color: #D6E1F0
βοΈ I hope this helps you and happy coding!
0 - @denieldenPosted about 2 years ago
Hi Mysoona, congratulations on completing the challenge, great job! π
Some little tips for optimizing your code:
- add
main
tag and wrap the card for improve the Accessibility - you can use
article or div
tag instead ofp
to the container card for improve the Accessibility p
is only for contain textimg
element must have analt
attribute, it's very important!- remove all unnecessary code, the less you write the better as well as being clearer: for example the
a
container of image in this case - remove all
margin
from card container (in this case thep
) - use flexbox to the body to center the card. Read here -> best flex guide
- after, add
min-height: 100vh
to body because Flexbox aligns child items to the size of the parent container - instead of using
px
use relative units of measurement likerem
-> read here
Hope this help! Happy coding π
0 - add
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord