reagent-project / reagent-forms

Bootstrap form components for Reagent
339 stars 78 forks source link

single-select list not setting active item #152

Closed nustiueudinastea closed 5 years ago

nustiueudinastea commented 5 years ago

Hi there, I am having a weird issue where a single select list item is not being set to active when the class is set explicitly via the {:class ""} attribute. The following example does not work:

[:div {:class "list-group-item" :key id} (:name item)]

while this works:

[:div.list-group-item {:key id} (:name item)]

This is the whole component.

(defn single-select-list [properties items]
  [:div {:class "form-group"}
      [:ul (conj {:class "list-group"} properties)
      (for [[id item] items]
        [:div {:class "list-group-item" :key id} (:name item)])]])

Took me a long time to figure out the problem and I am totally baffled and don't understand why this would happen. I though the two hiccup representations above are equivalent. Is this a bug or am I missing something?

Thanks!

yogthos commented 5 years ago

Hi,

Nothing jumps out at me regarding using the different syntax for creating the class. However, the example doesn't appear to be using the library as far as I can tell. The library components must use :field key to identify each widget, so you'd write something like the following:

(defn single-select-list [properties items]
  [:div {:class "form-group"}
   [:ul (merge {:field :single-select
                :id :pick-one
                :class "list-group"}
               properties)
  (for [[id item] items]
    [:li {:class "list-group-item" :key id} (:name item)])]])

(def items
  [["foo" {:name "Foo"}]
   ["bar" {:name "Bar"}]])

(defn home-page []
  (r/with-let [doc (r/atom {})]
    [:div
     [:p (str @doc)]
     [bind-fields (single-select-list {} items) doc]]))

The above case works as expected for me. Note that it's important to evaluate the single-select-list function eagerly before it's passed to bind-fields as the template has to be a data structure that reagent-forms can walk and inject components into.

nustiueudinastea commented 5 years ago

Hi @yogthos, here is the calling code:

[bind-fields
              (single-select-list {:field :single-select :id :init-wizard.step2.selected-provider} dns-providers)
              (util/form-events [:init-form :step2])]

I also used your items and single-select-list functions in my code and I get the same weird behaviour. I even removed my util/form-events and replaced it with a ratom and it does not change. I want to make it clear that even though the selection is not highlighted, the right ratom get's populated both with the old code and your code, so the only problem seems to be the lack of the "active" attribute in the class attribute.

The code in question can be found here: https://github.com/protosio/frontend/blob/b477b29b5d0ca771230e0cdeb9341d45a8d4e6c8/src/protosfrontend/init/views.cljs#L66

Any suggestions on how I could troubleshoot this further? I don't mind falling back to the implicit class notation but I would definitely like to understand this behaviour.

yogthos commented 5 years ago

It seems correct to me overall, but looks like there's a typo here:

(defn single-select-list [properties items]
  [:div {:class "form-group"}
      [:ul {:class "list-group"} properties
      (for [item items]
[:div {:class "list-group-item" :key (:id item)} (:name item)])]])

it should be [:ul (merge {:class "list-group"} properties) .... Note that you can mix the inline class declaration as well with [:ul.list-group properties ....

nustiueudinastea commented 5 years ago

Hey @yogthos , thanks for catching that. I fixed it locally before finding the active attribute problem but it hasn't influenced the behaviour.

I will close this issue for now since it's not easy to reproduce. If I get the chance I will try to dig in deeper and see if I can figure out the root cause. Thanks for your time and your work on reagent is much appreciated!