Design comparison
Solution retrospective
The hr isn't as thin as the design and I don't know how to resize it.
Community feedback
- @PhoenixDev22Posted over 2 years ago
Hello Ogunola Zainab Ololade,
Congratulation on completing this challenge.
I have some suggestions regarding your solution:
HTML
-
Page should contain a level-one heading. You can use
<h1>instead of <h3>in this challenge (as it's not a whole web page). -
Since there's a :hover state on the image and means it's interactive, So there should be an interactive element around them. When you create a component that could be interacted with a user , always remember to include interactive elements like(button, textarea, input, ..)
for this imagine what would happen when you click on the image, there are two possible ways:
1: If clicking the image would show a popup where the user can see the full NFT, here you use
<button>.`2:If clicking the image would navigate the user to another page to see the NFT, here use
<a>.-
The link wrapping the equilibrium image should either have
Sr-onlytext, anaria-labeloralttext that says where that link takes you. -
For any decorative images, each img tag should have empty
alt=""as you did and addaria-hidden="true"attributes to make all web assistive technologies such as screen reader ignore those images in(icon-view, icon-ethereum, icon-clock). -
In each
<li>it would better to wrap the text in<p>. So there should be<img>and<p>, that way you can align them centrally. -
If you wish to draw a horizontal line, you should do so using appropriate CSS. Remove the
<hr />, you can useborder-top:to the avatar's part.card-footer.you can read in MDN -
The avatar's alt should not be empty. You can use the creator's name
Jules Wyvern. Read more how to write an alt text -
To use more semantic tags , you may use
<figure>and<figcaption>for the avatar's part.
CSS:
-
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.(you can remove filter: brightness(250%);)
-
In order to center the card on the middle of the page , you can use the flexbox properties and
min-height: 100vhfor the<body>add a little padding to the body that way it stops the card from hitting the edges of the browser (no need for position absolute and margin: auto). -
Really important to keep css specificity as low/flat as possible. The best way to do that is single class selectors.
-
Do not use the id's to target the elements on CSS , using ID's creates problem due to the specificity , always use class so that it could be more manageable and reusable.
-
Consider using rem and em units as they are flexible, specially for font size If your web content font sizes are set in absolute units, such as pixels, the user will not be able to re-size the text or control the font size based on their needs. Relative units “stretch” according to the screen size and/or user’s preferred font size, and work on a large range of devices.
-
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.
Overall , your solution is good. Hopefully this feedback helps.
Marked as helpful0 -
Please log in to post a comment
Log in with GitHub
Join 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