Skip to content
OverviewSolutions
16
Comments
18
Ruben
@RubenSmn

All comments

  • Ruben•550
    @RubenSmn

    I see that you made a custom line instead of using the provided svg. That looks really good!

    I did find some things you could improve upon:

    • The icon in the button is very small.

    • A transition effect on the shadow to create a more fluid effect.

    • Increase the usage of semantic elements, currently you're building a lot with div elements. These are a great way of getting to look semantic elements the way you want.

    For example your div with the container class could be a main and you could switch the span and p to a heading tag (h1, h2).

    I hope you find this useful!

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi Nico, I just looked at your solution for the Advice generator app and the desktop version looks really good. But when I switch to mobile the width of the div is not responsive.

    I see in your code you're using Tailwind CSS and have a specific desk break point. You could add a default width of something like 80% to the container div.

    <div
  className="w-[80%] bg-c-dark-grayish-blue relative m-5 p-5 rounded-xl flex flex-col justify-center items-center gap-5 desk:w-[33%] desk:gap-8 desk:px-10"
>
  ...
</div>

    I also used Tailwind CSS for this. If you want reference something take a look at my solution

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi Leon, I just looked at your solution for this challenge. Nice job with the theme using the users preferred color scheme! Overall great UI.

    I did find some things you could improve upon.

    • When trying to remove the whole word from the search input I was unable to do so. When arriving at the last letter the word just popped back in place. Probably something to do with your state.

    • A nice feature might be to let the user know that the play button has been clicked, maybe even change the icon.

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi Mooonika, I just looked at your code for this challenge and you did a nice job on separating the CSS and JavaScript from the HTML. As you project expands this becomes more important so starting early is a good practice!

    I did found some minor issue with the body which does not start at the top of the page. There is a empty space due to the margin of the main element. This makes the page scrollable which is not a big issue but you could fix this by instead of top margin on the main add it as top padding on the body.

    Hope you find this helpful and happy hacking!

  • Ruben•550
    @RubenSmn

    Hi Jakub, nice work on this Notification page challenge. I looked at your code and found some things you could improve upon.

    • A good practice is the separation of concerns, currently you have all your code in the same file. You can move the JavaScript and CSS to their own file which makes your code more readable. Once your project becomes bigger it is easier to search for something.

    • A nice feature you can add here and practice JavaScript even more is to try and add functionality to click on individual unread notifications to make them read.

    Hope you find this useful, happy hacking!

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi Mike, I just had a look at your solution for this challenge which looks really good visually.

    I have some suggestion I want to share with you.

    • The useEffect in your index.tsx has no dependency array. Which means it will run every render. You can fix this by including the theme in the array.

    • In your QuerySection you're passing some unused state dictData.

    • I would suggest to move the header to a new component where you can handle the theme and font this leaves the QuerySection with just the code for the actual query section.

    • Instead of sending all the theme state (theme and setTheme) down to the other components. Create a toggleTheme function which you can pass down.

    Hope you find this useful and happy hacking!

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi RoboXGamer, nice job on this new challenge!

    To align your element in the center you could use flexbox.

    Here is a code snippet you can use, the min-height makes sure we set the body to the whole vertical screen height. And then we apply the flexbox styles. justify-content centers the items horizontally and align-items centers them vertically.

    body {
  min-height: 100vh;
  display: flex;
  justify-content: center;
  align-items: center;
}

    Hope this is helpful and happy hacking!

  • Ruben•550
    @RubenSmn

    Hi Marcos, great UI and the great use of Framer Motion. I myself am trying to learn it because I think it is very powerful and can make great animation as you showed.

    I really like that the url reflects the state of the input, very handy and good feature for the UX.

    I did found some things that I think you could improve upon.

    • you can play the button before you searched anything, this seems a bit odd.
    • when searching for the word bubble the user is not able to view all the information down to the bottom
  • Ruben•550
    @RubenSmn

    Hi Aravind, overall great work with this multi step form challenge. I did find some minor details that you could improve upon.

    • When selecting a yearly plan instead of a monthly the prices don't update to the yearly price.

    • When you hover over the addons it would be nice to show a cursor: pointer to more indicate that these are clickable

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi Diyar, nice work on this multi step form. I really like the feedback you get when selecting a plan and addon, the blue looks good.

    I did find some minor things you could improve on with the UI/UX

    • focus state on the input labels
    • interaction on the Next Step and Go Back button
  • Ruben•550
    @RubenSmn

    Hi Ayush, great job on the challenge. I found a few things you could improve upon.

    • the button has no real interaction state so it feels a little plain.

    You could improve this by adding a hover effect as well as a cursor: pointer.

    • the component does not very well to mobile.

    You can build this by using media queries. A recommended approach is to first build the mobile version and then make it responsive to adjust to desktop sizes.

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi Annang, great work on this challenge. I had a look at the code and found some things you could improve on.

    Instead of a Api component a more recommended/best practice in React is by using a custom hook.

    // api
const useApi = (url) => {
  const [data, setData] = useState(null);
  const [loading, setLoading] = useState(false);
  const [error, setError] = useState(null);

  useEffect(() => {
    setLoading(true);
    axios
      .get(url)
      .then((response) => {
        setData(response.data);
      })
      .catch((err) => {
        setError(err);
      })
      .finally(() => {
        setLoading(false);
      });
  }, [url]);

  return { data, loading, error };
};

// app
const App = () => {
  const { data, loading, error } = useApi("...");
};

    You're currently reloading the whole page when clicking on the button. This is not really the React way of doing things.

    A solution for could be to introduce a new return prop from the api hook, a re-fetch function. Here we move the fetch functionality from the useEffect into its own function and call the function in the useEffect. When returning we can return a new prop called refetch with our new function.

    const useApi = (url) => {
  const [data, setData] = useState(null);
  const [loading, setLoading] = useState(false);
  const [error, setError] = useState(null);

  function fetchData() {
    setLoading(true);
    axios
      .get(url)
      .then((response) => {
        setData(response.data);
      })
      .catch((err) => {
        setError(err);
      })
      .finally(() => {
        setLoading(false);
      });
  }

  useEffect(() => {
    fetchData();
  }, [url]);

  return { data, loading, error, refetch: fetchData };
};

    The usage of this in the app is as simple as switching the newDatas function for the refetch one.

    const { data, loading, error, refetch } = useApi(
  "https://api.adviceslip.com/advice"
);

<button onClick={refetch} className="">
  ...
</button>;
    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi Luca, great work on this multi step form! I was impressed by the animation on the check icon! I did find some things that could be improved.

    UI/UX improvements:

    • changing the plan to yearly does not auto adjust the height of the cards
    • switch button does not have a pointer as a cursor
    • the Go Back and Next Step buttons do not have a hover effect

    State/JS improvements:

    • there is no plan pre-selected
    • when selecting a yearly plan the add-on prices don't update
    • when you select a plan and deselect it on the Finish screen there is no plan only the (Monthly) text. But the total price still has the previously selected plan price.

    I hope you find this helpful, happy hacking!

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi Obialor, I was looking at the code for the border-radius as you mentioned.

    This goes from top left to top right to bottom right and finally to bottom left. Some more detailed information can be found on MDN docs.

    You can implement this by using border-radius: 10px 0 0 10px for the image and border-radius: 0 10px 10px 0 for the message. If you do it this way its recommended to not place the border-radius on both elements at once since they need to have different values.

    Another way you could solve this is to wrap both the img and .message in the .container and apply styling to the container. This way you don't have to apply the border-radius to the individual elements.

    .container {
  border-radius: 10px;
  overflow: hidden; /* This makes sure the image has also rounded corners */
}

    Of course this approach will need some additional styling to adopt the new layout.

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi Frank, just played around with your calculator.

    When calculating something like 5+6 which result in 11 after that I wanted to immediately calculate 4*2 but instead of that the input did no clear and I got 114*2, not sure if this was something intended but I would like to mention it.

    From that great work!

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi, just played around with your calculator.

    I did however find that you'll have to exactly click on the theme button in order to change the theme. Maybe it is a nice feature to also be able to click the number of the theme you want to change to.

    When calculating something like 5+6 which results in 11 after that I wanted to immediately calculate 4*2 but instead of that the input did not clear and I got 114*2, not sure if this was something intended but I would like to mention it.

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi, I did some playing around and found that you're able to pass in a negative value in the input. When clicking Add to cart the cart gets a -amount label.

    When I tried to add a extra item after one was already in the cart the count did not increase. Instead it changed to the newly added amount.

    • added 2 > cart: 2
    • added 1 > cart: 1

    Once there are multiple items in the cart the delete functionality does not seem to work that well.

    Marked as helpful
  • Ruben•550
    @RubenSmn

    Hi Wagner, looks really great and the hover effect works perfect! I had some issues with the hovering as well. I solved this using a extra container div the add the color there.

    You could even improve this component further by adding some color for the day indication (3 days left) and the credit (Creation of) currently the text is a little hard to read.

    Marked as helpful