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

Submitted

Responsive advice generator app

#react#sass/scss#bootstrap
Naveen Ongoleβ€’ 80

@Naveen39O

Desktop design screenshot for the Advice generator app coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
  • API
2junior
View challenge

Design comparison


SolutionDesign

Solution retrospective


This is the first time I have used an API request in the challenge. I have used React to build the app and I used useState hook to store the data. Is this ok? or Should I call the API request from useEffect hook?

Community feedback

@elidrissidev

Posted

Hey Naveen πŸ‘‹,

Great work on your solution, keep on going!

I have some feedback for you, first is an accessibility one:

  • The dice button doesn't have any descriptive text inside. For accessibility reasons, buttons and links must always have some text inside that describes their action, in the case where it's not a part of the design, consider making the text visually hidden.
  • The dice image is mainly for decoration, and when you add a proper title ☝️, there will be no need for accessibility software to announce the alt text, in this case, you can make it empty so that it's not announced.

This one is coding-style related:

  • Try to keep things consistent in your code so other people find it easier to read. For example, you have some random spaces in your JSX between the attribute and value. In general, try to follow the common conventions, in this case, no spaces between attribute and its value in JSX.

This one is related to React:

  • Know when and what to separate something to its own component. For example, you have created an Advice component, but it only contains an h1 and the rest including the button is in the App component. What I would do in this case is extract the whole card to the Advice component.
  • I would also recommend separating data fetching logic to a custom hook. For example, you can create a hook called useAdvice and inside of it you would put the state and the fetching function, you would then return those to be consumed by the component. e.g:
function useAdvice() {
  const [advice, setAdvice] = useState(null);
  const [isLoading, setIsLoading] = useState(false);
  const [error, setError] = useState(null);

  const fetchAdvice = () => {
    setIsLoading(true);
    axios.get("https://api.adviceslip.com/advice")
      .then((res) => 
        {
          setAdvice(res.data.slip); 
          setIsLoading(false);
          setError(null);
        })
      .catch((error) => 
        {
          console.log(error);
          setError(error);
          setIsLoading(false);
        });
  }

  return { advice, isLoading, error, fetchAdvice }
}
  • I would remove the hard-coded advice that's shown initially on page load, instead you can replace it by a useEffect with an empty dependency array that will fire when the component mounts and fetch the initial advice from the API.

Phew, that was a lot πŸ˜…. Hope it helps, Good luck!

Marked as helpful

1

Naveen Ongoleβ€’ 80

@Naveen39O

Posted

@elidrissidev Thanks for the detailed review and ideas for improvement.

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord