edit some components to make it more generic, add eslint configeraions, and react-router-dom #23
Conversation
and take value of controlled components : Select, Input, Radio .
client/src/Router.js
Outdated
| const Router = () => ( | ||
| <BrowserRouter> | ||
| <Switch> | ||
| <Route exact path="/" render={() => <Login />} /> |
There was a problem hiding this comment.
What do you think about using the "/" path for the customer homepage, and putting all of the admin-related pages under "/admin/"? So then admin login is at the path /admin/, AddFlight is /admin/addflight, and so on.
I imagine that most of the visitors to the site would be customers, so this might make the site easier to use (and remember) for them.
There was a problem hiding this comment.
Done. We were worried about getting react-router-dom working that we didn't think about his 😄
client/src/Router.js
Outdated
| const Router = () => ( | ||
| <BrowserRouter> | ||
| <Switch> | ||
| <Route exact path="/" render={() => <Login />} /> |
There was a problem hiding this comment.
Having the login route use a render function prop is good, because that's where you can check whether they already have a logged-in token, and can redirect them right into /flights or whereever they should go 😀
| <button className={className} onClick={onClick}> | ||
| {children} | ||
| </button> | ||
| ); |
|
|
||
| class Select extends Component { | ||
| selectValue = e => { |
There was a problem hiding this comment.
You might need to add the class properties babel plugin for this class property syntax to work (to get transformed properly in the build step).
It's really easy - you can take a look here: https://www.npmjs.com/package/babel-plugin-transform-class-properties
It's just an npm package, and then one line added to your .babelrc
We use class properties a lot at my work! We love them.
There was a problem hiding this comment.
I was trying to find something to get rid of that annoying red alarm. thanks for pointing this out :)
| <a href="#">Update flight status</a> | ||
| </li>) | ||
| } | ||
| {thirdTap && ( |
There was a problem hiding this comment.
We have a third tap on the side navigation bar(Update flight status) which appears only when you click on a flight from the flights' table. I'll change the name to be more readable.
There was a problem hiding this comment.
I think this is very good so far, just solve the comments @tomduggan85 had pointed out and everything will be merged tomorrow.
@amusameh
related to issue #21