Skip to content
OverviewSolutions
3
Comments
7
Renaldy Prasetio
@iceofice

All comments

  • Renaldy Prasetio•70
    @iceofice

    Hi there! Wow, your animation is sick! But yeah I think it is overkill for this design but maybe try to adjust the speed? I am not too sure about it. I also see on your mobile hamburger, it broke, I can see the error on the console, maybe you can check it out and try to fix it. In your mobile design, your paragraphs are broken, some are out of view some is too large (try to adjust the padding!). Other than that good job! I am new here so I hope it helps, I would love to learn those sick animations though hahaha. Hope we can help each other out in the future!

  • Renaldy Prasetio•70
    @iceofice

    Hi there!! I also just finished this challenge and I noticed that you liked my solution. Thank you for that! Great job finishing this challenge, everything looks good. I can give feedback about the submenu on the desktop design, instead of making the user need to click the menu then see the submenu, I would much prefer just showing the submenu on hover. That's all for me. Hope we can help each other out in the future!

    Marked as helpful
  • Renaldy Prasetio•70
    @iceofice

    Hi! I also just finished this challenge. I have some feedback for you. I can't find your mobile design and your page is not really responsive. When I tried making it smaller the image is gone or moved outside of the view. For me personally, I like to use overflow-x: hidden; properties and play around in different sizes. In the menu, I think it will be a better idea to show the submenu on hover also. That's all from me! Hope we can help each other!

  • Renaldy Prasetio•70
    @iceofice

    Hi, Congrats on finishing this challenge! I see the issue for the background, it is not shown because the path is wrong. For the accessibility issues, you need to make it in semantic HTML, In your markup, <div class="container">...</div> should be <main class="container">...</main>. Hope it fix the issues, let me know if it works! Keep it up!

  • Renaldy Prasetio•70
    @iceofice

    Hi there, It looks great! Congrats on finishing this challenge. Keep it up and let's help each other out!

  • Renaldy Prasetio•70
    @iceofice

    Hi Krishna Bansal,

    Welcome to Frontend Mentor! Well, I am also new here so let's up our skills together! I also finish this challenge in 2 days. I am not really familiar with React but here are my feedbacks,

    1. Add a default value for the timeframe, you can set 1 of them to be active so there won't be any 0 value there, {state === "" ? timeframe["weekly"]["current"] : timeframe[state]["current"]}

    2. Make it responsive! I am not really sure how to do it on React but it will be worth checking out.

    3. Remove any unnecessary console.log() that is used for debugging.

    Great job on finishing this challenge! Keep it up! I really like that animation. It makes me want to learn about animation 😅.

  • Renaldy Prasetio•70
    @iceofice

    Thanks to @BlueTampon and @fidelim! I have managed to revise my JS code.

    function assignData(timeframe) {
  var currentTimeframeElements = document.querySelectorAll(".current");
  var previousTimeframeElements = document.querySelectorAll(".previous");
  for (var i = 0; i < currentTimeframeElements.length; i++) {
    currentTimeframeElements[i].innerHTML = data[i].timeframes[timeframe].current;
    previousTimeframeElements[i].innerHTML = data[i].timeframes[timeframe].previous;
  }
}

    It works wonderfully!