-
Notifications
You must be signed in to change notification settings - Fork 2
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
Final group capstone - Book an Appointment #19
base: main
Are you sure you want to change the base?
Conversation
Set-up of Final Capstone react front end
Implement Sign in/sign up
Fixed reserve conflict
Add logout button
✅ Deploy Preview for stately-tulumba-fce256 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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.
Hi @bobb-Rob,
Good job so far!
There are some issues that you still need to work on to prepare your project for the final evaluation, but you are almost there!
Suggested Changes ♻️
Check the comments under the review.
You can use as many of my suggestions as you want. If there is anything you would like to skip - feel free to do that. However, I strongly recommend you take them into account as they can make your code better.
Cheers and Happy coding!👏👏👏
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
# Cars BnB App |
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.
- There was no open PR for your backend. I tried to review it but this happens when I try to create the database:
This means I couldn't review both the backend and the frontend functionality because there was no API running to consume. Also, I wasn't able to run the tests (because I couldn't create the db).
On the frontend, I was able to get to the initial screen, but not any further.
If this is a bug, kindly fix it. If this is not a bug and your app requires something extra not mentioned in your instructions, kindly add it.
Kindly ensure the app works (both backend and frontend) to make a proper review on the functionality.
Since there is no PR for the backend, I'll leave my comments here in this review.
# Cars BnB App |
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.
- Your backend API does not have documentation:
You could use rswag. It's simple to use, and it adds tests and documentation at the same time. 💪
# Cars BnB App |
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.
- Your backend project does not have a readme file or a license. Kindly go here and follow this template. 👍
# Cars BnB App |
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.
- On your backend, you are using Rspec, but the "test" folder still exists which is used by another test framework. Kindly delete that folder if not using it.
# Cars BnB App | ||
|
||
## Welcome! 👋 |
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.
- FRONTEND: I could get to the first screen and it's very beautiful. Let's show off your front-end app. Kindly add a screenshot to your readme so everybody can view it. 💪
## 📝 License | ||
|
||
All rights reserved. |
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.
- On the "License" section of your readme, there is no link, and there is no license in your frontend project.
Tips and extra help
Go [here] (https://github.com/microverseinc/readme-template), grab the MIT.md
file from that repo, paste it on the root of your project, and link to it in the "License" section of your readme.
That way, your app will be well documented 💪
import './styles/App.css'; | ||
import './styles/Cars.css'; | ||
|
||
function App() { |
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.
- For many components and functions, you are using the keyword
function
. Kindly refactor those functions and components to ES6 arrow functions.
Explanation
The function
keyword has been soft deprecated in almost all the software industry in favor of ES6 arrow functions.
You can make a search in Vscode for the "function" word, and correct as needed. Shouldn't take you more than 5-10 min so don't worry.
That way, your app will comply with the latest standards 💪
// import { Route, Routes } from 'react-router-dom'; | ||
import Navbar from './navbar/Navbar'; | ||
import Cars from './cars/Cars'; | ||
import './main.css'; | ||
|
||
function Main() { | ||
return ( | ||
<div className="main"> | ||
<Navbar /> | ||
|
||
{/* <Routes> | ||
<Route exact path="/" element={<Cars />} /> | ||
</Routes> */} |
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.
- Your project has several files which still have some old commented code. Kindly remove all the code that was commented by you.
That way, the cleanliness of the repo will be improved 💪
README.md
Outdated
Install all project dependencies by running the command below | ||
|
||
``` | ||
` $ npm install` | ||
``` | ||
### Usage | ||
|
||
Run | ||
``` | ||
`$ npm start or deploy the index.html from the build/public folder |
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.
- Your code needs a specific port to run the frontend. This is because your frontend code requires the server to run specifically on port :3001:
Please, be sure to add that in your instructions: Rails needs to run on port 3001 for the frontend to work well.
You can instruct your users on your readme instructions to run the rails server like this:
rails server -p 3001
That way, your users won't get confused and will be able to run your app every time 💪
Add tests
Fixed the delete car and reservation issue.
We implemented the following:
Basics
We followed the layout of the provided design.
Features
The user logs in to the website, only by typing the username (a proper authenticated login is a requirement if your group is made of 5 people).
In the navigation panel, the user can see links to:
Cars.
"Reserve" form.
"My reservations".
"Add car" (the link is visible to everybody).
"Delete car" (the link is visible to everybody).
On the main page, the user can see list of cars
When the user selects a specific item, they can see the details page with its full description
In the details page, the user can click the "Reserve" button
When the user clicks the "Add item" link in the navigation panel they can see a form for adding a new item.
Make the app responsive, creating both mobile and desktop versions.