Design comparison
Solution retrospective
I would appreciate some feedback :)
Community feedback
- @PhoenixDev22Posted over 2 years ago
Hello David Trstenjak,
Great work! I have some suggestions regarding your solution:
HTML
-
First of all , you have used so may unnecessary <div> and <span>’s. Consider using semantic elements before these generic containers.(it’s not recommended to use
<div>
and<span >
alone to wrap meaningful content) -
There should be two landmark elements as children of the body element - a
main
(which will be the NFT component ) and afooter
(which will be the attribution).<Footer>
should not be in the<main >
. -
Page should contain a level-one heading. In this challenge , as it’s not a whole page, you can have
<h1>
visually hidden withsr-only
class and use<h2>
forEquilibrium #3429
Or you can use<h1>
for theEquilibrium #3429
-
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>
.-
Also use
<a>
to wrapJules Wyvern and Equilibrium #3429
-
The link wrapping the equilibrium image(
image-equilibrium
) should either haveSr-only
text, anaria-label
oralt
text that says where that link takes you. -
Images must have alternate text
-
For any decorative images, each img tag should have empty
alt=""
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
). -
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 -
You can use unordered list
<ul>
to wrapclass="container4"
. In each<li>
should be<img>
and<p>
, then 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 use border-top: to the avatar's part -
To use more semantic tags , you may use
<figure>
and<figcaption>
for the avatar's part.
CSS:
- These are the reason of content overflow.
width: 25%; and height: 70vh
, it differs from a browser to another.Using percentages makes you lose the control of the layout Consider using max-width to the card instead.
It's not recommended to set height to component, let the content of the component define the height.
-
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.
-
The icon view doesn’t really need to be in HTML , you can use CSS. AS there’s no need for another clutter in the HTML.
Aside these, Great work on this one. Hopefully this feedback helps.
Marked as helpful1 -
- @drAsifAli123Posted over 2 years ago
Although your design is good but always keep in mind to change the design in inspect manual and see if things look ok then implement the design accordingly. You can inspect manual in the chrome browser.
Steps:- 1)Right click on your chrome Browser page on your current project then at the last click on inspect.
2)That will open the console and in the console you can use box model to change container height to 550px and padding and margin accordingly.
Note:- Before closing the console panel make sure you know what properties to change in the final css stylesheet because in the console changes are temporary.
I hope this will help you to improve your practice.
Happy Coding🎈
Marked as helpful0 - @kxnzxPosted over 2 years ago
Hello David,
Well done making this challenge. I have some suggestions.
- Even though this is a fictive site, and maybe even part of a bigger site, it is good practice to wrap the container in main tag, the title in an <h1>Equilibrium #3429</h1> and the attribution in a footer. This is important for accessibility.
- Remove the height on the container (never give elements a specified height, unless necessary). The image is stretched. Let the content define the height. You can tweak the height by using padding on the container.
- Also remove the width on the container and do this instead: give it a max-width of 21rem; By using max-width you tell the browser to not exceed the width, but be less if he has to.
Marked as helpful0
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