reagent-project / reagent-forms

Bootstrap form components for Reagent
339 stars 78 forks source link

Input field :numeric throws exception #68

Closed reifytech closed 8 years ago

reifytech commented 8 years ago

This :input field:

(def num-form
  [:input {:field :numeric
           :id :number-field}])

(defn aview []
  (fn []
    (let [val (atom {})]
      [bind-fields num-form val])))

Throws: Uncaught Error: No protocol method ICounted.-count defined for type number: 1 cljs$core$missing_protocol @ core.cljs:261 cljs$core$_count @ core.cljs:442 cljs$core$count @ core.cljs:1594 reagent$impl$template$input_set_value @ template.cljs:132 reagent.impl.component.custom_wrapper.c @ component.cljs:104 assign.notifyAll @ react.inc.js:1049 ON_DOM_READY_QUEUEING.close @ react.inc.js:13995 Mixin.closeAll @ react.inc.js:16756 Mixin.perform @ react.inc.js:16697 Mixin.perform @ react.inc.js:16683 assign.perform @ react.inc.js:14899 flushBatchedUpdates @ react.inc.js:14979 ReactPerf.measure.wrapper @ react.inc.js:13427 Mixin.closeAll @ react.inc.js:16756 Mixin.perform @ react.inc.js:16697 ReactDefaultBatchingStrategy.batchedUpdates @ react.inc.js:9199 enqueueUpdate @ react.inc.js:15019 enqueueUpdate @ react.inc.js:14537

yogthos commented 8 years ago

The let statement has to be outside the fn:

(defn aview []
  (let [val (atom {})]
    (fn []
      [bind-fields num-form val])))

What happens there is that you create a state using the let, then return the function that gets called during subsequent repaints. This allows the state to be preserved.

reifytech commented 8 years ago

Thanks for the reply.

I'm getting the same error with the let outside of the fn and also with the atom as a global:

(def val (atom {}))
(def num-form
  [:input {:field :numeric
           :id :number-field}])

(defn about-page []
  (fn []
    [bind-fields num-form val]))

and


(def num-form
  [:input {:field :numeric
           :id :number-field}])

(defn about-page []
  (let [val (atom {})]
    (fn []
      [bind-fields num-form val])))

In all 3 cases, changing the :numeric to :text results in no error thrown.

yogthos commented 8 years ago

I'm not able to reproduce the issue, I've created a test project here with your example and it appears to work as expected when I run it.

yogthos commented 8 years ago

Ah actually, I'm seeing the warning now in the console even when the value is set. I'll take a look and see what's causing it.

yogthos commented 8 years ago

ok I've got a fix cheked in and I should be able to push it out later today

yogthos commented 8 years ago

I just pushed out a new version that should handle this correctly. I changed the behavior to set the document value on blur event as well.

reifytech commented 8 years ago

Thank you for the quick fix. However, when I test this with multiple fields, I'm still seeing the error when I move the focus back to the input field (from number-field-2 back to number-field-1 in this example):

(def num-form
  [:div
   [:input {:field :numeric
            :id :number-field-1}]
   [:input {:field :numeric
            :id :number-field-2}]])

(defn aview []
  (let [val (atom {})]
    (fn []
      [:div
       [:p "value:" (str @val)]
       [bind-fields num-form val]])))

(defn home-page []
  [:div [:h2 "numeric test"]
   [aview]])

(defn mount-root []
  (reagent/render [home-page] (.getElementById js/document "app")))

(defn init! []
  (mount-root))
reifytech commented 8 years ago

And just to clarify, I tested this with the 0.5.7 version.

yogthos commented 8 years ago

I'm not seeing the issue with 0.5.7, have you tried cleaning the js output, sometimes the compiler caches stuff

reifytech commented 8 years ago

Yes, you are correct. Cleaning and restarting the project got rid of those errors. Thank you!

On Thu, Sep 3, 2015 at 9:57 PM, Dmitri Sotnikov notifications@github.com wrote:

I'm not seeing the issue with 0.5.7, have you tried cleaning the js output, sometimes the compiler caches stuff

— Reply to this email directly or view it on GitHub https://github.com/reagent-project/reagent-forms/issues/68#issuecomment-137635538 .