Skip to content
OverviewSolutions
30
Comments
10
Yuko Horita
@Sloth247

All comments

  • Yuko Horita•645
    @Sloth247Posted almost 3 years ago

    Hi Michael,

    Nice work! I like the animation effects and the fancy favicon😁

    I have the following suggestions;

    Issues in Accessibility Report

    • <div> should not be inside <a> tag. You can check from this site
    • In Navbar.js, you include <li> inside <NavLink> however, <NavLink> should be inside <li> and <li> must be directly under <ul>. I think these will fix your issues in the report.

    I am sorry, I maybe a bit of OCD, I think the font weight of h2 in Destination.js needs to be 400 and give more letter-spacing. Also please give more margins/paddings between the planet choice buttons.

    I think the website will be much closer to the design if you utilise the figma file, as this is free+ challenge.

    I appreciate if you give some feedback to my solution too!

    Marked as helpful
    0
  • Yuko Horita•645
    @Sloth247Posted over 3 years ago

    Hi JibbyCodes, this is the best solution that I've ever seen on FEM for this challenge. I may need to revise my coding based on your solution 😁 I'm sorry if I am wrong but the useEffect for the theme in your context file should be "useLayoutEffect". I did a bit of research after finishing my solution and according to official React documentation, "useLayoutEffect will be flushed synchronously, before the browser has a chance to paint.", so this hook might be good to use there.

    I appreciate if you give me feedback on my solution too 😊

    Your solution is really nice.

    Marked as helpful
    0
  • Yuko Horita•645
    @Sloth247Posted over 3 years ago

    I like your animation, and would like to use GSAP to my future projects! You included my profile icon which is funny 😋

    1
  • Yuko Horita•645
    @Sloth247Posted over 3 years ago

    This is just a question but why do you use form for the price card?

    0
  • Yuko Horita•645
    @Sloth247Posted over 3 years ago

    It looks awesome! I like your banner on Github too. I also tried this challenge a while ago and could not close the panels by only HTML and CSS (without JS). I appreciate if you give me a feedback on mine too!

    Marked as helpful
    0
  • Yuko Horita•645
    @Sloth247Posted over 3 years ago

    Hey, I have just done the same challenge, I achieved the similar design with the original in footer with psudo element ::after. Please see my solution if you would like! https://www.frontendmentor.io/solutions/meet-landing-page-w-sass-sa598Ef7n

    0
  • Yuko Horita•645
    @Sloth247Posted over 3 years ago

    Animation looks awesome, but I cannot click the dropdown menu when it's in desktop view size. All interactive elements are focusable by keyboard but menus in nav bar cannot be selected by pressing Enter key. Github link shows error, so I cannot check your coding.

    Marked as helpful
    0
  • Yuko Horita•645
    @Sloth247Posted over 3 years ago

    Hi, it looks good. However I still see the modal is kept open when I move from mobile view to desktop view. I suggest you set up conditions regarding media query in JS or convert navigation to modal in CSS on media query for mobile view.

    Marked as helpful
    0
  • Yuko Horita•645
    @Sloth247Posted over 3 years ago

    I see the wrong error message "Looks like this is not an email" when I press submit without input anything in email section. This should be "email cannot be blank". Also, after inputting correct information and press submit button, the error messages are still shown.

    0
  • Yuko Horita•645
    @Sloth247Posted over 3 years ago

    The error message "Looks like this is not an email" in the original design does not show.

    Marked as helpful
    1