diff --git a/frontend/src/app/main/ui/workspace/colorpicker.cljs b/frontend/src/app/main/ui/workspace/colorpicker.cljs index c886629871..e749dd259b 100644 --- a/frontend/src/app/main/ui/workspace/colorpicker.cljs +++ b/frontend/src/app/main/ui/workspace/colorpicker.cljs @@ -607,7 +607,7 @@ :top top-offset :maxHeight max-height-top})))) -(defn- group-sets +(defn group-sets "Groups sets by their parent path (everything before the last '/') if present. The set name is always the last part of the path. @@ -777,6 +777,10 @@ (mf/with-memo [tokens-lib active-sets-names color-tokens] (some-> tokens-lib (ctob/get-sets) + ;; Show highest-precedence (last-defined) sets first in the + ;; picker; the Tokens panel itself keeps definition order. + ;; https://github.com/penpot/penpot/issues/10552 + (reverse) (add-tokens-to-sets) (filter-active-sets active-sets-names) (filter-non-empty-sets) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 2e473f1411..3c03ee7e18 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -54,6 +54,7 @@ [frontend-tests.tokens.style-dictionary-test] [frontend-tests.tokens.token-errors-test] [frontend-tests.tokens.workspace-tokens-remap-test] + [frontend-tests.ui.colorpicker-token-set-order-test] [frontend-tests.ui.comments-clustering-test] [frontend-tests.ui.comments-position-modifier-test] [frontend-tests.ui.ds-controls-numeric-input-test] @@ -129,6 +130,7 @@ 'frontend-tests.tokens.style-dictionary-test 'frontend-tests.tokens.token-errors-test 'frontend-tests.tokens.workspace-tokens-remap-test + 'frontend-tests.ui.colorpicker-token-set-order-test 'frontend-tests.ui.comments-clustering-test 'frontend-tests.ui.comments-position-modifier-test 'frontend-tests.ui.ds-controls-numeric-input-test diff --git a/frontend/test/frontend_tests/ui/colorpicker_token_set_order_test.cljs b/frontend/test/frontend_tests/ui/colorpicker_token_set_order_test.cljs new file mode 100644 index 0000000000..34347ffa26 --- /dev/null +++ b/frontend/test/frontend_tests/ui/colorpicker_token_set_order_test.cljs @@ -0,0 +1,65 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en EspaƱa SL + +(ns frontend-tests.ui.colorpicker-token-set-order-test + (:require + [app.main.ui.workspace.colorpicker :refer [group-sets]] + [cljs.test :as t :include-macros true])) + +;; https://github.com/penpot/penpot/issues/10552 +;; The Color tokens picker shows sets in reverse of their definition order +;; (highest-precedence / last-defined set first). The fix reverses the raw +;; set seq once, before it reaches `group-sets`. These tests drive +;; `group-sets` with already-reversed input -- matching what it receives in +;; the real pipeline -- to prove the `group-by` call inside `group-sets` +;; doesn't scramble that reversal, for both an ungrouped list and a +;; subgrouped one. + +(defn- flat-entries + "Reduce group-sets' output (a mix of single-set and multi-set grouped + entries) to a flat [group name] seq, in output order." + [grouped] + (mapcat (fn [{:keys [group sets]}] + (map (fn [{:keys [name]}] [group name]) sets)) + grouped)) + +(t/deftest reversed-flat-set-list + (let [;; definition order: global, alias, semantic (ascending precedence) + definition-order [{:id 1 :set "global" :tokens []} + {:id 2 :set "alias" :tokens []} + {:id 3 :set "semantic" :tokens []}] + result (group-sets (reverse definition-order))] + (t/is (= [[nil "semantic"] [nil "alias"] [nil "global"]] + (flat-entries result))))) + +(t/deftest reversed-subgroup-members-are-not-scrambled + (let [;; brand/subgroup/one is defined before brand/subgroup/two + definition-order [{:id 1 :set "brand/subgroup/one"} + {:id 2 :set "brand/subgroup/two"}] + definition-order (map #(assoc % :tokens []) definition-order) + result (group-sets (reverse definition-order))] + ;; "two" has higher precedence (defined later) so it must lead -- + ;; group-by must not silently restore definition order within the + ;; subgroup's own member list. + (t/is (= [["brand/subgroup" "two"] ["brand/subgroup" "one"]] + (flat-entries result))))) + +(t/deftest reversed-mixed-flat-and-subgroup-set-list + (let [;; definition order: an ungrouped set, then a two-member subgroup, + ;; then another ungrouped set (highest precedence). + definition-order [{:id 1 :set "global" :tokens []} + {:id 2 :set "brand/subgroup/one" :tokens []} + {:id 3 :set "brand/subgroup/two" :tokens []} + {:id 4 :set "primitives" :tokens []}] + result (group-sets (reverse definition-order)) + entries (flat-entries result)] + ;; Every set is present exactly once, and nothing outside the + ;; subgroup reordered its two members relative to each other. + (t/is (= #{[nil "global"] [nil "primitives"] + ["brand/subgroup" "one"] ["brand/subgroup" "two"]} + (set entries))) + (t/is (= [["brand/subgroup" "two"] ["brand/subgroup" "one"]] + (filter #(= (first %) "brand/subgroup") entries)))))