-
Notifications
You must be signed in to change notification settings - Fork 151
[WDFT OCT21] MIGUEL ANGEL ABAD #77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Well done! |
|
Thanks @RaselJam but it's far from being well done!! I spent most of the exercise time reading the documentation about react-router-dom v6... 😢 It's not even styled ... poor result, shame on me. |
| </h2> | ||
| </main> | ||
| <nav> | ||
| <Link to="/phones">List of Phones</Link> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggesttion : As you are using a util to store your path in utils/path. i think it will be a good idea to use it here too, insetad of hardcoded to="/phones"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was using but I hardcoded it when I was testing the new react-router-dom. I haven't time to use it properly in the HomePage component. Time flied yesterday... Thanks anyway for your feedback! It's always welcome! 😄
we have 6 more days to make it perfect. Rome has not been build in 3h . |
I'm doing it as a personal project in the next 6 days too, I had in mind to style it with tailwind and make all the bonus points until the deadline. This is the start of our future as professional developers, so let's make it excellent! 👍 |
Completed without styles, phone details displayed and spinner.
More than 1h stucked with react-router-dom version change from @5 to @6 and the Routing.