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

Improve guides [ci skip] #19106

Merged
merged 2 commits into from
Mar 2, 2015
Merged

Improve guides [ci skip] #19106

merged 2 commits into from
Mar 2, 2015

Conversation

teeceepee
Copy link
Contributor

The code has been chaged from :'data-confirm' to :data, modify the documentation.

@eileencodes
Copy link
Member

Thanks for the pull request @teeceepee! 😄 I don't know if this is the best change though. The option is method: not method: :delete. If I was a brand new Rails developer reading this I'd think the only counterpart to method: is :delete, you know what I mean?

I do think an example could be good here though, but it should be below the main explanation. Can you update your pull request? Thanks!

@teeceepee
Copy link
Contributor Author

@eileencodes Just change :'data-confirm' to :data, OK?

@eileencodes
Copy link
Member

@teeceepee ah I see. I'm sorry I didn't look at the surrounding documentation. Your change does make sense in that context so no need to do the changes I asked for.

But while we're editing this paragraph can you change the last 2 sentences to say:

This is done via the JavaScript file jquery_ujs which is automatically included in your application's layout (app/views/layouts/application.html.erb) when you generated the application. Without this file, the confirmation dialog box won't appear.

It just changed "into" to "in" and "wouldn't" to "won't" for a better sounding sentence. Thank you!

@teeceepee
Copy link
Contributor Author

@eileencodes I have changed the guide as what you said.

eileencodes added a commit that referenced this pull request Mar 2, 2015
@eileencodes eileencodes merged commit f08c3f7 into rails:master Mar 2, 2015
@eileencodes
Copy link
Member

Thanks @teeceepee!

@teeceepee
Copy link
Contributor Author

@eileencodes You collaborators are so fast!

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.

None yet

2 participants