Design comparison
Community feedback
- @PhoenixDev22Posted over 2 years ago
Hello @MohamedWagdy7 ,
-
For any decorative images, each
img
tag should have emptyalt=""
andaria-hidden="true"
attributes to make all web assistive technologies such as screen reader ignore those images. In this case, all images are decorative only. -
To tackle the accessibility issues. you can add a
<h1>
withclass="sr-only"
(Hidden visually, but present for assistive tech).
.sr-only { border: 0 !important; clip: rect(1px, 1px, 1px, 1px) !important; -webkit-clip-path: inset(50%) !important; clip-path: inset(50%) !important; height: 1px !important; margin: -1px !important; overflow: hidden !important; padding: 0 !important; position: absolute !important; width: 1px !important; white-space: nowrap !important; }
-
don't capitalise in html, let css text transform take care of that. Remember screen readers won't be able to Read capitalised text as they will often read them letter by letter thinking they are acronyms.
-
I don't recommend to use
<br>
(use margin and padding wll do to the elements) -
border-radius
andoverflow hidden
to the container that wraps the three cards.so you don't have to set it to individual corners. -
You can use flexbox properties and min-height: 100vh; to the body to center the component on the middle of the page.(no need for padding )
body{ display:flex; align-items: center; justify-content: center; width: 100%; min-height: 100vh;
-
use unitless numbers for line-height values to avoid unexpected results. The MDN documentation explains it well
-
Never use
px
for font-size. -
what about the mobile solution ? it t should be a flex column.
Hopefully this feedback helps
Marked as helpful1@MohamedWagdy7Posted over 2 years ago@PhoenixDev22 what a great benefit. thanks, bro.
1 -
- @denieldenPosted over 2 years ago
Hi Mohamed, great job!
To make it look as close to the design as possible add
border-radius: 1rem 0 0 1rem;
toone
class andborder-radius: 0 1rem 1rem 0;
tothree
class.For center the card remove
margin
frommain
tag and move the flex property to the body. Also setheigth
of body to100vh
because Flexbox aligns to the size of the parent container.Hope this help and happy coding!
0
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