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

Simple, non-redux example improvements #138

Closed
justin808 opened this issue Oct 30, 2015 · 7 comments
Closed

Simple, non-redux example improvements #138

justin808 opened this issue Oct 30, 2015 · 7 comments

Comments

@justin808
Copy link
Member

  1. This should take props so as not to depend on the first ajax request: https://github.com/shakacode/react_on_rails/blob/master/app/assets/javascripts/react_on_rails.js#L159
  2. We should not be passing empty props here:
    <%= react_component('SimpleCommentScreen', {}, generator_function: false, prerender: false) %>
  3. We need to break this up into a container component that does the Ajax call and a view component.
    See article on Container Components. This is the file to break up: https://github.com/shakacode/react-webpack-rails-tutorial/blob/master/client/app/components/SimpleCommentScreen.jsx
  4. Change /app/assets/javascripts/application.js to not have rails specific startup code. See comment here

@robwise We'll need to incorporate this into the generator.

@josiasds Would you like to take this one on?

@justin808
Copy link
Member Author

This one is up for grabs until we assign somebody!

@robwise
Copy link
Contributor

robwise commented Oct 31, 2015

@justin808 I agree with this approach in general. While making the generators, I was having to first make my own custom apps in order to be sure that what I was generating would actually be a working app. The more we can make the tutorial the model on which to base our generators, the better. Obviously, there are some things in the tutorial that should not go in the generators, as the former is showing off and the latter is KISS. But things like the containers folder, the bundles folder, components using es2015 class syntax, etc. should all agree between the two.

@josiasds josiasds self-assigned this Dec 7, 2015
@justin808
Copy link
Member Author

@josiasds and @MaMute Is this issue still valid?

@josiasds
Copy link
Member

@justin808 Yes, we should break SimpleCommentScreen into a container to handle the requests and a view component.

@justin808
Copy link
Member Author

@josiasds can we close this issue?

@josiasds
Copy link
Member

@justin808 It looks like item 3 is still valid. SimpleCommentScreen is the only component that still doesn't follow the rules in the project. We have AJAX calls and HTML rendering together, and I think we should split it into a container and a view component.

I'll try to take it this weekend.

@justin808
Copy link
Member Author

@josiasds Can we close this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants