Latest comments
- @Necta-glitch@Catalyst497
Hey Jose.
I've seen your project solution and I must say, you did a good job regarding accuracy and neatness. Couldn't look at your code though. GitHub is telling me it is not found.
A few improvements you could make though... You see that icon that displays when you hover over the image, you could actually make it transition. How? You can have the two divs stacked on top of one another. Upper for the icon and lower for the image. You can then make the upper one to have an opacity of 0 on default and of 1 when hovered on, then add a transition property for the opacity for whatever duration of your choice. And voila!
You have a few issues on accessibilty, I guess you should use landmarks such as <header></header>, <div id="root"></div> in your code. Would help a bit with the accessibility.
I am also a junior developer like you and would love to see future projects that you build. This is my solution to this particular challenge: https://www.frontendmentor.io/solutions/nft-preview-card-AMDO5woVyU
Have a nice time coding.
Marked as helpful - @Maritxx@Catalyst497
Hi Marit,
I just looked at your project solution and I must say, your solution looks very detailed. I also like the fact that you removed all the default stylings to every possible HTML element with the reset CSS. Yeah, those default stylings could be really troublesome sometimes :(
Just to add, I think your body tag could use a little bit of margin to it. The way the content touches the top of the page might not look really good to the users. You could try adding a little bit of
body{ margin: 1rem }
I think you should add a common a class to elements if you want to give them a common style or better still I use this(I know it's a lazy developer thing):
parent > * {property: style}
. Now this is only a good fit if you want the direct children of an element to have a common style to them. It just works for me, you know.I hope you find this feedback helpful. Happy Coding.
- @Shekinah007@Catalyst497
Wow, I love this solution.
I just think if you were using flexbox for your navbar. You could add an align-items: center to it to make the items vertically centered.
You can find my solution here: https://www.frontendmentor.io/solutions/flexbox-plenty-of-grid-layout-_rQIc3AANQ
Marked as helpful