Design comparison
Solution retrospective
I think the eye icon on the overlay image is not well centered, How can I solve this? Is ok the opacity of the eye icon?
any other suggestion will be well received.
thanks
Community feedback
- @antaryaPosted about 2 years ago
Hi 👋,
Good work on this challenge 🚀.
I have a couple of suggestions/improvements:
HTML
[1. The anchor element cannot have another anchor element as a child.
- Check "Permitted content" paragraph https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#properties
CSS
[2. As
.overlay
has already correct position and size, one way to center child elements would be:.overlay { display: flex; justify-content: center; align-items: center; }
The icon itself does not need any styling in this case except the colour.
[3. While the solution for centering above is different, it would be helpful to understand the cause of the previous issue.
The child anchor element's height is calculated using
font
,font-size
andline-height
, and thenimg
is placed on the text baseline. You can try to changevertical-align
on the image to see how it changes the position. The height is huge compared to the image, and the image is placed similar to how the text would be placed. You can, for example, fix that by settingfont-size: 0
on the anchor element, but in your case, you just remove the child anchor element. Here is related material:- First two paragraphs explain how the image is placed and what are default values - https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#styling_with_css
- Useful technic to deal with space under image - https://stackoverflow.com/questions/31444891/mystery-white-space-underneath-image-tag/31445364#31445364
- line-height and default value - https://developer.mozilla.org/en-US/docs/Web/CSS/line-height#values
- Deep dive CSS: font metrics, line-height and vertical-align - https://iamvdo.me/en/blog/css-font-metrics-line-height-and-vertical-align
[4. When the viewport height is small, the card is only partially visible, and there is no scrollbar. You can change the body element
height
style tomin-height
. Also, a bit of padding applied to themain
element will add space otherwise on smaller screens it is on the edge of the viewport.General
[5. Use css reset to have a good starting point, e.g. https://piccalil.li/blog/a-modern-css-reset/.
I hope this will be helpful.
Keep up the good work 🚀. Cheers!
Marked as helpful1@damiandev22Posted about 2 years ago@antarya thanks for your time. your suggestions are valuable.
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