From 264c8ff7af09828422ba211a7b6433b28135281d Mon Sep 17 00:00:00 2001 From: rodrigo stevaux Date: Wed, 7 Dec 2016 10:52:01 -0200 Subject: [PATCH] Add comments --- app/actions/action-types.js | 5 +++++ app/actions/index.js | 10 ++++++++++ app/app.js | 1 + app/components/new-todo-input.js | 8 ++++++++ app/components/todos-list.js | 8 ++++++++ app/initialize.js | 3 +++ app/todos.js | 4 ++++ 7 files changed, 39 insertions(+) diff --git a/app/actions/action-types.js b/app/actions/action-types.js index 159439d..8f22a29 100644 --- a/app/actions/action-types.js +++ b/app/actions/action-types.js @@ -1,3 +1,8 @@ +// Use "todos/CHECK_TODO", +// it is just good practice. +// When we have more than a single reducer +// working, we can quickly tell what reducer +// an action belongs to (and avoid conflicts). export const CHECK_TODO = "CHECK_TODO" export const ADD_TODO = "ADD_TODO" export const REMOVE_TODO = "REMOVE_TODO" diff --git a/app/actions/index.js b/app/actions/index.js index 79b259a..d088801 100644 --- a/app/actions/index.js +++ b/app/actions/index.js @@ -1,8 +1,18 @@ +// I think the index file should just +// import stuff and export stuff. +// Move this to an actions.js file. + +// import * as t, and you can +// access the members as t.CHECK_TODO +// for example. This way you can +// protect the global namespace and keep it clean. import { CHECK_TODO, ADD_TODO, REMOVE_TODO } from './action-types'; + +// This should come as the first statement in file. "use strict"; export const checkTodo = (todo) => { diff --git a/app/app.js b/app/app.js index 1dde888..bc5e70c 100644 --- a/app/app.js +++ b/app/app.js @@ -2,6 +2,7 @@ import React from 'react'; import TodoList from 'components/todos-list'; import NewTodoInput from 'components/new-todo-input'; +// Very clean code! const App = () => (
diff --git a/app/components/new-todo-input.js b/app/components/new-todo-input.js index d3cb0b0..74e9356 100644 --- a/app/components/new-todo-input.js +++ b/app/components/new-todo-input.js @@ -3,6 +3,14 @@ import {bindActionCreators} from 'redux'; import {connect} from 'react-redux'; import { addTodo} from '../actions/index'; +// The way to avoid using document.getElementById and acessing the DOM +// directly is to add an event handler to and use a local state. +// Something like: +// this.setState({newTodoText: event.target.value})} +// You can also make it go through Redux here, +// but local state is not always evil, +// specially in this case +// where you are just storing the value from a form field. class NewTodoInput extends Component{ keyPressHandler(e){ diff --git a/app/components/todos-list.js b/app/components/todos-list.js index 0573fa8..1f3d90b 100644 --- a/app/components/todos-list.js +++ b/app/components/todos-list.js @@ -3,6 +3,14 @@ import {bindActionCreators} from 'redux'; import {connect} from 'react-redux'; import {delTodo, checkTodo} from '../actions/index'; +// Extract the code inside map to a TodoListItem component. +// Pass each todo as a prop to the Item component. +// This will also simplify your code and you'll be able +// to put all render code inside render(). +// It will be simpler. Instead of having +// a method return all items, +// iterate on render(). + class TodoList extends Component{ createListItems(){ diff --git a/app/initialize.js b/app/initialize.js index f708fbd..f314492 100644 --- a/app/initialize.js +++ b/app/initialize.js @@ -3,6 +3,9 @@ import Redux from 'redux'; import ReactDOM from 'react-dom'; import {start} from './todos' +// Put the code from todos.js here. +// No need to have two files doing pretty much +// the same. document.addEventListener('DOMContentLoaded', () => { start(); }); diff --git a/app/todos.js b/app/todos.js index 15a3fe9..7a4eedc 100644 --- a/app/todos.js +++ b/app/todos.js @@ -5,9 +5,13 @@ import {Provider} from 'react-redux'; import Reducer from 'reducers/reducer' import App from 'app'; +// Move this to initialize, that makes +// more sense don't you think? export function start () { const store = createStore(Reducer); +// Reformat, it looks weird with the +// comma on its own big line. Use ESLint. ReactDOM.render(