-
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
Fixed issue #518: Textarea and select value isn't preserved #542
base: master
Are you sure you want to change the base?
Conversation
…en going back/forward
Hi thanks for the contribution. This would need automated tests in order to be accepted. The tests should also verify whether regular Does this handle |
Given this PR is already a few months old, I have decided to jump in and help with the tests, see https://github.com/masone/jquery-pjax/commit/f9c06149f6058a1fd440d3e885c4d225a9e58e29. I can confirm that When testing @the-ress' changes, the one for the single I'm new to jquery-pjax, so please have a thorough look before merging my changes. |
Thank you for the tests. I never managed to set up the environment to be able to run them. The single value select was a bug in the original PR. I fixed it and merged your test branch. |
@the-ress I'm sorry you had problems running the test suite locally. You need to have Ruby with Bundler installed and run |
@@ -562,6 +562,21 @@ function uniqueId() { | |||
} | |||
|
|||
function cloneContents(container) { | |||
// Preserve textarea values | |||
var textarea = findAll(container, "textarea") | |||
textarea.text(function(i, text){return textarea[i].value}) |
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.
Isnt this missing a .each
?
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.
No, text() has similar functionality: http://api.jquery.com/text/#text-function
Ugh sorry, I messed up the git history on the temporary |
elem = $(elem); | ||
var values = $(elem).val(); | ||
values = $.isArray(values) ? values : [values]; | ||
elem.find('option[selected]').attr('selected', false); |
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 not happy that this performs DOM changes on elements before they are cloned. How about just making these changes on the elements after they have been cloned?
Fixed issue #518: Textarea and select value isn't preserved when going back/forward