typo in util.cljs fetch-merged-css

[PiotrWasik]

is
use-toplevel (get :use-toplevel defaults (if (= tag :main) true false))

should be
use-toplevel (get defaults :use-toplevel (if (= tag :main) true false))

line 308

[PiotrWasik]

... also may I suggest in line 294 in util.cljs

class (reduce into [] [(if (string? acl) [acl] acl) (if (string? bcl) [bcl] bcl)])

in addition to string? allow for symbol? - it will be consistent with raw hiccup that allows for symbols as css class designators.

[PiotrWasik]

text.cljs:165 - instead of (reduce (partial dissoc m) [:class :style]) should be (reduce dissoc m [:class :style])

same style util.cljs:306 instead of (reduce (partial dissoc options) [:class :style :attr]) should be (reduce dissoc options [:class :style :attr])

I understand the intention is to remove :class :style and :attr keys from the map.

also in util.cljs:317 I believe the return value should be changed from (reduce combine-css [defaults options user]) to (reduce combine-css [defaults xoptions user])

For example imagine I use [re-com/button :class "foo" ...] - then in buttons.cljs:61 the cmerger is created with button-css-spec and args, which includes {:class "foo"}. "foo" will be captured as class variable in the closure, and it will become a part of user map, util.cljs:310
It is also part of options, exactly the same "foo", because it is passed from button in buttons.cljs:64 in this expression (flatten-attr (cmerger :main {:class class :disabled? disabled?})) therefore ending twice in the return value, so the resulting tag has "foo" twice in the class list. if I change it the return value to (reduce combine-css [defaults xoptions user]) it ends up only once because now, with the previous fix, xoptions has no :class anymore.

And in

(into {} (for [k [:class :style :attr]
                                   :when (contains? defaults k)
                                   :let [v (get defaults k)]]
                               [k (if (fn? v) (v options) v)])

I would read from options, not xoptions which now have no :class and sometimes (e.g. in button component) is used.

Is it correct please? I only started eyeballing the code this weekend, so I may be missing something.

To sum it up for now, I would change the whole merge-css function to

(defn merge-css [css-desc {:as params :keys [class style attr parts]}]
  ;; it really works in dev, check once for prod build if it uses this version, not the original one
  #_(.log js/console "Hello in merge-css")
  (for [[k v] css-desc
        :when (not (and (keyword? k) (map? v)))]
    (throw (js/Error. "CSS description must contain only keywords and maps")))

  (defn combine-css [a b]
    (let [a (or a {})
          b (or b {})
          acl (:class a)
          bcl (:class b)
          class (reduce into [] [(if (or (string? acl)
                                         (symbol? acl)) [acl] acl)
                                 (if (or (string? bcl)
                                         (symbol? bcl)) [bcl] bcl)])
          style (reduce into {} [(:style a) (:style b)])
          attr (reduce into {} [(:attr a) (:attr b)])]
      (into {}
            [(when-not (empty? class) [:class class])
             (when-not (empty? style) [:style style])
             (when-not (empty? attr) [:attr attr])])))

  (defn fetch-merged-css
    ([tag]
     (fetch-merged-css tag {}))
    ([tag options]
     (let [xoptions (reduce dissoc options [:class :style :attr])
           defaults (get css-desc (or tag :main))
           use-toplevel (get defaults :use-toplevel (= tag :main))
           user (combine-css (get parts tag)
                             (and use-toplevel {:class class :style style :attr attr}))
           defaults (into {} (for [k [:class :style :attr]
                                   :when (contains? defaults k)
                                   :let [v (get defaults k)]]
                               [k (if (fn? v) (v options) v)]))]
       (when (and tag (not (contains? css-desc tag)))
         (println "Missing!!!: " tag))
       (reduce combine-css [defaults xoptions user]))))
  fetch-merged-css)

[BnMcGn]

Thanks!
I'll have a look at your suggestions.

Just to forestall too much effort on this branch: the main re-com project has gone in a significantly different direction than merge-css. Re-com-tailwind will need a rewrite in order to remain compatible. I don't have time to do that right now.

So probably just bugfixes on this code base. You might want to look over the current branch of re-com to see what has been done there. Try looking in https://github.com/day8/re-com/tree/master/src/re_com/theme.

[PiotrWasik]

so if I want to use tailwind, am I to try to shoehorn (e.g. by set!-ing things) the tailwind classes in the main branch? Perhaps starting with base components like box h-box etc. then progress to basic ones like button as I need?

are there any examples of applying themes or perhaps even replacing bootstrap? I don't know if tailwind and bootstrap play well together.

[PiotrWasik]

sorry, one more question - can I somehow "start small" with the existing framework to get tailwind? e.g. in your branch I tried this, taken from a tailwind template like https://tailwindui.com/templates/keynote - I bought a source code for it, and I replaced colours with more green, then I injected them into re-com.buttons/button-css-spec

(def button-base (str "flex justify-center items-center rounded-lg p-2 text-base font-semibold text-white "
                      "focus:outline-none focus-visible:outline-2 focus-visible:outline-offset-2 "
                      "active:text-white/70 h-10"))

(def button-class-green
  (str button-base " bg-green-600 hover:bg-green-500 focus-visible:outline-green-500"))

(def button-class-teal
  (str button-base " bg-teal-600 hover:bg-teal-500 focus-visible:outline-teal-500"))


(set! re-com.buttons/button-css-spec
  {:main {:class (fn [{:keys [color class disabled?]}]
                   (case color
                    ;; here I am injecting my tw classes
                     :green button-class-green
                     :teal button-class-teal
                     nil (cond-> ["rc-button"]
                           true re-com-tailwind.functions/tw-btn
                           (empty? class) ((if disabled?
                                             re-com-tailwind.functions/tw-btn-default-disabled
                                             re-com-tailwind.functions/tw-btn-default)))))}
   :wrapper {:class ["rc-button-wrapper" "display-inline-flex"]}
   :tooltip {:class ["rc-button-tooltip"]}})

[BnMcGn]

Have you tried just pushing your tailwind classes into each component using the :class parameter or perhaps the :parts interface? It's probably simpler than tampering with the *-css-spec structure.

[PiotrWasik]

Yes, I started with adding my classes using the :class parameter, but it appended my classes to the ones from css-specs, rather than overwriting it. In my simple cases it did not cause any issues, but I thought modifying the css-spec was cleaner.

Importantly, I started looking into re-com main branch as you warned me that merge-css is no longer there, so maybe indeed it is not worth investing in the re-com-tailwind branch. I see there is no equivalent of css-specs there, but classes like "rc-button" (undefined in .css, probably left for users' customisation) and "btn" from bootstrap are included in the component source code. Over this week (and after day job is done!) I will try to understand how themes work.

Given that the tailwind branch is obsolete and it is not as polished as the main branch (e.g. tooltip over buttons in the tailwind branch don't work well), perhaps I should use the main branch, accept that it is bootstrap, and try to style the components with tailwind classes uses some tailwind configuration tricks like prefixed classes and forced high specificity by including app ID. At least it will get me started, and if I have time to port it to tw later fully, I can try - but starting from the main branch. I don't know how much the complex components like tables, date pickets etc. rely on bootstrap in re-com, maybe not rely at all?

[BnMcGn]

The re-com maintainers intend to remove the hard coded bootstrap dependency. I don't know how far they have gotten.

I built the merge-css tool and the *-css-spec structures in order to do this. The re-com maintainers decided on a different toolset/format to do the same thing, discussed here: day8/re-com#327.

As you say, it seems to mostly be in the themes folder. I imagine that someone will still need to go through all of the components and separate out the css bits into their chosen data structures. I don't have time for it now. Having done it once already, I assure you that it is plenty of work!

My recollection is that there is plenty of bootstrap stuff scattered through all of the components.