-
Notifications
You must be signed in to change notification settings - Fork 2k
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
pjax form submit heroku example #489
base: heroku
Are you sure you want to change the base?
Conversation
Thanks for the submission. What were you trying to do here? |
Sorry, I added description to the first comment. I just want to see |
Gotcha. Well, your pull request is based on a commit that's very old: from year 2012 it seems. Then you upgraded the library files in the commit "Update jquery and jquery.pjax.js". That wasn't the proper way to upgrade these files. A better way would be to base your changes on a newer commit, preferrably last commit on our master branch. A way to update this pull request would be:
|
Well, Anyway I tried your instruction... git fetch origin master pick 88b7074 first draft
pick aa45e93 hook it up
pick 636006b home n whatnot
pick bf9c0f0 sweet
pick c2460f0 better explanation
pick 8044633 make errything work
pick 00a7f5a finishing touch
pick 914b638 favicon
pick e1e376e cleaner, better
pick 4e5454d readme
pick 1bf695f view
pick f00abaf no pjax for you :(
pick 23cb50b load
pick e2c656e update from master
pick 6ea1d31 update pjax
pick c2a7265 update pjax... again!
pick c7f80c5 iOS web apps <3 pjax
pick 64ff5ce update pjax
# pick 7b152cf Update jquery and jquery.pjax.js.
pick a0d7c26 pjax form submit.
# Rebase 56b950b..a0d7c26 onto 56b950b
#
...
Hmm, |
Oh I'm sorry, I provided you with wrong instructions. I thought that you opened a PR for our "master" branch, like most of our PRs. But you made a perfectly valid pull request for our "heroku" branch which was indeed slightly out of date because it only serves as a demo app. Ignore my above instructions. |
<script src="jquery.cookie.js"></script> | ||
<script src="pages.js"></script> | ||
<!-- <script src="jquery.js"></script> --> | ||
<script src="//code.jquery.com/jquery-1.11.2.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 from loading from CDN. You may remove the old vendored jquery.js
and references to it.
@mislav Thanks for your review. |
…p seems to work well for later jquery versions.
@mislav I updated most of codes where you pointed. Also updated https://pjax-form.herokuapp.com/. |
if ( !$(':checkbox').attr('checked') ) | ||
$.fn.pjax = $.noop | ||
if ( !$(':checkbox').prop('checked') ) | ||
$.fn.pjax = $.pjax.submit = $.noop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we instead do $.pjax.enable/disable()
based on the state of checkbox?
https://pjax-form.herokuapp.com/
I just add a form to the heroku example branch. I think it help someone want to try pjax form submit.
Current status is let it just work. To be merged there still some work needed.
$.pjax.submit
.jquery.cookie.js
contained in the branch does not work with new jquery. replace with new one, if available, or use an alternative, or ..... -> sessionStorage!