From 9cd8b5aea8cff904584685df9d8733088f9a5a48 Mon Sep 17 00:00:00 2001 From: Nikita Prokopov Date: Tue, 16 Aug 2016 19:14:20 +0200 Subject: [PATCH 1/2] Fixed wrapped inputs with non-string values & checkboxes --- src/sablono/interpreter.cljc | 75 ++++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 12 deletions(-) diff --git a/src/sablono/interpreter.cljc b/src/sablono/interpreter.cljc index ec80fc0..43e10bf 100644 --- a/src/sablono/interpreter.cljc +++ b/src/sablono/interpreter.cljc @@ -12,14 +12,14 @@ ;; waiting for requestAnimationFrame #?(:cljs - (defn wrap-form-element [element] + (defn wrap-value-input [element] (js/React.createClass #js {:displayName (str "wrapped-" element) :getInitialState (fn [] (this-as this - #js {"state_value" (aget (.-props this) "value")})) + #js {"state_value" (str (aget (.-props this) "value"))})) :onChange (fn [e] (this-as this @@ -31,8 +31,7 @@ (fn [new-props] (this-as this (let [state-value (aget (.-state this) "state_value") - element (js/ReactDOM.findDOMNode this) - element-value (.-value element)] + element-value (.-value (js/ReactDOM.findDOMNode this))] ;; on IE, onChange event might come after actual value of an element ;; have changed. We detect this and render element as-is, hoping that ;; next onChange will eventually come and bring our modifications anyways. @@ -41,7 +40,7 @@ ;; https://github.com/tonsky/rum/issues/86 (if (not= state-value element-value) (.setState this #js {"state_value" element-value}) - (.setState this #js {"state_value" (aget new-props "value")}))))) + (.setState this #js {"state_value" (str (aget new-props "value"))}))))) :render (fn [] (this-as this @@ -50,28 +49,80 @@ (gobject/extend element-props (.-props this) - #js {:value (or (aget (.-state this) "state_value") js/undefined) + #js {:value (aget (.-state this) "state_value") :onChange (aget this "onChange") :children (aget (.-props this) "children")}) (js/React.createElement element element-props))))}))) -#?(:cljs (def wrapped-input (wrap-form-element "input"))) -#?(:cljs (def wrapped-select (wrap-form-element "select"))) -#?(:cljs (def wrapped-textarea (wrap-form-element "textarea"))) +#?(:cljs + (defn wrap-checked-input [element] + (js/React.createClass + #js + {:displayName (str "wrapped-" element) + :getInitialState + (fn [] + (this-as this + #js {"state_checked" (boolean (aget (.-props this) "checked"))})) + :onChange + (fn [e] + (this-as this + (let [handler (aget (.-props this) "onChange")] + (when-not (nil? handler) + (handler e) + (.setState this #js {"state_value" (.. e -target -checked)}))))) + :componentWillReceiveProps + (fn [new-props] + (this-as this + (let [state-checked (aget (.-state this) "state_checked") + element-checked (.-checked (js/ReactDOM.findDOMNode this))] + ;; on IE, onChange event might come after actual value of an element + ;; have changed. We detect this and render element as-is, hoping that + ;; next onChange will eventually come and bring our modifications anyways. + ;; Ignoring this causes skipped letters in controlled components + ;; https://github.com/reagent-project/reagent/issues/253 + ;; https://github.com/tonsky/rum/issues/86 + (if (not= state-checked element-checked) + (.setState this #js {"state_checked" element-checked}) + (.setState this #js {"state_checked" (boolean (aget new-props "checked"))}))))) + :render + (fn [] + (this-as this + ;; NOTE: if switch to macro we remove a closure allocation + (let [element-props #js {}] + (gobject/extend + element-props + (.-props this) + #js {:checked (aget (.-state this) "state_checked") + :onChange (aget this "onChange") + :children (aget (.-props this) "children")}) + (js/React.createElement element element-props))))}))) + +#?(:cljs (def wrapped-input (wrap-value-input "input"))) +#?(:cljs (def wrapped-checked (wrap-checked-input "input"))) +#?(:cljs (def wrapped-select (wrap-value-input "select"))) +#?(:cljs (def wrapped-textarea (wrap-value-input "textarea"))) #?(:cljs (defn create-element [type props & children] (let [class (case (keyword type) :input - (if (and props (or (exists? (.-checked props)) - (exists? (.-value props)))) - wrapped-input "input") + (cond (or (nil? props) + (undefined? props)) + "input" + (or (= "radio" (.-type props)) + (= "checkbox" (.-type props))) + (if (exists? (.-checked props)) wrapped-checked "input") + (exists? (.-value props)) + wrapped-input + :else + "input") :select (if (and props (exists? (.-value props))) wrapped-select "select") :textarea (if (and props (exists? (.-value props))) wrapped-textarea "textarea") + ;; else (name type)) children (remove nil? children)] (if (empty? children) From 09a443a2e52d19640726add358f861684c92e01a Mon Sep 17 00:00:00 2001 From: Nikita Prokopov Date: Wed, 17 Aug 2016 15:22:45 +0200 Subject: [PATCH 2/2] Fixed tests, fixed existence of value/checked check --- src/sablono/interpreter.cljc | 105 +++++++++++------------------------ 1 file changed, 32 insertions(+), 73 deletions(-) diff --git a/src/sablono/interpreter.cljc b/src/sablono/interpreter.cljc index 43e10bf..a930bb4 100644 --- a/src/sablono/interpreter.cljc +++ b/src/sablono/interpreter.cljc @@ -12,26 +12,26 @@ ;; waiting for requestAnimationFrame #?(:cljs - (defn wrap-value-input [element] + (defn wrap-form-element [element property coerce] (js/React.createClass #js {:displayName (str "wrapped-" element) :getInitialState (fn [] (this-as this - #js {"state_value" (str (aget (.-props this) "value"))})) + #js {"state_value" (coerce (aget (.-props this) property))})) :onChange (fn [e] (this-as this (let [handler (aget (.-props this) "onChange")] (when-not (nil? handler) (handler e) - (.setState this #js {"state_value" (.. e -target -value)}))))) + (.setState this #js {"state_value" (aget (.-target e) property)}))))) :componentWillReceiveProps (fn [new-props] (this-as this (let [state-value (aget (.-state this) "state_value") - element-value (.-value (js/ReactDOM.findDOMNode this))] + element-value (aget (js/ReactDOM.findDOMNode this) property)] ;; on IE, onChange event might come after actual value of an element ;; have changed. We detect this and render element as-is, hoping that ;; next onChange will eventually come and bring our modifications anyways. @@ -40,7 +40,7 @@ ;; https://github.com/tonsky/rum/issues/86 (if (not= state-value element-value) (.setState this #js {"state_value" element-value}) - (.setState this #js {"state_value" (str (aget new-props "value"))}))))) + (.setState this #js {"state_value" (coerce (aget new-props property))}))))) :render (fn [] (this-as this @@ -49,82 +49,41 @@ (gobject/extend element-props (.-props this) - #js {:value (aget (.-state this) "state_value") - :onChange (aget this "onChange") + #js {:onChange (aget this "onChange") :children (aget (.-props this) "children")}) + (aset element-props property (aget (.-state this) "state_value")) (js/React.createElement element element-props))))}))) -#?(:cljs - (defn wrap-checked-input [element] - (js/React.createClass - #js - {:displayName (str "wrapped-" element) - :getInitialState - (fn [] - (this-as this - #js {"state_checked" (boolean (aget (.-props this) "checked"))})) - :onChange - (fn [e] - (this-as this - (let [handler (aget (.-props this) "onChange")] - (when-not (nil? handler) - (handler e) - (.setState this #js {"state_value" (.. e -target -checked)}))))) - :componentWillReceiveProps - (fn [new-props] - (this-as this - (let [state-checked (aget (.-state this) "state_checked") - element-checked (.-checked (js/ReactDOM.findDOMNode this))] - ;; on IE, onChange event might come after actual value of an element - ;; have changed. We detect this and render element as-is, hoping that - ;; next onChange will eventually come and bring our modifications anyways. - ;; Ignoring this causes skipped letters in controlled components - ;; https://github.com/reagent-project/reagent/issues/253 - ;; https://github.com/tonsky/rum/issues/86 - (if (not= state-checked element-checked) - (.setState this #js {"state_checked" element-checked}) - (.setState this #js {"state_checked" (boolean (aget new-props "checked"))}))))) - :render - (fn [] - (this-as this - ;; NOTE: if switch to macro we remove a closure allocation - (let [element-props #js {}] - (gobject/extend - element-props - (.-props this) - #js {:checked (aget (.-state this) "state_checked") - :onChange (aget this "onChange") - :children (aget (.-props this) "children")}) - (js/React.createElement element element-props))))}))) +#?(:cljs (def wrapped-input (wrap-form-element "input" "value" str))) +#?(:cljs (def wrapped-checked (wrap-form-element "input" "checked" boolean))) +#?(:cljs (def wrapped-select (wrap-form-element "select" "value" str))) +#?(:cljs (def wrapped-textarea (wrap-form-element "textarea" "value" str))) -#?(:cljs (def wrapped-input (wrap-value-input "input"))) -#?(:cljs (def wrapped-checked (wrap-checked-input "input"))) -#?(:cljs (def wrapped-select (wrap-value-input "select"))) -#?(:cljs (def wrapped-textarea (wrap-value-input "textarea"))) +#?(:cljs + (defn defined? [x] + (and (not (nil? x)) + (not (undefined? x))))) #?(:cljs (defn create-element [type props & children] - (let [class (case (keyword type) - :input - (cond (or (nil? props) - (undefined? props)) - "input" - (or (= "radio" (.-type props)) - (= "checkbox" (.-type props))) - (if (exists? (.-checked props)) wrapped-checked "input") - (exists? (.-value props)) - wrapped-input - :else - "input") - :select - (if (and props (exists? (.-value props))) - wrapped-select "select") - :textarea - (if (and props (exists? (.-value props))) - wrapped-textarea "textarea") - ;; else - (name type)) + (let [class (or (when (some? props) + (case (name type) + "input" + (case (.-type props) + "radio" (when (defined? (.-checked props)) wrapped-checked) + "checkbox" (when (defined? (.-checked props)) wrapped-checked) + #_else (when (defined? (.-value props)) wrapped-input)) + "select" (when (defined? (.-value props)) wrapped-select) + "textarea" (when (defined? (.-value props)) wrapped-textarea) + #_else nil)) + (name type)) children (remove nil? children)] + ;; React does not allow for value/checked to be nil, only js/undefined + (when (some? props) + (when (nil? (.-value props)) + (set! (.-value props) js/undefined)) + (when (nil? (.-checked props)) + (set! (.-checked props) js/undefined))) (if (empty? children) (js/React.createElement class props) (apply js/React.createElement class props children)))))