Skip to content
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

First do it #60

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,6 @@ npm-debug.log

# Ignore bundle dependencies
vendor/ruby

# RVM gemset
.ruby-gemset
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious if this should be project specific.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I keep all gems in isolated gemsets for two reasons:

  1. I maintain yo generator, so I scaffold a lot for testing purposes. It's easier for me to delete scaffolded test gems by deleting gemset.
  2. To sleep easily.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should .ruby-gemset be in developer's global gitignore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should .ruby-gemset be in developer's global gitignore?

If everybody is using gemset-per-app strategy, then no. Otherwise — yes.

38 changes: 19 additions & 19 deletions client/assets/javascripts/actions/CommentActionCreators.js
Original file line number Diff line number Diff line change
@@ -1,61 +1,61 @@
import CommentsManager from '../utils/CommentsManager';
import * from '../constants/ActionTypes';
import * as actionTypes from '../constants/ActionTypes';

export function incrementAjaxCounter() {
return {
type: INCREMENT_AJAX_COUNTER
}
type: actionTypes.INCREMENT_AJAX_COUNTER,
};
}

export function decrementAjaxCounter() {
return {
type: DECREMENT_AJAX_COUNTER
}
type: actionTypes.DECREMENT_AJAX_COUNTER,
};
}

export function fetchCommentsSuccess(comments) {
return {
type: FETCH_COMMENTS_SUCCESS,
comments
type: actionTypes.FETCH_COMMENTS_SUCCESS,
comments,
};
}

export function fetchCommentsFailure(error) {
return {
type: FETCH_COMMENTS_FAILURE,
error
type: actionTypes.FETCH_COMMENTS_FAILURE,
error,
};
}

export function submitCommentSuccess(comment) {
return {
type: SUBMIT_COMMENT_SUCCESS,
comment
type: actionTypes.SUBMIT_COMMENT_SUCCESS,
comment,
};
}

export function submitCommentFailure(error) {
return {
type: SUBMIT_COMMENT_FAILURE,
error
}
type: actionTypes.SUBMIT_COMMENT_FAILURE,
error,
};
}

export function fetchComments() {
return function(dispatch) {
return dispatch => {
return CommentsManager.fetchComments().then(
comments => dispatch(fetchCommentsSuccess(comments)),
error => dispatch(fetchCommentsFailure(error))
);
}
};
}

export function submitComment(comment) {
return function(dispatch) {
return dispatch => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

dispatch(incrementAjaxCounter());
return CommentsManager.submitComment(comment).then(
comment => dispatch(submitCommentSuccess(comment),
commentFromServer => dispatch(submitCommentSuccess(commentFromServer)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

error => dispatch(submitCommentFailure(error))
).then(() => dispatch(decrementAjaxCounter()));
}
};
}
4 changes: 2 additions & 2 deletions client/assets/javascripts/components/Comment.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ const Comment = React.createClass({
render() {
const rawMarkup = marked(this.props.text);
return (
<div className='comment'>
<h2 className='comment-author'>
<div className="comment">
<h2 className="comment-author">
{this.props.author}
</h2>
<span dangerouslySetInnerHTML={{__html: rawMarkup}}/>
Expand Down
2 changes: 1 addition & 1 deletion client/assets/javascripts/components/CommentBox.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const CommentBox = React.createClass({

propTypes: {
pollInterval: React.PropTypes.number.isRequired,
actions: PropTypes.object.isRequired,
actions: React.PropTypes.object.isRequired,
},

componentDidMount() {
Expand Down
2 changes: 1 addition & 1 deletion client/assets/javascripts/components/CommentForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const CommentForm = React.createClass({

propTypes: {
ajaxSending: React.PropTypes.bool.isRequired,
actions: PropTypes.object.isRequired,
actions: React.PropTypes.object.isRequired,
},

getInitialState() {
Expand Down
4 changes: 2 additions & 2 deletions client/assets/javascripts/components/CommentList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ const CommentList = React.createClass({

render() {
const reversedData = this.props.comments.slice(0).reverse();
const commentNodes = reversedData.map((comment, index) => {
const commentNodes = this.props.comments.get('comments').map((comment, index) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to sort out the reversed data. The reducer is putting the new one at that the top. I'm not sure if we should have the store in reverse order, and not reverse for display or vice versa.

// `key` is a React-specific concept and is not mandatory for the
// purpose of this tutorial. if you're curious, see more here:
// http://facebook.github.io/react/docs/multiple-components.html#dynamic-children
return (
<Comment author={comment.author} key={index} text={comment.text}/>
<Comment author={comment.get('author')} key={index} text={comment.get('text')}/>
);
});

Expand Down
4 changes: 2 additions & 2 deletions client/assets/javascripts/components/CommentScreen.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import { fetchComments, submitComment } from '../actions/CommentActionCreators';

const CommentActionsCreators = [fetchComments, submitComment];
const CommentActionsCreators = { fetchComments, submitComment };

function select(state) {
// Which part of the Redux global state does our component want to receive as props?
Expand All @@ -20,7 +20,7 @@ const CommentScreen = React.createClass({

render() {
const { dispatch, ...other } = this.props;
const actions = bindActionCreators(CommentActionCreators, dispatch);
const actions = bindActionCreators(CommentActionsCreators, dispatch);
return (
<div>
<CommentBox pollInterval={5000} {...other}
Expand Down
42 changes: 21 additions & 21 deletions client/assets/javascripts/reducers/commentReducer.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
import * from '../constants/ActionTypes';
import * as actionTypes from '../constants/ActionTypes';

import Immutable from 'immutable';
import { Map } from 'immutable';

const initialState = Immutable.Map({
const initialState = Map({
comments: [],
ajaxCounter: 0,
fetchCommentError: '',
submitCommentError: '',
});

export default function commentReducer(state = initialState, action) {
export default function comments(state = initialState, action) {
switch (action.type) {
case FETCH_COMMENTS_SUCCESS:
return state.merge({ comments: action.comments, fetchCommentError: '' });
case FETCH_COMMENTS_FAILURE:
return state.merge({ fetchCommentError: action.error });
case SUBMIT_COMMENT_SUCCESS:
return state.withMutatations(mState => {
mState.updateIn(['comments'], comments => comments.unshift(action.comment)).
merge({ submitCommentError: '' });
});
case SUBMIT_COMMENT_FAILURE:
return state.merge({ submitCommentError: action.error });
case INCREMENT_AJAX_COUNTER:
return state.merge({ ajaxCounter: state.get('ajaxCounter') + 1 });
case DECREMENT_AJAX_COUNTER:
return state.merge({ ajaxCounter: state.get('ajaxCounter') - 1 });
default:
return state;
case actionTypes.FETCH_COMMENTS_SUCCESS:
return state.merge({ comments: action.comments, fetchCommentError: '' });
case actionTypes.FETCH_COMMENTS_FAILURE:
return state.merge({ fetchCommentError: action.error });
case actionTypes.SUBMIT_COMMENT_SUCCESS:
return state.withMutatations(mState => {
mState.updateIn(['comments'], comments => comments.unshift(action.comment)). // Seems to me this is a mutation, for `comments` we should use List type instead
merge({ submitCommentError: '' });
});
case actionTypes.SUBMIT_COMMENT_FAILURE:
return state.merge({ submitCommentError: action.error });
case actionTypes.INCREMENT_AJAX_COUNTER:
return state.merge({ ajaxCounter: state.get('ajaxCounter') + 1 });
case actionTypes.DECREMENT_AJAX_COUNTER:
return state.merge({ ajaxCounter: state.get('ajaxCounter') - 1 });
default:
return state;
}
}
3 changes: 3 additions & 0 deletions client/assets/javascripts/reducers/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import comments from './commentReducer';

export default { comments };
11 changes: 7 additions & 4 deletions client/assets/javascripts/stores/CommentStore.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { createStore, applyMiddleware } from 'redux';
import commentReducer from '../reducers/commentReducer';
import { createStore, applyMiddleware, combineReducers } from 'redux';
import thunk from 'redux-thunk';
import reducers from '../reducers';

// Smth wrong with this one, off for now
import loggerMiddleware from '../middleware/loggerMiddleware';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes -- let's see if we can get this one working afterwards, maybe a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will take a look into this one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.


// applyMiddleware supercharges createStore with middleware:
const createStoreWithMiddleware = applyMiddleware(thunk, loggerMiddleware)(createStore);
export default createStoreWithMiddleware(commentReducer);
const createStoreWithMiddleware = applyMiddleware(thunk)(createStore);
export default createStoreWithMiddleware(combineReducers(reducers));
10 changes: 4 additions & 6 deletions client/gulpfile.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
const gulp = require('gulp');
const eslint = require('gulp-eslint');
const eslint = require('eslint/lib/cli');

// Note: To have the process exit with an error code (1) on
// lint error, return the stream and pipe to failOnError last.
gulp.task('lint', function gulpLint() {
return gulp.src(['assets/javascripts/**/*.jsx', 'scripts/**/*.jsx', '*.js'])
.pipe(eslint())
.pipe(eslint.format())
.pipe(eslint.failOnError());
gulp.task('lint', function gulpLint(done) {
eslint.execute('--ext .js,.jsx .');
return done();
});

gulp.task('default', ['lint'], function gulpDefault() {
Expand Down
16 changes: 12 additions & 4 deletions client/server.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,26 @@
/*eslint-disable no-console, func-names, no-var */
/* eslint-disable no-console, func-names, no-var */
var bodyParser = require('body-parser');
var webpack = require('webpack');
var WebpackDevServer = require('webpack-dev-server');
var config = require('./webpack.hot.config');
var sleep = require('sleep');

var comments = [{author: 'Pete Hunt', text: 'Hey there!'},
{author: 'Justin Gordon', text: 'Aloha from @railsonmaui'},];
var comments = [
{author: 'Pete Hunt', text: 'Hey there!'},
{author: 'Justin Gordon', text: 'Aloha from @railsonmaui'},
];

var server = new WebpackDevServer(webpack(config), {
publicPath: config.output.publicPath,
hot: true,
noInfo: false,
stats: {colors: true},
stats: {
colors: true,
hash: false,
version: false,
chunks: false,
children: false,
},
});

server.app.use(bodyParser.json(null));
Expand Down
6 changes: 4 additions & 2 deletions client/webpack.common.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ module.exports = {
// jquery: 'var jQuery'
// },
resolve: {
root: [path.join(__dirname, 'scripts'),
path.join(__dirname, 'assets/javascripts'),],
root: [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, best to put white space changes in separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

path.join(__dirname, 'scripts'),
path.join(__dirname, 'assets/javascripts'),
],
extensions: ['', '.webpack.js', '.web.js', '.js', '.jsx', '.scss', '.css', 'config.js'],
},
module: {
Expand Down