Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

All comments

  • Kitalphar• 150

    @Kitalphar

    Posted

    Great job!

    Design mostly matches the design images, with some small mistakes like the desktop success-message is not centered within the parent div and the text inside the sign-up-form on the mobile view having too much inline padding, but overrall doesn't affect functionality.

    Some suggestions to fix:

    • :hover effect works well, however the same effect should be applied to :focus for keyboard navigation which is missing.
    • The red coloring with the error message after an invalid e-mail, remains even after a valid e-mail address is typed in, and it doesn't reset even after the success-message.

    Have a great day!

    0
  • Kitalphar• 150

    @Kitalphar

    Posted

    Great work!

    I like the animations on the socials and the fade-in effect for their container, however the same effect seems to be applied to the Author's details, which feels more like an error than a feature if i'm to be honest.

    Hope you have a great day!

    Marked as helpful

    1
  • tsuki - 青• 120

    @tsukiyx

    Submitted

    What are you most proud of, and what would you do differently next time?

    I feel proud of understanding Grid every time while practicing.

    testimonials-grid-section-main

    2

    Kitalphar• 150

    @Kitalphar

    Posted

    Greetings~

    Great solution, i particularly like the short and easy to read css file.

    One suggestion i have is that instead of px consider using rem for better accessibility. rem units are relative to the root element font size. People with visual impairments often increase text sizes, like their default browser font size for better readability, using rem ensures your layout remains functional.

    Marked as helpful

    0
  • Kitalphar• 150

    @Kitalphar

    Posted

    Greetings~

    Frontend Mentor's learning path requires that i leave feedback, and it choose your solution.

    Unfortunately all i see is a link to your incomplete portfolio and the source code's link is broken. I wasn't able to find the solution in your Github repositories either thus i am unable to write anything useful.

    Hope you have a great day!

    0
  • Kitalphar• 150

    @Kitalphar

    Posted

    Great job!

    I like your solution of hiding the unneeded images. I believe in this challenge one goal is to get challengers to use/learn about the srcset attribute for img/picture elements, however as i had a little trouble with that as well, i appreciate the simple solution.

    My one critique is that the button doesn't work (mainly because it's not a button) and it also doesn't change color on :hover. While your div can be made to work as a button it's simpler to just use a button element and style it according to your need.

    0
  • Kitalphar• 150

    @Kitalphar

    Posted

    Great job!

    Seems wider on desktop than in the design but other than that i think everything is well made. Sadly i have nothing useful to say.

    Have a great day!

    0
  • Kitalphar• 150

    @Kitalphar

    Posted

    Great Job!

    Looks great and close to the original design. I noticed focusing with keyboard doesn't work, the reason for this is that a <div> element can't be focused. You could add a tabindex to the <div> or by using <a> elements (instead of the current <p>) nested inside the <div> ...however in this case only the <a> will be focused, you would need to have it fill the parent for the same effect. Consider looking into the :focus-within pseudo class.

    0
  • Kitalphar• 150

    @Kitalphar

    Posted

    Greetings~

    Great work! Learning path sorta forces me to comment, which is a little funny considering both React and Tailwind is a little beyond me at this point ... The only thing i could say is that to me it seems like the mobile version of the design does not match.

    0
  • Kitalphar• 150

    @Kitalphar

    Posted

    Great work!

    Due to having used vw units for the container width the container grows wider than probably intended, but it did gave me the idea of maybe using the resize CSS property, or adding an option to enlarge the image on hover. In the future in case on the design i'm working on the image is small it could help the camera pick the QR code up better. Just food for thought...

    0