Skip to content
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 skipped keys in controlled components on IE #127

Merged
merged 1 commit into from
Jul 20, 2016
Merged

Fixed skipped keys in controlled components on IE #127

merged 1 commit into from
Jul 20, 2016

Conversation

tonsky
Copy link
Contributor

@tonsky tonsky commented Jul 19, 2016

This PR fixes a problem when fast typing results in some keys gone missing in IE (even relatively contemporary IE 11)

tonsky/rum#86
reagent-project/reagent#253

What happens is on IE onChange event is fired out of order, with a significant delay (not sure if it’s a React trying to “normalize” IE behaviour by polling it with a timeout or an actual IE problem). That causes problems with re-render happening on requestAnimationFrame.

Imagine a component:

(def *value (atom ""))

(rum/defc comp < rum/reactive []
  [:input {:type "text"
           :value (rum/react *value)
           :on-change (fn [e] (reset! *value (.. e -target -value)))}])

Typical order of events, if I type "QW" into the input:

  1. Keydown on Q
  2. Keydown on W
  3. Onchange fires with input.value = Q
  4. on-change callback is called, *value got reset to Q
  5. Re-rendering is scheduled
  6. In normal browsers, this is when second on-change should fire, setting *value to QW and causing no problems. Not on IE
  7. Animation frame happens and comp is re-rendered. Notice that the *value still holds Q, but actual value of input in DOM is QW. Second on-change has not fired yet, but value has already changed without us noticing it.
  8. React tries to render input and notices that we want to render <input value=Q> while on DOM it’s <input value=QW>. Diffing kicks in, React resets value back to Q.
  9. Because of us tampering with DOM, second on-change never fires, cursor position got messed up, etc.

This patch changes wrapped components, adds detecting of this situation (onChange and actual DOM value disagreeing) and works around that.

@r0man r0man merged commit 579f3d7 into r0man:master Jul 20, 2016
@r0man
Copy link
Owner

r0man commented Jul 20, 2016

@tonsky Merged and pushed a snapshot to Clojars. Thanks!

@tonsky
Copy link
Contributor Author

tonsky commented Jul 20, 2016

Any change to see 0.7.3 (0.8?) some time soon?

@r0man
Copy link
Owner

r0man commented Jul 20, 2016

@tonsky Yes, just released 0.7.3.

@tonsky
Copy link
Contributor Author

tonsky commented Jul 20, 2016

🤘

On Wed, Jul 20, 2016 at 4:26 PM r0man notifications@github.com wrote:

@tonsky https://github.com/tonsky Yes, just released 0.7.3.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#127 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AARabGSpKIEasDSAco6CXspSzAB4gtgyks5qXffWgaJpZM4JQLDd
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants