Preserve order of CSS classes

The order of CSS classes matters
see http://stackoverflow.com/questions/15670631/does-the-order-of-classes-listed-on-an-item-affect-the-css

Change the underlying representation from sets to vectors

Adopt the convention : :className comes after the keywordized CSS classes.
ie. [:div.a.b {:className "c d"}] ;;=> "a b c d"

Deduplicate the same classes
ie. [:div.a.b.b {:className "c d"}] ;;=> "a b c d"

CHANGELOG.md

@@ -0,0 +1,7 @@
+# Changelog
+
+## Master (unreleased)
+
+### Changes
+
+* [#98](https://github.com/r0man/sablono/issues/98) Preserve CSS class order

src/sablono/compiler.clj

@@ -32,9 +32,6 @@
          (keyword? value)
          (string? value))
      value
-     (and (sequential? value)
-          (= 1 (count value)))
-     (first value)
      (and (or (sequential? value)
               (set? value))
           (every? string? value))

src/sablono/interpreter.cljc

@@ -74,7 +74,7 @@
    (defn attributes [attrs]
      (let [attrs (clj->js (util/html-to-dom-attrs attrs))
            class (.-className attrs)
-           class (if (array? class) (join " " (sort class)) class)]
+           class (if (array? class) (join " " class) class)]
        (if (blank? class)
          (js-delete attrs "className")
          (set! (.-className attrs) class))

src/sablono/normalize.cljc

@@ -22,35 +22,37 @@
     :else x))
 
 (defn class
-  "Normalize `class` into a set of classes."
+  "Normalize `class` into a vector of classes."
   [class]
   (cond
     (nil? class)
     nil
 
     (list? class)
     (if (symbol? (first class))
-      #{class}
-      (set (map class-name class)))
+      [class]
+      (map class-name class))
 
     (symbol? class)
-    #{class}
+    [class]
 
     (string? class)
-    #{class}
+    [class]
 
     (keyword? class)
-    #{(class-name class)}
+    [(class-name class)]
+
     (and (or (set? class)
              (sequential? class))
          (every? #(or (keyword? %)
                       (string? %))
                  class))
-    (apply sorted-set (map class-name class))
+    (mapv class-name class)
 
     (and (or (set? class)
              (sequential? class)))
-    (set (map class-name class))
+    (mapv class-name class)
+
     :else class))
 
 (defn attributes
@@ -64,8 +66,8 @@
   "Like clojure.core/merge but concatenate :class entries."
   [& maps]
   (let [maps (map attributes maps)
-        classes (map #(into #{} %) (map :class maps))
-        classes (apply set/union classes)]
+        classes (map :class maps)
+        classes (vec (dedupe (apply concat classes)))]
     (cond-> (apply merge maps)
       (not (empty? classes))
       (assoc :class classes))))

src/sablono/util.cljc

@@ -59,7 +59,13 @@
 (defn join-classes
   "Join the `classes` with a whitespace."
   [classes]
-  (join " " (sort (flatten (seq classes)))))
+  (->> (map #(cond
+               (string? %) %
+               :else (seq %))
+            classes)
+       (flatten)
+       (dedupe)
+       (join " ")))
 
 (defn wrapped-type?
   "Return true if the element `type` needs to be wrapped."

test/sablono/compiler_test.clj

@@ -17,6 +17,11 @@
   (toString [this]
     (.toString (.val this))))
 
+(defmethod print-method JSValueWrapper
+  [^JSValueWrapper v, ^java.io.Writer w]
+  (.write w "#js ")
+  (.write w (pr-str (.val v))))
+
 (defn wrap-js-value [forms]
   (prewalk
    (fn [form]
@@ -78,7 +83,7 @@
          js/React.createElement "div"
          (if (clojure.core/map? attrs)
            (sablono.interpreter/attributes
-            (sablono.normalize/merge-with-class {:class #{"foo"}} attrs))
+            (sablono.normalize/merge-with-class {:class ["foo"]} attrs))
            #js {:className "foo"})
          (clojure.core/remove
           clojure.core/nil?
@@ -302,10 +307,10 @@
   (are-html-expanded
    '[:div.a {:class (if (true? true) "true" "false")}]
    '(js/React.createElement
-     "div" #js {:className (sablono.util/join-classes #{"a" (if (true? true) "true" "false")})})
+     "div" #js {:className (sablono.util/join-classes ["a" (if (true? true) "true" "false")])})
    '[:div.a.b {:class (if (true? true) ["true"] "false")}]
    '(js/React.createElement
-     "div" #js {:className (sablono.util/join-classes #{"a" "b" (if (true? true) ["true"] "false")})})))
+     "div" #js {:className (sablono.util/join-classes ["a" "b" (if (true? true) ["true"] "false")])})))
 
 (deftest test-issue-3-recursive-js-literal
   (are-html-expanded
@@ -345,7 +350,7 @@
        js/React.createElement "div"
        (if (clojure.core/map? attrs)
          (sablono.interpreter/attributes
-          (sablono.normalize/merge-with-class {:class #{"aa"}} attrs))
+          (sablono.normalize/merge-with-class {:class ["aa"]} attrs))
          #js {:className "aa"})
        (clojure.core/remove
         clojure.core/nil?
@@ -426,9 +431,20 @@
                 [(sablono.interpreter/interpret attrs)])))))))
 
 (deftest test-class-as-set
-  (are-html-expanded
-   [:div.a {:class #{"a" "b" "c"}}]
-   '(js/React.createElement "div" #js {:className "a b c"})))
+  (is (= (compile [:div.a {:class #{"a" "b" "c"}}])
+         (wrap-js-value
+          '(js/React.createElement "div" #js {:className "a b c"})))))
+
+(deftest test-class-as-list
+  (is (= (compile [:div.a {:class (list "a" "b" "c")}])
+         (wrap-js-value
+          '(js/React.createElement "div" #js {:className (sablono.util/join-classes ["a" (list "a" "b" "c")])})))))
+
+(deftest test-class-as-vector
+  (is (= (compile [:div.a {:class (vector "a" "b" "c")}])
+         (wrap-js-value
+          '(js/React.createElement
+            "div" #js {:className (sablono.util/join-classes ["a" (vector "a" "b" "c")])})))))
 
 (deftest test-class-merge-symbol
   (let [class #{"b"}]
@@ -442,3 +458,16 @@
            "div" nil nil
            (sablono.interpreter/interpret
             (case :a :a "a"))))))
+
+(deftest test-compile-attr-class
+  (are [form expected]
+      (= {:class expected} (compile-attr :class form))
+    nil nil
+    "foo" "foo"
+    '("foo" "bar" ) "foo bar"
+    ["foo" "bar"] "foo bar"
+    #{"foo" "bar"} "foo bar"
+    '(set "foo" "bar")
+    '(sablono.util/join-classes (set "foo" "bar"))
+    '[(list "foo" "bar")]
+    '(sablono.util/join-classes [(list "foo" "bar")])))

test/sablono/core_test.cljs

@@ -504,7 +504,7 @@
   (is (= (let [input-classes ["large" "big"]]
            (html-vec [:input.form-control
                       (merge {:class input-classes})]))
-         [:input {:class "big form-control large"}])))
+         [:input {:class "form-control large big"}])))
 
 (deftest test-issue-33-number-warning
   (is (= (html-vec [:div (count [1 2 3])])
@@ -534,6 +534,35 @@
   (is (= (html-vec [:div.a {:class #{"a" "b" "c"}}])
          [:div {:class "a b c"}])))
 
+
+(deftest test-class-duplication
+  (is (= (html-vec [:div.a.a.b.b.c {:class "c"}])
+         [:div {:class "a b c"}]))  )
+
+(deftest test-class-order
+  (is (= (html-vec [:div.a.b.c {:class "d"}])
+         [:div {:class "a b c d"}]))
+  (is (= (html-vec [:div.a.b.c {:class ["foo" "bar"]}])
+         [:div {:class "a b c foo bar"}])))
+
+(deftest test-class-as-set
+  (is (= (html-vec [:div.a {:class #{"a" "b" "c"}}])
+         [:div {:class "a b c"}]))
+  (is (= (html-vec [:div.a {:class (set ["a" "b" "c"])}])
+         [:div {:class "a b c"}])))
+
+(deftest test-class-as-list
+  (is (= (html-vec [:div.a {:class '("a" "b" "c")}])
+         [:div {:class "a b c"}]))
+  (is (= (html-vec [:div.a {:class (list "a" "b" "c")}])
+         [:div {:class "a b c"}])))
+
+(deftest test-class-as-vector
+  (is (= (html-vec [:div.a {:class ["a" "b" "c"]}])
+         [:div {:class "a b c"}]))
+  (is (= (html-vec [:div.a {:class (vector "a" "b" "c")}])
+         [:div {:class "a b c"}])))
+
 (deftest test-issue-80
   (is (= (html-vec
           [:div
@@ -552,15 +581,19 @@
            (do
              [:div {:class (vector "foo" "bar")}])])
          [:div {}
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]])))
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]])))
 
 (deftest test-issue-90
   (is (= (html-vec [:div nil (case :a :a "a")])
          [:div {} "a"])))
+
+(deftest test-complex-scenario
+  (is (= (html-vec [:div.a {:class (list "b")} (case :a :a "a")])
+         [:div {:class "a b"} "a"])))

test/sablono/interpreter_test.cljs

@@ -92,14 +92,14 @@
            (do
              [:div {:class (vector "foo" "bar")}])])
          [:div {}
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]
-          [:div {:class "bar foo"}]])))
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]
+          [:div {:class "foo bar"}]])))
 
 (deftest test-issue-90
   (is (= (interpret [:div nil (case :a :a "a")])

test/sablono/normalize_test.cljc

@@ -21,11 +21,11 @@
     [{:a 1} {:b 2}]
     {:a 1 :b 2}
     [{:a 1 :class :a} {:b 2 :class "b"} {:c 3 :class ["c"]}]
-    {:a 1 :b 2 :c 3 :class #{"a" "b" "c"}}
+    {:a 1 :b 2 :c 3 :class ["a" "b" "c"]}
     [{:a 1 :class :a} {:b 2 :class "b"} {:c 3 :class (seq ["c"])}]
-    {:a 1 :b 2 :c 3 :class #{"a" "b" "c"}}
-    ['{:a 1 :class #{"a"}} '{:b 2 :class #{(if true "b")}}]
-    '{:a 1 :class #{"a" (if true "b")} :b 2}))
+    {:a 1 :b 2 :c 3 :class ["a" "b" "c"]}
+    ['{:a 1 :class ["a"]} '{:b 2 :class [(if true "b")]}]
+    '{:a 1 :class ["a" (if true "b")] :b 2}))
 
 (deftest test-strip-css
   (are [x expected]
@@ -56,23 +56,23 @@
   (are [class expected]
       (= expected (normalize/class class))
     nil nil
-    :x #{"x"}
-    "x" #{"x"}
-    ["x"] #{"x"}
-    [:x] #{"x"}
-    '(if true "x") #{'(if true "x")}
-    'x #{'x}
-    '("a" "b") #{"a" "b"}))
+    :x ["x"]
+    "x" ["x"]
+    ["x"] ["x"]
+    [:x] ["x"]
+    '(if true "x") ['(if true "x")]
+    'x ['x]
+    '("a" "b") ["a" "b"]))
 
 (deftest test-attributes
   (are [attrs expected]
       (= expected (normalize/attributes attrs))
     nil nil
     {} {}
     {:class nil} {:class nil}
-    {:class "x"} {:class #{"x"}}
-    {:class #{"x"}} {:class #{"x"}}
-    '{:class #{"x" (if true "y")}} '{:class #{(if true "y") "x"}}))
+    {:class "x"} {:class ["x"]}
+    {:class ["x"]} {:class ["x"]}
+    '{:class ["x" (if true "y")]} '{:class ["x" (if true "y")]}))
 
 (deftest test-children
   (are [children expected]
@@ -93,7 +93,7 @@
     [:div] ["div" {} '()]
     [:div {:class nil}] ["div" {:class nil} '()]
     [:div#foo] ["div" {:id "foo"} '()]
-    [:div.foo] ["div" {:class #{"foo"}} '()]
-    [:div.a.b] ["div" {:class #{"a" "b"}} '()]
-    [:div.a.b {:class "c"}] ["div" {:class #{"a" "b" "c"}} '()]
-    [:div.a.b {:class nil}] ["div" {:class #{"a" "b"}} '()]))
+    [:div.foo] ["div" {:class ["foo"]} '()]
+    [:div.a.b] ["div" {:class ["a" "b"]} '()]
+    [:div.a.b {:class "c"}] ["div" {:class ["a" "b" "c"]} '()]
+    [:div.a.b {:class nil}] ["div" {:class ["a" "b"]} '()]))

test/sablono/util_test.cljc

@@ -41,6 +41,16 @@
   (is (not (u/element? 1)))
   (is (not (u/element? "x"))))
 
+(deftest test-join-classes
+  (are [classes expected]
+      (= expected (u/join-classes classes))
+    ["a"] "a"
+    #{"a"} "a"
+    ["a" "b"] "a b"
+    #{"a" "b"} "a b"
+    ["a" ["b"]] "a b"
+    ["a" (set ["a" "b" "c"])] "a b c"))
+
 #?(:cljs
    (deftest test-as-str
      (are [args expected]