Preserve input values in cache #666
Conversation
seanpdoyle
force-pushed
the
preserve-input-values
branch
from
9d0cd2d to
5ae9df1
Compare
Re-submission of hotwired#70 This change will save the values of `<input>`s, `<textarea>`s, and `<select>`s when creating a snapshot so that when you navigate back or forward in history, form fields will not be reset. This behavior was already present prior to [5dfc79e][] for `<input>`s and `<textarea>`s but not `<select>`s (see [turbolinks/turbolinks#238][]). [5dfc79e]: hotwired@5dfc79e [turbolinks/turbolinks#238]: turbolinks/turbolinks#238
seanpdoyle
force-pushed
the
preserve-input-values
branch
from
5ae9df1 to
712c268
Compare
|
Thank you for the review @marcoroth. I've re-structured the |
|
Beautiful, that's even better! |
| for (const [optionIndex, option] of Array.from(source.options).entries()) { | ||
| clonedSelectElements[index].options[optionIndex].selected = option.selected | ||
| } | ||
| } |
There was a problem hiding this comment.
FYI, the following approach is around 3x to 4x faster. Not an issue for small selects, but if the page contains large selects (e.g. Country Selector) it adds up.
for (const [index, source] of selectElements.entries()) {
[...clonedSelectElements[index].selectedOptions].forEach(option => { option.selected = false });
[...source.selectedOptions].forEach(option => clonedSelectElements[index].options[option.index].selected = true)
}There was a problem hiding this comment.
Would it be even faster if the [...] syntax were replaced with a for ... of loop? I can open a follow-up PR.
There was a problem hiding this comment.
I've opened #669. Thank you!
Follow-up to hotwired#666 Following [a suggestion][] provided by review of hotwired#666, this commit remove unnecessary loops when cloning a page's form controls to be cached by a snapshot. [a suggestion]: hotwired#666 (comment)
Follow-up to #666 Following [a suggestion][] provided by review of #666, this commit remove unnecessary loops when cloning a page's form controls to be cached by a snapshot. [a suggestion]: #666 (comment)
|
👋 Thanks for this PR and all your hard work! Big fan of the library :) I've been doing some research here. I'm familiarizing myself with the cloning process and am curious why certain inputs receive special treatment. As you can see in the linked PR, I'm observing unexpected (to me at least) behavior in radio buttons (and check boxes) on restoration visits. Curious if anything sticks out to you since you're intimately familiar with this bit of the code :) Also, curious if you find the linked behavior surprising -- or if this is something that must be handled in application code (given it's just vanilla form w/o any third-party libs, I think I expect it to work? 🤔) |
Re-submission of #70
This change will save the values of
<input>s,<textarea>s, and<select>s when creating a snapshot so that when you navigate back orforward in history, form fields will not be reset.
This behavior was already present prior to 5dfc79e for
<input>sand
<textarea>s but not<select>s (seeturbolinks/turbolinks#238).