Skip to content
OverviewSolutions
7
Comments
3
Daniel
@Catalyst497

All comments

  • Jose Nectali•180
    @Necta-glitchSubmitted about 2 years ago

    The most diffcult to my was the hover over the image with the icon

    Tailwind css

    1
    Daniel•170
    @Catalyst497Posted about 2 years ago

    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
    0
  • Marit•400
    @MaritxxSubmitted over 2 years ago

    Hi!

    I finished the order summary challenge. The main question I had this time was regarding the best practice of (consistent) spacing between elements. Where do you apply padding or margins? Should I create a new class or apply it directly to the elements? And in doing so, do I just give each a padding-top or should I use the shorthand padding property?

    Any general feedback is also appreciated!

    Order Summary using Flexbox

    1
    Daniel•170
    @Catalyst497Posted over 2 years ago

    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.

    1
  • Shekinah•140
    @Shekinah007Submitted over 2 years ago

    Getting the image size right and scale correctly for different screen sizes was kind of annoying. Wasn't expecting that.

    CSS FLEX

    2
    Daniel•170
    @Catalyst497Posted over 2 years ago

    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
    1