-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Implement React [email protected] #393
Changes from 41 commits
19046a0
24d5343
441faac
5c3c6c2
d3e51e2
c75ff80
ce00c61
b761625
8fa5ca5
558727d
164b862
694ddac
3e2cfc5
5667b09
180e669
2e5ce1d
33e87de
a194252
a1b920e
c91dea4
b8fa9ad
82f1ee4
70c1d9c
578176d
aa1fb15
707e12a
a2b6bca
2e2d60d
407144e
3f56197
a5ef8d2
c6609c0
1e72aaa
3fc6175
ab0e2a3
408632f
46b7d26
27169b3
c35174f
f651fc5
057033a
bd13289
a5cf0e0
26387b8
8ad2d6e
57a54e7
e0a8524
8d68161
e7c9c3f
95e6c48
e665193
c155977
d94c0ee
fb0902d
d116756
251fba8
3efc625
975fe9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import React from 'react'; | ||
import {Link} from 'react-router'; | ||
import { Link } from 'react-router-dom'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. which formatting does our linter support? Spacing between {} in imports or none? I can't remember. cc: @kwelch. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure, I know I prefer with spaces. I also believe prettier default is with spaces. |
||
import '../styles/about-page.css'; | ||
|
||
// Since this component is simple and static, there's no parent container for it. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import React from 'react'; | ||
import {shallow} from 'enzyme'; | ||
import { shallow } from 'enzyme'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above. |
||
import AboutPage from './AboutPage'; | ||
|
||
describe('<AboutPage />', () => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,48 +32,48 @@ class FuelSavingsForm extends React.Component { | |
<h2>Fuel Savings Analysis</h2> | ||
<table> | ||
<tbody> | ||
<tr> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nickytonline & @coryhouse There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Point taken. You can leave it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @coryhouse thanks for the confirmation. |
||
<td><label htmlFor="newMpg">New Vehicle MPG</label></td> | ||
<td><FuelSavingsTextInput onChange={this.fuelSavingsKeypress} name="newMpg" value={fuelSavings.newMpg}/> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td><label htmlFor="tradeMpg">Trade-in MPG</label></td> | ||
<td><FuelSavingsTextInput onChange={this.fuelSavingsKeypress} name="tradeMpg" value={fuelSavings.tradeMpg}/> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td><label htmlFor="newPpg">New Vehicle price per gallon</label></td> | ||
<td><FuelSavingsTextInput onChange={this.fuelSavingsKeypress} name="newPpg" value={fuelSavings.newPpg}/> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td><label htmlFor="tradePpg">Trade-in price per gallon</label></td> | ||
<td><FuelSavingsTextInput onChange={this.fuelSavingsKeypress} name="tradePpg" value={fuelSavings.tradePpg}/> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td><label htmlFor="milesDriven">Miles Driven</label></td> | ||
<td> | ||
<FuelSavingsTextInput | ||
<tr> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once again, question about the spacing changes. Is this our eslint config doing this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nickytonline This one looks like its from my text-editor. I use an atom package There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TobiahRex - Would you be willing to rollback the whitespace changes in this PR to keep the diff minimal? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nickytonline & @coryhouse After checking again, i think this is a correct addition of whitespace. The |
||
<td><label htmlFor="newMpg">New Vehicle MPG</label></td> | ||
<td><FuelSavingsTextInput onChange={this.fuelSavingsKeypress} name="newMpg" value={fuelSavings.newMpg}/> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td><label htmlFor="tradeMpg">Trade-in MPG</label></td> | ||
<td><FuelSavingsTextInput onChange={this.fuelSavingsKeypress} name="tradeMpg" value={fuelSavings.tradeMpg}/> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td><label htmlFor="newPpg">New Vehicle price per gallon</label></td> | ||
<td><FuelSavingsTextInput onChange={this.fuelSavingsKeypress} name="newPpg" value={fuelSavings.newPpg}/> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td><label htmlFor="tradePpg">Trade-in price per gallon</label></td> | ||
<td><FuelSavingsTextInput onChange={this.fuelSavingsKeypress} name="tradePpg" value={fuelSavings.tradePpg}/> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td><label htmlFor="milesDriven">Miles Driven</label></td> | ||
<td> | ||
<FuelSavingsTextInput | ||
onChange={this.fuelSavingsKeypress} | ||
name="milesDriven" | ||
value={fuelSavings.milesDriven}/> | ||
miles per | ||
<select | ||
miles per | ||
<select | ||
name="milesDrivenTimeframe" | ||
onChange={this.onTimeframeChange} | ||
value={fuelSavings.milesDrivenTimeframe}> | ||
<option value="week">Week</option> | ||
<option value="month">Month</option> | ||
<option value="year">Year</option> | ||
</select> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td><label>Date Modified</label></td> | ||
<td>{fuelSavings.dateModified}</td> | ||
</tr> | ||
<option value="week">Week</option> | ||
<option value="month">Month</option> | ||
<option value="year">Year</option> | ||
</select> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td><label>Date Modified</label></td> | ||
<td>{fuelSavings.dateModified}</td> | ||
</tr> | ||
</tbody> | ||
</table> | ||
|
||
|
@@ -85,11 +85,12 @@ class FuelSavingsForm extends React.Component { | |
); | ||
} | ||
} | ||
const { func, object } = PropTypes; | ||
|
||
FuelSavingsForm.propTypes = { | ||
saveFuelSavings: PropTypes.func.isRequired, | ||
calculateFuelSavings: PropTypes.func.isRequired, | ||
fuelSavings: PropTypes.object.isRequired | ||
saveFuelSavings: func.isRequired, | ||
calculateFuelSavings: func.isRequired, | ||
fuelSavings: object.isRequired | ||
}; | ||
|
||
export default FuelSavingsForm; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,18 @@ | ||
import React, { Component } from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import { ConnectedRouter } from 'react-router-redux'; | ||
console.log('%cConnectedRouter', 'background:red;', ConnectedRouter); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like a console.log slipped in |
||
import { Provider } from 'react-redux'; | ||
import routes from '../routes'; | ||
import { Router } from 'react-router'; | ||
import App from './App'; | ||
|
||
export default class Root extends Component { | ||
render() { | ||
const { store, history } = this.props; | ||
return ( | ||
<Provider store={store}> | ||
<Router history={history} routes={routes} /> | ||
<ConnectedRouter history={history}> | ||
<App /> | ||
</ConnectedRouter> | ||
</Provider> | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,20 @@ | ||
import {createStore, compose, applyMiddleware} from 'redux'; | ||
import reduxImmutableStateInvariant from 'redux-immutable-state-invariant'; | ||
import thunk from 'redux-thunk'; | ||
import createHistory from 'history/createBrowserHistory'; | ||
// 'routerMiddleware': the new way of storing route changes with redux middlware since rrV4. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing "e" in middleware |
||
import { routerMiddleware } from 'react-router-redux'; | ||
import rootReducer from '../reducers'; | ||
|
||
export const history = createHistory(); | ||
function configureStoreProd(initialState) { | ||
const reactRouterMiddleware = routerMiddleware(history); | ||
const middlewares = [ | ||
// Add other middleware on this line... | ||
|
||
// thunk middleware can also accept an extra argument to be passed to each thunk action | ||
// https://github.com/gaearon/redux-thunk#injecting-a-custom-argument | ||
thunk, | ||
reactRouterMiddleware, | ||
]; | ||
|
||
return createStore(rootReducer, initialState, compose( | ||
|
@@ -19,6 +24,7 @@ function configureStoreProd(initialState) { | |
} | ||
|
||
function configureStoreDev(initialState) { | ||
const reactRouterMiddleware = routerMiddleware(history); | ||
const middlewares = [ | ||
// Add other middleware on this line... | ||
|
||
|
@@ -28,6 +34,7 @@ function configureStoreDev(initialState) { | |
// thunk middleware can also accept an extra argument to be passed to each thunk action | ||
// https://github.com/gaearon/redux-thunk#injecting-a-custom-argument | ||
thunk, | ||
reactRouterMiddleware, | ||
]; | ||
|
||
const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose; // add support for Redux dev tools | ||
|
@@ -49,4 +56,4 @@ function configureStoreDev(initialState) { | |
|
||
const configureStore = process.env.NODE_ENV === 'production' ? configureStoreProd : configureStoreDev; | ||
|
||
export default configureStore; | ||
export default configureStore; |
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.
Just want to ask this again before we move forward...
Are we sure we want to keep this?
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.
cc: @coryhouse
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 vote for dropping it. Easy to add if desired, and avoids an alpha dep