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

Controlled input cursor jumps when used with layers #1698

Closed
sophiebits opened this Issue Jun 16, 2014 · 5 comments

Comments

Projects
None yet
5 participants
@sophiebits
Member

sophiebits commented Jun 16, 2014

See http://jsfiddle.net/Bobris/ZZtXn/2/ (try typing at the beginning of the text box).

@sophiebits sophiebits added the bug label Jun 16, 2014

@syranide

This comment has been minimized.

Show comment
Hide comment
@syranide

syranide Jun 17, 2014

Contributor

Am I right in thinking that React.renderComponent being async (updating the DOM) is the problem?

Contributor

syranide commented Jun 17, 2014

Am I right in thinking that React.renderComponent being async (updating the DOM) is the problem?

zpao added a commit to zpao/react that referenced this issue Jul 21, 2014

Add ReactUpdates.setImmediate for async callbacks
Callbacks passed to this setImmediate function are called at the end of the current update cycle, which is guaranteed to be asynchronous but in the same event loop (with the default batching strategy).

This is useful for new-style refs (#1373, #1554) and for fixing #1698.

Test Plan: jest

zpao added a commit to zpao/react that referenced this issue Jul 21, 2014

Use setImmediate to defer value restoration
Depends on #1758.

Fixes #1698.

Previously, controlled components would update too soon when using something like ReactLayeredComponentMixin (i.e., before the layer's updates could propagate from the parent), causing the cursor to jump even when always updating the new model value to match the DOM state. With this change, we defer the update until after all nested updates have had a chance to finish, which prevents the cursor from misbehaving.

Also cleaned up the logic around updating a bit -- the .value and .checked updates in ReactDOMInput weren't being relied on at all so I removed them and opted for a simple forceUpdate instead. I also got rid of _isChanging which hasn't been necessary since the introduction of update batching.

Test Plan: Tested the example in http://jsfiddle.net/Bobris/ZZtXn/2/ and verified that the cursor didn't jump. Changed the code to filter out numbers and verified that the field prevents typing numbers (attempting to do so still causes the cursor to jump to the end). Also verified that controlled and uncontrolled radio buttons, textareas, and select boxes work.

sophiebits added a commit to sophiebits/react that referenced this issue Jul 22, 2014

Add ReactUpdates.setImmediate for async callbacks
Callbacks passed to this setImmediate function are called at the end of the current update cycle, which is guaranteed to be asynchronous but in the same event loop (with the default batching strategy).

This is useful for new-style refs (#1373, #1554) and for fixing #1698.

Test Plan: jest

sophiebits added a commit to sophiebits/react that referenced this issue Jul 22, 2014

Use setImmediate to defer value restoration
Depends on #1758.

Fixes #1698.

Previously, controlled components would update too soon when using something like ReactLayeredComponentMixin (i.e., before the layer's updates could propagate from the parent), causing the cursor to jump even when always updating the new model value to match the DOM state. With this change, we defer the update until after all nested updates have had a chance to finish, which prevents the cursor from misbehaving.

Also cleaned up the logic around updating a bit -- the .value and .checked updates in ReactDOMInput weren't being relied on at all so I removed them and opted for a simple forceUpdate instead. I also got rid of _isChanging which hasn't been necessary since the introduction of update batching.

Test Plan: Tested the example in http://jsfiddle.net/Bobris/ZZtXn/2/ and verified that the cursor didn't jump. Changed the code to filter out numbers and verified that the field prevents typing numbers (attempting to do so still causes the cursor to jump to the end). Also verified that controlled and uncontrolled radio buttons, textareas, and select boxes work.

slorber referenced this issue in moreartyjs/moreartyjs Jul 28, 2014

@zpao zpao closed this in #1759 Jul 30, 2014

zpao added a commit that referenced this issue Jul 30, 2014

Merge pull request #1759 from spicyj/gh-1698
Use setImmediate to defer value restoration
@ericclemmons

This comment has been minimized.

Show comment
Hide comment
@ericclemmons

ericclemmons Nov 7, 2014

Contributor

Is there a resolution for this within the client codebase? Looking @spicyj's fiddle (and having the exact same issue on v0.11.x), I still can't figure out the solution...

Contributor

ericclemmons commented Nov 7, 2014

Is there a resolution for this within the client codebase? Looking @spicyj's fiddle (and having the exact same issue on v0.11.x), I still can't figure out the solution...

anarchistkasan pushed a commit to anarchistkasan/react.backbone that referenced this issue Jan 20, 2015

@damianmr

This comment has been minimized.

Show comment
Hide comment
@damianmr

damianmr Feb 12, 2015

Same question here. Any workaround?

damianmr commented Feb 12, 2015

Same question here. Any workaround?

@zigomir

This comment has been minimized.

Show comment
Hide comment
@zigomir

zigomir Feb 12, 2015

@damianmr I also thought I had a problem with it, but later discovered it was my own fault. Are maybe using value={foo} where defaultValue={foo} might do?

zigomir commented Feb 12, 2015

@damianmr I also thought I had a problem with it, but later discovered it was my own fault. Are maybe using value={foo} where defaultValue={foo} might do?

@damianmr

This comment has been minimized.

Show comment
Hide comment
@damianmr

damianmr Feb 12, 2015

Nope, React shows a warning in such cases.

damianmr commented Feb 12, 2015

Nope, React shows a warning in such cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment