Design comparison
Solution retrospective
Any feedback would be highly appreciated
Community feedback
- @PhoenixDev22Posted over 2 years ago
Hello @jonathan401,
Congratulation on completing this frontend mentor challenge.
I have some suggestions regarding your solution:
-
First of all , you have used some extra
<div>
. -
Anything with a hover style in design means it's interactive. you need to add an interactive element
<a>
around the image ,Equilibrium #3429
. -
the
icon-view
doesn't really need to be in the html, you could do it with css. If you want it to stay in html it needs to be aria-hidden or role presentation with empty alt. -
There are so many ways to add the hover effect on the image , The one I would use, using pseudo-elements to change the teal bg color to a hsla. Then opacity can be changed from 0 to 1 on the pseudo element on hover as there is no reason to have the extra clutter in the html.
The link wrapping the equilibrium image should either have
Sr-only
text, anaria-label
oralt
text that says where that link takes you.-
For any decorative images, each img tag should have empty
alt=""
andaria-hidden="true"
attributes to make all web assistive technologies such as screen reader ignore those images in(icon-view, icon-ethereum, icon-clock
). -
To use more semantic tags , you can use <ul> to wrap
class="card-info"
and in each<li>
there would be<img>
and<p>
. -
The avatar's alt should not be
"avatar"
It's meaningless , you can use the creator's nameJules Wyvern
. Read more how to write an alt text -
Remove
overflow: hidden
from the body as *. It hides real content potentially, and will eventually create problems with readability and responsivity, it allows for images and text leaving the viewport halfways and is in general bad for accessibility. It won't adapt well on tiny screens. -
Add display : block to the image that removes the little gap i see under the image.
.card-content .card-image img
-
font-weight: 300px;
remove the px for font weight. -
If you set a page width, choose 100% over 100vw to avoid surprise horizontal scrollbars.
-
font-size: 62.5%;
this has huge accessibility implications for those of us with different font size or zoom requirements. It's recommended not change the html or root font size.
General points :
-
Remember a css reset on every project. That will do things like set the images to display block and make all browsers display elements the same.
-
Really important to keep css specificity as low/flat as possible. The best way to do styling is single class selectors.
-
It's recommended to use
em
andrem
units .Bothem
andrem
are flexible, Usingpx
won't allow the user to control the font size based on their needs.
Overall , your solution is good. Hopefully this feedback helps.
Marked as helpful0@jonathan401Posted over 2 years ago@PhoenixDev22 Thank you so much for this. I will definitely get the project fixed up
0@jonathan401Posted over 2 years ago@PhoenixDev22 so I was able to fix most of the code. The only problem I encountered was that I really couldn't find a way to apply the hover effect to the image using pseudo selectors.
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