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

Don't merge - Steven for review #33

Open
wants to merge 148 commits into
base: review-base
Choose a base branch
from
Open

Don't merge - Steven for review #33

wants to merge 148 commits into from

Conversation

stevecass
Copy link
Collaborator

No description provided.

kevalwell and others added 30 commits April 10, 2015 12:07
Conflicts:
	db/schema.rb
	new file:   app/controllers/questions_controller.rb
	new file:   app/views/questions/new.html.erb
	modified:   config/routes.rb
	modified:   app/controllers/questions_controller.rb
	modified:   app/views/questions/new.html.erb
added index method to user controller
	new file:   app/views/questions/show.html.erb
	modified:   app/controllers/questions_controller.rb
…the user of each

	modified:   app/views/questions/show.html.erb
	modified:   config/routes.rb
Hoa Nguyen & Hoa Nguyen and others added 12 commits April 12, 2015 17:34
	modified:   app/controllers/answers_controller.rb
	modified:   app/controllers/comments_controller.rb
	modified:   app/views/comments/new.html.erb
Nifty Search Bar for User Index
Conflicts:
	app/controllers/comments_controller.rb
	app/views/comments/new.html.erb
	app/views/questions/show.html.erb
Conflicts:
	app/views/comments/new.html.erb
	app/views/questions/show.html.erb
console.log(serverResponse)
$('#comments').append(serverResponse);
});
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Think this is not used? In any case, in general, it's better ux to show / hide / dynamically create forms with JS rather than get+partial (it's a good idea to avoid a network round-trip if we can.)

@stevecass
Copy link
Collaborator Author

Me likey. This is done well. We can talk in person later about some of the points raised here.

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

Successfully merging this pull request may close these issues.

4 participants