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

Input loses focus when initial value is nil #140

Closed
roman01la opened this issue Dec 29, 2016 · 7 comments
Closed

Input loses focus when initial value is nil #140

roman01la opened this issue Dec 29, 2016 · 7 comments

Comments

@roman01la
Copy link

I noticed that an input field loses focus on very first character typed in, when the initial value is nil. It works fine with an empty string.

Here's a minimal reproducible example:

(ns app.core
  (:require [sablono.core :as html :refer-macros [html]]))

(def value (atom nil))

(defn root [value]
  (html
   [:input {:value @value
            :on-change #(reset! value (.. % -target -value))}]))

(defn render [cmp]
  (js/ReactDOM.render cmp (js/document.getElementById "app")))

(render (root value))

(add-watch value :input (fn [_ _ _ n] (render (root value))))
@r0man
Copy link
Owner

r0man commented Dec 29, 2016

@roman01la Your input element changes from being a "uncontrolled" input (the value nil, which sablono maps to js/undefined) to being a "controlled" input ( a value different than nil). I think this is not supported by React. You should use either a controlled, or an uncontrolled input, but not switch.

Switching between them means for React you are using 2 different components. That's why you loose focus. You could use either the blank string in your atom or call str on value when building your input element.

I don't know of a better solution at the moment.

@roman01la
Copy link
Author

@r0man That's interesting, because ClojureScript compiler actually translates nil into null: https://github.com/clojure/clojurescript/blob/95fd110f55c57b890422763ed8f2644cfbf159de/src/main/clojure/cljs/compiler.cljc#L215
But in React when null is supplied you get a warning:

"Warning: value prop on input should not be null. Consider using the empty string to clear the component or undefined for uncontrolled components."

I agree that it's probably better to follow React requirements. But why does input lose focus? Does sablono render different components when switching between uncontrolled and controlled?

@r0man
Copy link
Owner

r0man commented Dec 29, 2016

@roman01la The idea was since nil isn't allowed anyways (you get a warning), use nil for uncontrolled inputs and anything else for controlled inputs. Not sure if that was a good idea. Regarding the focus, this is because React treats the change from uncontrolled to controlled as 2 different components. At least that is my understanding.

@roman01la
Copy link
Author

@r0man Interesting.. I just checked, in JavaScript the change from uncontrolled to controlled doesn't make input to loose focus http://jsbin.com/lurajaheko/edit?js,console,output

@r0man
Copy link
Owner

r0man commented Dec 29, 2016

@roman01la Hmm, interesting. I guess then we have an issue with this beast again :(
https://github.com/r0man/sablono/blob/master/src/sablono/interpreter.cljc#L15

@roman01la
Copy link
Author

roman01la commented Jan 8, 2017

@r0man I had some time to dig into interpreter and I found the issue. It is actually renders two different components, because of defined? check here https://github.com/r0man/sablono/blob/master/src/sablono/interpreter.cljc#L75

Should defined? check for both nil and undefined or just undefined? I think it should be only undefined because React allows null, though it throws a warning.

@r0man
Copy link
Owner

r0man commented Jan 11, 2017

Fixed in master

@r0man r0man closed this as completed Jan 11, 2017
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

No branches or pull requests

2 participants