Custom styling for labels for Tabs

[ducky427]

Hi,
Firstly a great library. Its really well done!

I am also building an atom-shell app using cljs, reagent and bootstrap. So this fits in really neatily into the stack.

I had a suggesting for enhancement, namely being able to apply a custom function to a label on a tab. I noticed that strictly speaking it doesn't need to be a string. The label just gets nested inside a :a tag.

I suggest that instead of :label being a requirement for an element :tabs, the user have an option to give a label-fn which when given an id produces the correct markup.

My use case is that I have dynamic tabs some of which can be closed. This can be done with the current API itself but not in a clean way.

Thanks!

P.S. I am happy to submit a PR.

[ducky427]

This case is different from places where you are expecting label to a string or valid hiccup markup because in this you are expected to store that data inside an atom or a map.

[mike-thompson-day8]

How about this compromise?

Create a new parameter called :label-fn which defaults to :label. This function will be called with one parameter: a tab spec and it is expected to "extract" the appropriate string of hiccup for showing in the tab.

That would allow you to supply a label-fn which looked up the :id of the tab and then used it to generate necessary hiccup (etc). This approach is flexible but 100% backwards compatible (given this function defaults to :label)

While we are at it, should we also create an :id-fn parameter which defaults to :id.

[ducky427]

Thats exactly what I had in mind. So the changes would be something like:

(let [id-fn       (or (:id-fn t)
                      (fn [x] (:id x)))
      label-fn    (or (:label-fn t) 
                      (fn [x] (:label x)))
      id          (id-fn t)
      label       (label-fn t)]
....

[mike-thompson-day8]

The nice thing about "named parameters" is that you can provide defaults.

So the default values for :label-fn would be :label

[mike-thompson-day8]

So this code:
https://github.com/Day8/re-com/blob/master/src/re_com/tabs.cljs#L20-L42

Would change to look like this (untested):

(defn horizontal-tabs
  [& {:keys [model tabs on-change label-fn id-fn]       ;;  <---   two added 
    :or   {label-fn :label id-fn :id}                      ;;  <---  added  defaults
      :as   args}]  
  {:pre [(validate-args-macro tabs-args-desc args "tabs")]}
  (let [current  (deref-or-value model)
        tabs     (deref-or-value tabs)
        _        (assert (not-empty (filter #(= current (:id %)) tabs)) "model not found in tabs vector")]
    [:ul
     {:class "rc-tabs nav nav-tabs noselect"
      :style (flex-child-style "none")}
     (for [t tabs]
       (let [id        (id-fn  t)                              ;; <--   change 
             label     (label-fn  t)                         ;; <---    change 
             selected? (= id current)]                   ;; must use current instead of @model to avoid reagent warnings
         [:li
          {:class  (if selected? "active")
           :key    (str id)}
          [:a
           {:style     {:cursor "pointer"}
            :on-click  (when on-change (handler-fn (on-change id)))
            }
           label]]))]))

[ducky427]

ah of course. I was obviously doing the wrong thing.

Thanks!

[ducky427]

I got the propsed changes working in my branch. Further changes:

1. Use id-fn in assert on Line 26.
2. Add the two arguments to tabs-args-desc.

I haven't modified the other tab styles but they would also be impacted by 2).

[mike-thompson-day8]

Thanks! We'll push a new version later today.

[ducky427]

Thats great. Thanks @mike-thompson-day8!

[ducky427]

Works in v0.5.0.

Thanks a lot!