Design comparison
Solution retrospective
Hi, Im newbie coder who want to become front end dev. In this project I have a few difficulties that I couldn't figure out.
- Overlay is somehow not matching with the image position.
- I want the card to be vertically centered when opened with smaller display.
I appreciate your help and feedback on my project. Thanks!
Community feedback
- @PhoenixDev22Posted over 2 years ago
Hello Rina ,
Congratulation on completing this frontend mentor challenge.
I have some suggestions regarding your solution:
-
To tackle the accessibility issues: Page should contain a level-one heading . In this challenge , you can use
<h1>withclass="sr-only"(Hidden visually, but present for assistive tech). -
There should be two landmark components as children of the body element - a
main(which will be the NFT card ) and afooter(which will be the attribution).<Footer>should not be in the<main >. HTML5 landmark elements are used to improve navigation . -
All the card content is related , you have misused the article tag, Maybe the article tag for the whole card would be right but definitely not for a part of it . read more about the article usages in mdn .
-
Anything with a hover style in design means it's interactive. you need to add an interactive element
<a>around the image ,Equilibrium #3429 and Jules Wyvern. -
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). -
No need to mention
imagein the alt text as It’s going to be obvious to either a person or a machine when something they're accessing is alt text. Read more how to write an alt text -
The link wrapping the equilibrium image should either have
Sr-onlytext, anaria-labeloralttext that says where that link takes you. -
the
icon-viewdoesn'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.
-
To center the card on the middle of the page use flexbox properties and
min-heightto the body.And remove the margin from the .card
body { .... display: flex; align-items: center; justify-content: center; min-height: 100vh; } add display block to the img that would remove the little gap I see under the image img { max-width: 100%; display: block; }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
emandremunits .Bothemandremare flexible, Usingpxwon't allow the user to control the font size based on their needs.
You might have a look at my solution , to see how I did the hover effect on the image , it might help. Overall, your solution is good. Hopefully this feedback helps
Marked as helpful2@dodrinPosted over 2 years ago@PhoenixDev22 Hi Thank you so much for such detailed feedback! This is super helpful!!
0 -
- @Njoura7Posted over 2 years ago
hello, I 've just finished this project (and I couldn't submit it idk why) anyways those are two methods to center your element (horizontally and vertically) set the parent element (the element that contains the specific element that you want to center) to display flex for example let's say you want to center a <div> inside a <body>: body{ display:flex; justify-content:center; /horizontally/ align-items:center;/vertically/ } but be careful display flex will center all the elements, so in this case, you have to use only one container inside the body then nest the rest of the elements in it
second method go to the element that you wanna center: let's say it's a <div> div{ position:absolute; left:50%; top:50%; transform:translate(-50%,-50%); }
hope that was helpful :)
Marked as helpful1@alisbaiPosted over 2 years ago@Njoura7 I'm having the same trouble with other projects, I can't submit them too.
0@Njoura7Posted over 2 years ago@alisbai I think there is a bug or something in the website :/ the thing is when I submitted I wrote a whole paragraph and now everything is gone I guess hh let's hope it'll come back tomorrow
0@Njoura7Posted over 2 years ago@alisbai I think there is a bug or something in the website :/ the thing is when I submitted I wrote a whole paragraph and now everything is gone I guess hh let's hope it'll come back tomorrow
0@dodrinPosted over 2 years ago@Njoura7 Hi Aziz, Thank you for your advice. I just added updated my css but yeah it seems like there is bug on the website. i cant get a new screenshot. Anyways I managed to fix the issue with center the element. Cheers
0@alisbaiPosted over 2 years ago@Njoura7 hh, that happened to me twice. I have now to projects I need to submit. :)
0 - @MaryahceePosted over 2 years ago
Nice work. .container { height: 200px; position: relative; border: 3px solid green; }
.vertical-center { margin: 0; position: absolute; top: 50%; -ms-transform: translateY(-50%); transform: translateY(-50%); } Try this for smaller screen. Make sure you give the container a position of relative first.
Marked as helpful1 - @Luc0903Posted over 2 years ago
I tried making this challenge a week ago but had some trouble with images, do not look at my job haha. About yours... The overlay is something I'm struggling too, so I cannot really tell you anything about it. About the position, there is a property called vertical-align in css, but it does not work on this things. Pay attention to the border radius of the image and the font-weights you should use
Marked as helpful0@dodrinPosted over 2 years ago@Luc0903 Thanks for your reply:) good to know im not the only one struggling haha Happy coding!
0 - @shashreesamuelPosted over 2 years ago
Hey good job completing this challenge, keep up the good work
Your solution looks great however I think your card needs some margin from the top using the
margin-topproperty.In terms of your accessibility issues, simply wrap all your content between
maintags.I hope this helps
Cheers Happy coding 👍
1@dodrinPosted over 2 years ago@TheCoderGuru Thank you for your feedback! Its really motivating to receive comment on my work:) Cheers🙌
0 - @cjdemillePosted over 2 years ago
The overlay can be way trickier than you'd expect for being pretty simple CSS to write. One trick that will work with your code would be to set the font-size property on your .main-image class to 0.
It seems weird because there's no text in the image, but I remembered that when I was taking classes, that a return in your HTML can become a space.
It looked like your .main-image element was a couple pixels larger than the image, so I tried it out in dev tools and it worked.
It looks like you've already corrected the issue with verticle centering. I almost always forget to set the cross axis height of a flex element, but setting a height and width and then using the display of flex and justify-content: center and align-items: center should do the trick every time.
1@dodrinPosted over 2 years ago@cjdemille Hi! Thank you for the tip. Definitely something I never thought of. Cheers happy coding!
0
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