Discuss how to render/organize templates rendered in JavaScript

[rememberlenny]

1. If there is a form that is processing the submit, then make remote: true

   ie. https://github.com/18F/C2/blob/design/new-details/app/views/proposals/details/_observer.html.haml#L13

2. Then set up a js response on the controller and create a .js.erb that responds according to the form_for endpoint

   ie. js response: https://github.com/18F/C2/blob/design/new-details/app/controllers/observations_controller.rb#L11-L18

   ie. .js.erb file: https://github.com/18F/C2/blob/design/new-details/app/views/proposals/js/observations.js.erb

3. Use the .js.erb file to render the content/template that needs to be rendered in the page. Try to use as much of the existing JS files functions. Keep as much logic out of these .js.erb files and try not to pollute the global namespace with variables.

   ie. https://github.com/18F/C2/blob/design/new-details/app/views/proposals/js/observations.js.erb#L9

[jessieay]

@rememberlenny this overall strategy seems fine / standard to me.

For me, it is easier to evaluate a strategy when I see it implemented. I noticed that the linked branch has ~250 commits and 22k changes.

Is there a reason we are putting so much work on one branch without doing PRs? Is that branch just for a demo we are going to throw away and rebuild later? If so, totally cool to continue down the path of putting it all on there without review. If not, I think we need to discuss the process moving forward.

tl;dr I'd love to review some code that implements the above strategy, ideally in the form of a small PR that I can review / QA in under 30 mins.

[rememberlenny]

@jessieay This point about PR review is really great.

Right now, the plan was to build a beta feature on production situations/DB. We were iterating quickly on the actual code design/implementation, so the small PR process didn't make sense.

We are still doing some major changes on the design end, which are not all "final". The next phase is to build out tests in the existing code.

What are your thoughts on the best way to properly have reviewable chunks at this point?

[jessieay]

Ahhhh ok so if we do want this merged in (aka - not thrown away), the branch as-is is def too large to review.

Like you said, we need to find a way to break it into smaller chunks. It's OK if we PR those small chunks against a non-master branch if it doesn't make sense to merge the smaller pieces into master (like with EMAIL redesign).

I am not very familiar with what exactly this branch contains so I don't have any suggestions for how to break it up off the top of my head.

As a rule of thumb, I try to keep PRs under 500 changes (additions + deletions) unless it is a large stylistic change that results in a noisy diff but is not complicated to read (eg: changing 1000 single quotes to double or something like that).

Let me know if you'd like to talk through some ideas on how to break it up. Happy to talk through it together!