@GugaS1lva
Alex McGonagle
@thevolcanomanishereAll comments
- @thevolcanomanishere
Hi Gustavo,
Great work. The design is on-point. For your Javascript, I have some suggestions: https://github.com/GugaS1lva/Fr.Mentor-11--Advice_Generator-App/blob/main/pages/index.js
You should create a single function that wraps the fetch call, so that you are not writing out the same function multiple times. This is called DRY (dont repeat yourself). Maybe a "getAdvice" function. On line, 45, you can then reuse this function instead of writing it out again. It is also good practice not to write out new functions inside of your component. You should always write your functions above your components. This helps to organise things a little easier.
You don't need 2 different useStates. You can use a single useState like const [data, setData] = useState("Loading..."); You then have access to the id and advice either by creating a simple selector or de-structuring the object. You can put data.slip into advice, then you can access with data.id and data.advice. :)
- @Aysha-py@thevolcanomanishere
Great work so far :) You just need to correctly import the font and to use object-fit: contain so that your images don't resize within their container. https://developer.mozilla.org/en-US/docs/Web/CSS/object-fit
- @arlagonix@thevolcanomanishere
I love this. The transparent card, hover effects, and wave effect look amazing.
- @nirajbagdi@thevolcanomanishere
Quick one, change your main div to <main> to fix your accessibility issues
- @AYoNiEz@thevolcanomanishere
You can quickly fix your 'landmark' issue by setting the main <div> that surrounds your project to <main>
Marked as helpful - @denneb@thevolcanomanishere
You did a really great job on this project. I had a quick look at your folder structure and nothing jumped out as wrong! You have a well defined folder structure with things in their logical place.
For the background on your nav -> Add blur + opacity and it will match the reference design much closer.
Marked as helpful - @riccardofano@thevolcanomanishere
Try out Tailwind :) You can write your utility classes directly on each component!
Marked as helpful - @wyliemickelson@thevolcanomanishere
Looks good WYLIEMICKELSON’S! I would suggest looking at React + Tailwind to make your life much easier. React means that you can write HTML like code in your Javascript so you can avoid all those 'document.getBlahBlah'. Much less to keep track off. Tailwind lets you write CSS helpers directly on the components that you create and greatly simplifies styling.
- @MikiW03@thevolcanomanishere
Looks good! I noticed you haven't yet implemented the mobile view yet. Your current desktop design just needs the body font changed to grey, and maybe reduce the font weight a little too. Then your design will be far more similar to the reference.
Marked as helpful - @ChiamakaUI@thevolcanomanishere
Great work! You are nearly there. Change the weight of the font + the color to grey and your design will be much closer to the reference. Also, change the border color of the textarea to grey.
- @Ebunski@thevolcanomanishere
Looks great! You just need to add a border radius to the 'you' with the blue background. Also your font has very tight kerning. Check it out here -> https://developer.mozilla.org/en-US/docs/Web/CSS/font-kerning
Check out tailwind for CSS. You will move 5x quicker and can write your css on the components directly
Marked as helpful
