-
Notifications
You must be signed in to change notification settings - Fork 2k
Send submitting elements value along with post #390
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -117,16 +117,22 @@ function handleClick(event, container, options) { | |
function handleSubmit(event, container, options) { | ||
options = optionsFor(container, options) | ||
|
||
var form = event.currentTarget | ||
var form = event.currentTarget, | ||
$form = $(form) | ||
|
||
if (form.tagName.toUpperCase() !== 'FORM') | ||
throw "$.pjax.submit requires a form element" | ||
|
||
var data = $form.serializeArray() | ||
|
||
if (submitButton = $form.data(submitButtonAttr)) | ||
data.push(submitButton) | ||
|
||
var defaults = { | ||
type: form.method.toUpperCase(), | ||
url: form.action, | ||
data: $(form).serializeArray(), | ||
container: $(form).attr('data-pjax'), | ||
data: data, | ||
container: $form.attr('data-pjax'), | ||
target: form | ||
} | ||
|
||
|
@@ -135,6 +141,28 @@ function handleSubmit(event, container, options) { | |
event.preventDefault() | ||
} | ||
|
||
var submitClickSelectors = 'form[data-pjax] input[type=submit],\ | ||
form[data-pjax] button[type=submit],\ | ||
form[data-pjax] button:not([type])', | ||
submitButtonAttr = 'pjax-submit-button-value' | ||
|
||
// Private: sends value of submitting element along form post | ||
// | ||
// event - "click" jQuery.Event | ||
// | ||
// Returns nothing. | ||
function handleSubmitClick(event) { | ||
var $submit = $(this), | ||
$form = $submit.closest('form') | ||
|
||
if (name = $submit.attr('name')) { | ||
$form.data(submitButtonAttr, { name: name, value: $submit.val() }) | ||
} | ||
else { | ||
$form.data(submitButtonAttr, null) | ||
} | ||
} | ||
|
||
// Loads a URL with ajax, puts the response body inside a container, | ||
// then pushState()'s the loaded URL. | ||
// | ||
|
@@ -798,7 +826,10 @@ function enable() { | |
maxCacheLength: 20, | ||
version: findVersion | ||
} | ||
|
||
$(window).on('popstate.pjax', onPjaxPopstate) | ||
|
||
$(document).on('click', submitClickSelectors, handleSubmitClick) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't you better use a submit event.. Click events are sent a lot more often and this could impact performance There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @staabm My understanding was that the submit event can only be bound to a form. "It can only be attached to elements." - http://api.jquery.com/submit/Click events will always result in a submission, unless the default function of the button has been overridden. Perhaps there is call for an ignored class, that can be applied to buttons you intend to override? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me say it in other words... Can't you just extend the submit handling which pjax already does instead of adding another listener? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @staabm if you can suggest a better approach I'd genuinely be happy to see it. Started down the same thought process as you initially. However, there is no cross browser solution (that I have found) that allows you to locate the submitting element within the submit event. My understanding is that only Gecko and Trident provide this data. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The submitted button value is only required when pjaxing a form, right? Noone would expect it when using pjax on a link, wouldn't they? So, adjust You also won't need this hidden field and instead pass the value alongside other data using jquery's ajax() with the data option... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now all the submit stuff is completely opt in. So I don't think any "submit button" click handlers should be initial installed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @josh The click handler is only added to buttons within There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a suggestion, we could add a configuration setting for This would have the advantage of allowing the user to prevent the binding by passing I'm not suggesting that this has any implication for the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Right; I keep forgetting what's core behavior and what's not. The default unobtrusiveness of pjax makes this feature tricky to implement. |
||
} | ||
|
||
// Disable pushState behavior. | ||
|
@@ -822,6 +853,8 @@ function disable() { | |
$.pjax.reload = function() { window.location.reload() } | ||
|
||
$(window).off('popstate.pjax', onPjaxPopstate) | ||
|
||
$(document).off('click', submitClickSelectors, handleSubmitClick) | ||
} | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
if ($.support.pjax) { | ||
module("handleSubmit", { | ||
setup: function() { | ||
var self = this | ||
stop() | ||
window.iframeLoad = function(frame) { | ||
self.frame = frame | ||
window.iframeLoad = $.noop | ||
start() | ||
} | ||
$("#qunit-fixture").append("<iframe src='basic_form.html'>") | ||
}, | ||
teardown: function() { | ||
delete window.iframeLoad | ||
} | ||
}) | ||
|
||
asyncTest("Form POST records submitting element's value", function() { | ||
var frame = this.frame | ||
|
||
frame.$("#main") | ||
.on("pjax:end", function() { | ||
equal(frame.$("#submit").text(), "submit value") | ||
start() | ||
}) | ||
|
||
frame.$("[name='submit']").click() | ||
}) | ||
|
||
module("handleSubmit (multiple submit buttons)", { | ||
setup: function() { | ||
var self = this | ||
stop() | ||
window.iframeLoad = function(frame) { | ||
self.frame = frame | ||
window.iframeLoad = $.noop | ||
start() | ||
} | ||
$("#qunit-fixture").append("<iframe src='form.html'>") | ||
}, | ||
teardown: function() { | ||
delete window.iframeLoad | ||
} | ||
}) | ||
|
||
asyncTest("Form POST records submitting element's value", function() { | ||
var frame = this.frame | ||
|
||
frame.$("#main") | ||
.on("pjax:end", function() { | ||
equal(frame.$("#submit").text(), "submit value") | ||
start() | ||
}) | ||
|
||
frame.$("input[name='submit']").click() | ||
}) | ||
|
||
asyncTest("Form POST records submitting element's value, when input is a button", function() { | ||
var frame = this.frame | ||
|
||
frame.$("#main") | ||
.on("pjax:end", function() { | ||
equal(frame.$("#submit").text(), "submit value") | ||
start() | ||
}) | ||
|
||
frame.$("button[name='submit']").click() | ||
}) | ||
|
||
module("handleSubmit (multiple forms)", { | ||
setup: function() { | ||
var self = this | ||
stop() | ||
window.iframeLoad = function(frame) { | ||
self.frame = frame | ||
window.iframeLoad = $.noop | ||
start() | ||
} | ||
$("#qunit-fixture").append("<iframe src='multiple_forms.html'>") | ||
}, | ||
teardown: function() { | ||
delete window.iframeLoad | ||
} | ||
}) | ||
|
||
asyncTest("Form POST records submitting element's value", function() { | ||
var frame = this.frame | ||
|
||
frame.$("#main") | ||
.on("pjax:end", function() { | ||
equal(frame.$("#submit").text(), "submit value") | ||
start() | ||
}) | ||
|
||
frame.$("#form2 [name='submit']").click() | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<%= title 'Form' %> | ||
|
||
<form action="/form_post.html" method="post" data-pjax> | ||
<input name="test" type="text" /> | ||
<input name="test1" type="text" /> | ||
<input name="submit" type="submit" value="submit value" /> | ||
</form> | ||
|
||
<script type="text/javascript"> | ||
$("#main") | ||
.pjax("a") | ||
.on('submit', 'form[data-pjax]', function(evt) { | ||
$.pjax | ||
.submit(evt, $("#main")); | ||
}) | ||
|
||
window.parent.iframeLoad(window) | ||
</script> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<%= title 'Form' %> | ||
|
||
<form action="/form_post.html" method="post" data-pjax> | ||
<input name="test" type="text" /> | ||
<input name="test1" type="text" /> | ||
<input name="submit3" type="submit" value="submit value 3" /> | ||
<input name="submit" type="submit" value="submit value" /> | ||
<input name="submit2" type="submit" value="submit value 2" /> | ||
<button name="submit" type="submit" value="submit value">Submit Button</button> | ||
</form> | ||
|
||
<script type="text/javascript"> | ||
$("#main") | ||
.pjax("a") | ||
.on('submit', 'form[data-pjax]', function(evt) { | ||
$.pjax | ||
.submit(evt, $("#main")); | ||
}) | ||
|
||
window.parent.iframeLoad(window) | ||
</script> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
<p id="submit"><%= params[:submit] %></p> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
<%= title 'Form' %> | ||
|
||
<form action="/form_post.html" method="post" data-pjax> | ||
<input name="test" type="text" /> | ||
<input name="test1" type="text" /> | ||
<input name="submit3" type="submit" value="submit value 3" /> | ||
<input name="submit" type="submit" value="submit value form 1" /> | ||
<input name="submit2" type="submit" value="submit value 2" /> | ||
</form> | ||
|
||
<form id="form2" action="/form_post.html" method="post" data-pjax> | ||
<input name="test" type="text" /> | ||
<input name="test1" type="text" /> | ||
<input name="submit3" type="submit" value="submit value 3" /> | ||
<input name="submit" type="submit" value="submit value" /> | ||
<input name="submit2" type="submit" value="submit value 2" /> | ||
</form> | ||
|
||
<script type="text/javascript"> | ||
$("#main") | ||
.pjax("a") | ||
.on('submit', 'form[data-pjax]', function(evt) { | ||
$.pjax | ||
.submit(evt, $("#main")); | ||
}) | ||
|
||
window.parent.iframeLoad(window) | ||
</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.
Not sure this selector works.. Do all pjax forms necessarily have a
data-pjax
attribute?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.
The Readme infers as much (https://github.com/defunkt/jquery-pjax#pjaxsubmit), but there is nothing cas iron. I think it is should be 'opt-in' functionality though. Do you agree?
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.
The samples in the code do not require it:
https://github.com/defunkt/jquery-pjax/blob/master/jquery.pjax.js#L111
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.
I think you need to capture the current submit element globally, no matter if a pjax-form is handled atm or not. I see no other way.
Any other ideas @mislav @josh ?
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.
I'm happy to do this. I just think that the form functionality as a whole should be opt-in.
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.
Or possibly opt-out?
form:not(:pjax-ignore)
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.
If there are no perf implications I would enable it by default, because then pjax-forms would behave like usual forms, which is what devs would expect in the first place IMO