Finished TodoMVC Implementation

[harismh]

Hey there,

Wishing you Happy Holidays and a Merry Christmas. I appreciate all of your work in developing this library, and for your blog posts/talks. They were immensely helpful for me, a Clojure novice, to understand data-driven apps.

From the readme, I noticed you called out a lack of a TodoMVC for Dumdom. So, I took the initiative to create one: https://github.com/harismh/todomvc-dumdom. Any feedback, even if it's fussy, is much appreciated as I don't have a solid grasp on idiomatic Clojure. I feel lots of annotated code is still needed before the repo becomes a good learning resource. I'm happy to add those comments if you feel my implementation is good enough.

One known issue I am still trying to nail down is the input not respecting the passed-in value. Even when the input value gets wiped from the global store, the input in the dumdom component retains its value. It seems to respect the state value on first mount but then ignores it on subsequent re-renders. Taking a page out of React, what I think I need is a controlled input. However, if there's another way please let me know.

Thanks again!

[cjohansen]

Hey, thanks for doing this! It looks very good 😊

The input field seems to be implemented the right way - dumdom components are "controlled" by default. What I suspect is the issue is that you're resetting the input value to nil, which is then ignored. This is a bit of a trap, unfortunately. Reset it to an empty string, and it should work.

I like how you separated the wiring, the data model, and the UI. If I were to make one suggestion, it would be to go even harder in this direction - make the UI entirely generic. The todo-checkbox is just a checkbox, there's nothing todo-specific about it. The prepare-ui-data function is a great place to convert all domain terms to generic UI terms.

I like to write it so that the keys on the outgoing data match the generic component names, something like:

{:items [{:title "Complete me"
          :toggle {:checked? false
                   :action [[:todo/edit-todo {:id "v3pYXsm0"
                                              :completed? true}]]} 
          :form {:value "Complete"
                 :action [[:todo/edit-todo {:id id}]]}}

         :actions {:edit [[:todo/edit-todo {:id "v3pYXsm0"
                                            :editing? true}]]
                   :delete [[:todo/delete-todo {:id "v3pYXsm0"}]]}
         ,,,]}

This way your components will just be a generic nice list with an optional checkbox and actions. By not tying the components to the specific actions, they can be used for pretty much anything that looks remotely similar.

[harismh]

Hey, thanks for the detailed review and comments. I'll double check that nil input string, pretty sure it's set as an empty string in the model and initial state, but there could be a state-transition I'm missing. Good lead, nonetheless.

I agree that I should generalize the components further. I passed-in most props already, but what's your opinion on fixed CSS classes? Because I'm inheriting styles from the TodoMVC folks, there are some components that have classes fixed, eg. .toggle in the checkbox case:

(defcomponent todo-checkbox [{:keys [id completed?]}]
  [:input.toggle {:type "checkbox"
                  :checked completed?
                  :on-change [[:todo/edit-todo {:id id :completed? (not completed?)}]]}])

Those fixed classes are what's driving me to avoid general definitions. Should I still define this as checkbox, or perhaps checkbox-toggle to hint at the class being used, or perhaps go even further and pass in the classes as args. If it's the third option, what do about multiple classes since that gets awkward quickly, {:container-class "xyz" :subcontainer-class: "abc" :class-before-label: "123" ...}.

[cjohansen]

The classes are outside your control, so I would just use them as is and not let them control your component naming. Treat the classes as an implementation detail :)

[harismh]

Appreciate the direction. Updated the repo with more generic UI components: https://github.com/harismh/todomvc-dumdom/commit/74dc1d80b23799531c9abe3c6b69eb0de3b9cb16. You weren't kidding about running out of names to label UI blocks.

[cjohansen]

Yeah, it's not always easy :D But the UI components get much more usable this way. Looks good! I'll add a link to the repo in the readme 👍

[harismh]

Thanks much! Going to add a lot more annotations to make it more learner-friendly, but I appreciate all of the review/guidance in getting the structure right.

All the best with Replicant :)