Skip to content
OverviewSolutions
16
Comments
18
Ruben
@RubenSmn

All comments

  • Ruben•550
    @RubenSmnPosted almost 2 years ago

    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
    0
  • Ruben•550
    @RubenSmnPosted almost 2 years ago

    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
    1
  • Ruben•550
    @RubenSmnPosted almost 2 years ago

    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
    0
  • Ruben•550
    @RubenSmnPosted almost 2 years ago

    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!

    0
  • Ruben•550
    @RubenSmnPosted almost 2 years ago

    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
    1
  • Mike•40
    @Mike-is-codingSubmitted almost 2 years ago

    I had a lot of difficulty using typescript in this project, but I got more comfortable using it towards the end as I started to understand the errors and my mistakes. Tailwind was also very difficult to use for me, it seemed very counterproductive. Overall, I think I learned a lot from this project and I'm really glad I completed it. Please leave any comments on anything I can improve on thank you.

    Dictionary Web App using Next.js, Tailwind, and Typescript

    1
    Ruben•550
    @RubenSmnPosted almost 2 years ago

    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
    1
  • Ruben•550
    @RubenSmnPosted almost 2 years ago

    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!

    0
  • Ruben•550
    @RubenSmnPosted almost 2 years ago

    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
    0
  • Ruben•550
    @RubenSmnPosted almost 2 years ago

    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
    0
  • Diyar•40
    @Diyar2222Submitted almost 2 years ago

    Hi there, the form validation and also mobile responsiveness parts were somehow difficult. I spent on them several hours. Overall, the project is very interesting. Any feedbacks and suggestions will be appreciated. Thanks.

    Multi-step form (React, Redux, Sass)

    1
    Ruben•550
    @RubenSmnPosted almost 2 years ago

    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
    0
  • Ruben•550
    @RubenSmnPosted almost 2 years ago

    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
    0
  • Ruben•550
    @RubenSmnPosted almost 2 years ago

    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
    0