mirror of
https://github.com/penpot/penpot.git
synced 2026-11-04 09:00:37 +00:00
♻️ Name conflicting properties conflicts, not clashes, in code
The conflict dialogs now say conflict throughout, so the code says it too. The attr source :clash becomes :conflict, clash-attrs becomes conflicting-attrs, hidden-clashes becomes hidden-conflicts, text-clash? becomes text-conflict?, the row class is-clash becomes is-conflicting, and the message keys resolve.tag.clash and resolve.tag.clashes become resolve.tag.conflict and resolve.tag.conflicts. The tests and their names follow, and the tag documentation now cites keys that exist. AI-assisted-by: claude-opus-5-5
This commit is contained in:
parent
9c6a2f498e
commit
30274b5320
@ -38,8 +38,8 @@ is written in; the tag never turns it uppercase.
|
||||
## Technical notes
|
||||
|
||||
```clj
|
||||
[:> tag* {:tone "warning"} (tr "workspace.branches.review.tag.clash")]
|
||||
[:> tag* {:tone "success" :variant "pill"} (tr "workspace.branches.review.decided")]
|
||||
[:> tag* {:tone "warning"} (tr "workspace.branches.resolve.tag.conflict")]
|
||||
[:> tag* {:tone "success" :variant "pill"} (tr "workspace.branches.resolve.decided")]
|
||||
```
|
||||
|
||||
## Usage guidelines (design)
|
||||
|
||||
@ -8,7 +8,7 @@
|
||||
"What the compare and conflict dialogs say about a diff entry, kept free
|
||||
of rendering: labels, values, categories, one-line summaries, the
|
||||
page/board a change lives on, how a conflict splits into the
|
||||
properties that clash and the ones only one side touched, and what the
|
||||
properties in conflict and the ones only one side touched, and what the
|
||||
copy rule decides for the copies of a changed component."
|
||||
(:require
|
||||
[app.common.data.macros :as dm]
|
||||
@ -589,14 +589,14 @@
|
||||
:boards []
|
||||
:count (count library)}))))
|
||||
|
||||
;; --- Conflicts: what clashes and what one side did alone
|
||||
;; --- Conflicts: which properties conflict and what one side did alone
|
||||
|
||||
(defn- attr-sources
|
||||
"Every changed attr of a conflict with who changed it, using the base
|
||||
value both sides started from. `:clash` attrs moved on both sides; a
|
||||
value both sides started from. `:conflict` attrs moved on both sides; a
|
||||
`:main` or `:branch` attr moved on that side alone, so its value is not
|
||||
in doubt. Without a base (both sides added the entity) every attr
|
||||
clashes."
|
||||
conflicts."
|
||||
[conflict]
|
||||
(let [base (:base conflict)
|
||||
has-base? (map? base)]
|
||||
@ -608,48 +608,48 @@
|
||||
:main main
|
||||
:branch branch
|
||||
:source (cond
|
||||
(not has-base?) :clash
|
||||
(not has-base?) :conflict
|
||||
(= base-v main) :branch
|
||||
(= base-v branch) :main
|
||||
:else :clash)})))
|
||||
:else :conflict)})))
|
||||
(sort-by (comp attr-label :attr))
|
||||
vec)))
|
||||
|
||||
(defn conflict-attr-rows
|
||||
"The attrs of a conflict a person decides on (see `attr-sources`).
|
||||
Technical attrs are left out: they follow the visible ones they come
|
||||
with. When only technical attrs clash, they are shown instead, so the
|
||||
with. When only technical attrs conflict, they are shown instead, so the
|
||||
conflict can still be decided."
|
||||
[conflict]
|
||||
(let [all (attr-sources conflict)
|
||||
visible (filterv #(not (contains? technical-attrs (:attr %))) all)]
|
||||
(if (and (not-any? #(= :clash (:source %)) visible)
|
||||
(some #(= :clash (:source %)) all))
|
||||
(if (and (not-any? #(= :conflict (:source %)) visible)
|
||||
(some #(= :conflict (:source %)) all))
|
||||
all
|
||||
visible)))
|
||||
|
||||
(defn clash-attrs
|
||||
(defn conflicting-attrs
|
||||
[rows]
|
||||
(into [] (comp (filter #(= :clash (:source %))) (map :attr)) rows))
|
||||
(into [] (comp (filter #(= :conflict (:source %))) (map :attr)) rows))
|
||||
|
||||
(defn- hidden-clashes
|
||||
"The clashing attrs `conflict-attr-rows` does not show."
|
||||
(defn- hidden-conflicts
|
||||
"The conflicting attrs `conflict-attr-rows` does not show."
|
||||
[conflict]
|
||||
(let [shown (into #{} (map :attr) (conflict-attr-rows conflict))]
|
||||
(into []
|
||||
(comp (filter #(= :clash (:source %)))
|
||||
(comp (filter #(= :conflict (:source %)))
|
||||
(map :attr)
|
||||
(remove shown))
|
||||
(attr-sources conflict))))
|
||||
|
||||
(defn prefill-resolution
|
||||
"The per-attr resolution a conflict starts with: every attr one side
|
||||
changed alone takes that side. The clashing attrs stay out, so the
|
||||
changed alone takes that side. The conflicting attrs stay out, so the
|
||||
conflict is not resolved until the user picks them; a conflict without
|
||||
a clash comes out resolved."
|
||||
conflicting attrs comes out resolved."
|
||||
[conflict]
|
||||
(into {}
|
||||
(comp (remove #(= :clash (:source %)))
|
||||
(comp (remove #(= :conflict (:source %)))
|
||||
(map (juxt :attr :source)))
|
||||
(attr-sources conflict)))
|
||||
|
||||
@ -671,17 +671,17 @@
|
||||
:else (prefill-resolution conflict))]
|
||||
(reduce (fn [res hidden] (assoc res hidden side))
|
||||
(assoc seed attr side)
|
||||
(hidden-clashes conflict))))
|
||||
(hidden-conflicts conflict))))
|
||||
|
||||
(defn choose-side
|
||||
"The resolution of `conflict` when every clashing attr takes `side`; the
|
||||
"The resolution of `conflict` when every conflicting attr takes `side`; the
|
||||
independent edits keep the side that made them."
|
||||
[conflict side]
|
||||
(if (per-attr? conflict)
|
||||
(let [rows (conflict-attr-rows conflict)]
|
||||
(reduce (fn [res attr] (assoc res attr side))
|
||||
(prefill-resolution conflict)
|
||||
(into (clash-attrs rows) (hidden-clashes conflict))))
|
||||
(into (conflicting-attrs rows) (hidden-conflicts conflict))))
|
||||
side))
|
||||
|
||||
(defn attr-choice
|
||||
|
||||
@ -6,7 +6,7 @@
|
||||
|
||||
(ns app.main.ui.workspace.sidebar.branch-review.conflicts
|
||||
"The decisions dialog: one entity both sides changed at a time. Only the
|
||||
properties that really clash ask for a choice; the ones a single side
|
||||
properties really in conflict ask for a choice; the ones a single side
|
||||
changed are filled in from that side, and the dialog says what each
|
||||
choice drops before the merge applies it."
|
||||
(:require-macros [app.main.style :as stl])
|
||||
@ -47,26 +47,26 @@
|
||||
:delete-modify "workspace.branches.conflicts.reason.delete-modify"
|
||||
:modify-delete "workspace.branches.conflicts.reason.modify-delete"})
|
||||
|
||||
(defn- text-clash?
|
||||
(defn- text-conflict?
|
||||
"A conflict whose text content both sides rewrote."
|
||||
[conflict]
|
||||
(and (brc/per-attr? conflict)
|
||||
(some #(and (= :content (:attr %)) (= :clash (:source %)))
|
||||
(some #(and (= :content (:attr %)) (= :conflict (:source %)))
|
||||
(brc/conflict-attr-rows conflict))))
|
||||
|
||||
(defn- loses-work?
|
||||
"Whether every choice on a text conflict drops a paragraph edit: both
|
||||
sides edited different paragraphs."
|
||||
[conflict]
|
||||
(and (text-clash? conflict)
|
||||
(and (text-conflict? conflict)
|
||||
(some #(= :dropped (:status %)) (brc/paragraph-rows conflict :main))
|
||||
(some #(= :dropped (:status %)) (brc/paragraph-rows conflict :branch))))
|
||||
|
||||
(defn- row-subtitle
|
||||
[conflict res]
|
||||
(let [rows (brc/conflict-attr-rows conflict)
|
||||
clash (brc/clash-attrs rows)
|
||||
drops (brc/dropped-values conflict res)]
|
||||
(let [rows (brc/conflict-attr-rows conflict)
|
||||
conflicting (brc/conflicting-attrs rows)
|
||||
drops (brc/dropped-values conflict res)]
|
||||
(cond
|
||||
(bm/conflict-resolved? conflict res)
|
||||
(cond
|
||||
@ -76,9 +76,9 @@
|
||||
(str/lower (str/join ", " (map (comp brc/attr-label :attr) drops))))
|
||||
:else (tr "workspace.branches.resolve.row.kept-both"))
|
||||
|
||||
(seq clash)
|
||||
(seq conflicting)
|
||||
(tr "workspace.branches.resolve.row.both-changed"
|
||||
(str/join ", " (map brc/attr-label clash)))
|
||||
(str/join ", " (map brc/attr-label conflicting)))
|
||||
|
||||
:else
|
||||
(some-> (get reason->label (:reason conflict)) tr))))
|
||||
@ -124,28 +124,28 @@
|
||||
[:span]]
|
||||
(for [{:keys [attr main branch source]} rows
|
||||
:let [choice (brc/attr-choice res attr)
|
||||
clash? (= source :clash)
|
||||
result (case (or choice (when-not clash? source))
|
||||
conflicting? (= source :conflict)
|
||||
result (case (or choice (when-not conflicting? source))
|
||||
:main main
|
||||
:branch branch
|
||||
nil)
|
||||
decided? (some? (or choice (when-not clash? source)))]]
|
||||
[:div {:class (stl/css-case :table-row true :is-clash (and clash? (nil? choice)))
|
||||
decided? (some? (or choice (when-not conflicting? source)))]]
|
||||
[:div {:class (stl/css-case :table-row true :is-conflicting (and conflicting? (nil? choice)))
|
||||
:key (str attr)}
|
||||
[:> text* {:as "span" :typography t/body-small :class (stl/css :td-name)}
|
||||
(brc/attr-label attr)]
|
||||
(if (= source :branch)
|
||||
[:> text* {:as "span" :typography t/body-small :class (stl/css :td-unchanged)}
|
||||
(tr "workspace.branches.resolve.unchanged")]
|
||||
[:button {:class (stl/css-case :td-pick true :is-chosen (and clash? (= choice :main)))
|
||||
:disabled (not clash?)
|
||||
[:button {:class (stl/css-case :td-pick true :is-chosen (and conflicting? (= choice :main)))
|
||||
:disabled (not conflicting?)
|
||||
:on-click #(on-pick attr :main)}
|
||||
[:> brl/value-chip* {:value main :is-old (not= choice :main)}]])
|
||||
(if (= source :main)
|
||||
[:> text* {:as "span" :typography t/body-small :class (stl/css :td-unchanged)}
|
||||
(tr "workspace.branches.resolve.unchanged")]
|
||||
[:button {:class (stl/css-case :td-pick true :is-chosen (and clash? (= choice :branch)))
|
||||
:disabled (not clash?)
|
||||
[:button {:class (stl/css-case :td-pick true :is-chosen (and conflicting? (= choice :branch)))
|
||||
:disabled (not conflicting?)
|
||||
:on-click #(on-pick attr :branch)}
|
||||
[:> brl/value-chip* {:value branch :is-old (not= choice :branch)}]])
|
||||
(if decided?
|
||||
@ -155,12 +155,12 @@
|
||||
(tr "workspace.branches.resolve.choose-side")])
|
||||
[:span {:class (stl/css :td-tag)}
|
||||
(cond
|
||||
;; a decided clash is done (green); an independent edit takes the
|
||||
;; a decided conflict is done (green); an independent edit takes the
|
||||
;; colour of the side it comes from, as the preview cards do
|
||||
(and clash? choice)
|
||||
(and conflicting? choice)
|
||||
[:> tag* {:tone "success"} (tr "workspace.branches.resolve.tag.your-choice")]
|
||||
clash?
|
||||
[:> tag* {:tone "warning"} (tr "workspace.branches.resolve.tag.clash")]
|
||||
conflicting?
|
||||
[:> tag* {:tone "warning"} (tr "workspace.branches.resolve.tag.conflict")]
|
||||
(= source :main)
|
||||
[:> tag* {:tone "purple"} (tr "workspace.branches.resolve.tag.from-main")]
|
||||
:else
|
||||
@ -279,10 +279,10 @@
|
||||
(brc/copies-entry copies conflict
|
||||
(get-in data [:pages-index (:page-id conflict) :objects])))
|
||||
per-attr? (brc/per-attr? conflict)
|
||||
text? (text-clash? conflict)
|
||||
text? (text-conflict? conflict)
|
||||
table-rows (if text? (filterv #(not= :content (:attr %)) rows) rows)
|
||||
clash-n (count (brc/clash-attrs rows))
|
||||
indep-n (- (count rows) clash-n)
|
||||
conflict-n (count (brc/conflicting-attrs rows))
|
||||
indep-n (- (count rows) conflict-n)
|
||||
resolved? (bm/conflict-resolved? conflict res)
|
||||
drops (when resolved? (brc/dropped-values conflict res))
|
||||
main-detail (some-> (:main-at diff-meta) ct/inst ct/timeago)
|
||||
@ -305,7 +305,7 @@
|
||||
[:> tag* {:tone "success"} (tr "workspace.branches.resolve.decided")]
|
||||
[:> tag* {:tone "warning"}
|
||||
(if per-attr?
|
||||
(dm/str (tr "workspace.branches.resolve.tag.clashes" (i18n/c clash-n))
|
||||
(dm/str (tr "workspace.branches.resolve.tag.conflicts" (i18n/c conflict-n))
|
||||
(when (pos? indep-n)
|
||||
(dm/str " - " (tr "workspace.branches.resolve.tag.independent" (i18n/c indep-n)))))
|
||||
(tr (get reason->label (:reason conflict) "workspace.branches.status.conflict")))])]
|
||||
@ -344,7 +344,7 @@
|
||||
|
||||
(when per-attr?
|
||||
(for [{:keys [attr main branch source]} rows
|
||||
:when (and (= :clash source) (not= attr :content))
|
||||
:when (and (= :conflict source) (not= attr :content))
|
||||
:let [choice (brc/attr-choice res attr)]]
|
||||
[:div {:class (stl/css :inline-actions) :key (str attr)}
|
||||
[:> text* {:as "span" :typography t/body-small :class (stl/css :muted)}
|
||||
@ -496,7 +496,7 @@
|
||||
(if (= mode :update) :main->branch :branch->main))))
|
||||
|
||||
;; independent edits are not in doubt: fill them in from the side that
|
||||
;; made them, so only the clashing properties wait for a choice
|
||||
;; made them, so only the conflicting properties wait for a choice
|
||||
(mf/with-effect [conflicts status]
|
||||
(when (and (seq conflicts) (not loading?))
|
||||
(let [seed (into {}
|
||||
|
||||
@ -117,7 +117,7 @@
|
||||
--table-row-bg-color: var(--color-background-secondary);
|
||||
}
|
||||
|
||||
&.is-clash {
|
||||
&.is-conflicting {
|
||||
--table-row-bg-color: var(--color-background-warning);
|
||||
}
|
||||
}
|
||||
|
||||
@ -6,7 +6,7 @@
|
||||
|
||||
(ns frontend-tests.ui.branch-review-common-test
|
||||
"How the decisions dialog splits a conflict: the properties both sides
|
||||
moved clash, the ones a single side moved come filled in from it."
|
||||
moved conflict, the ones a single side moved come filled in from it."
|
||||
(:require
|
||||
[app.common.files.branch-merge :as bm]
|
||||
[app.common.uuid :as uuid]
|
||||
@ -30,20 +30,20 @@
|
||||
[conflict]
|
||||
(into {} (map (juxt :attr :source)) (brc/conflict-attr-rows conflict)))
|
||||
|
||||
(t/deftest only-properties-both-sides-moved-clash
|
||||
(t/is (= {:rx :clash :width :branch :opacity :main} (sources card))))
|
||||
(t/deftest only-properties-both-sides-moved-conflict
|
||||
(t/is (= {:rx :conflict :width :branch :opacity :main} (sources card))))
|
||||
|
||||
(t/deftest without-a-base-every-property-clashes
|
||||
(t/is (= #{:clash} (set (vals (sources (assoc card :base nil :reason :add-add))))))
|
||||
(t/is (= #{:clash} (set (vals (sources (dissoc card :base)))))))
|
||||
(t/deftest without-a-base-every-property-conflicts
|
||||
(t/is (= #{:conflict} (set (vals (sources (assoc card :base nil :reason :add-add))))))
|
||||
(t/is (= #{:conflict} (set (vals (sources (dissoc card :base)))))))
|
||||
|
||||
(t/deftest independent-edits-come-filled-in
|
||||
(let [res (brc/prefill-resolution card)]
|
||||
(t/is (= {:width :branch :opacity :main} res))
|
||||
(t/is (not (bm/conflict-resolved? card res))
|
||||
"the clashing radius still waits for a choice")))
|
||||
"the conflicting radius still waits for a choice")))
|
||||
|
||||
(t/deftest choosing-the-clash-resolves-the-conflict
|
||||
(t/deftest choosing-the-conflicting-property-resolves-the-conflict
|
||||
(let [res (brc/choose-attr card nil :rx :branch)]
|
||||
(t/is (= {:rx :branch :width :branch :opacity :main} res))
|
||||
(t/is (bm/conflict-resolved? card res))
|
||||
@ -51,7 +51,7 @@
|
||||
(brc/dropped-values card res))
|
||||
"keeping the branch radius drops main's, and nothing else")))
|
||||
|
||||
(t/deftest a-conflict-without-a-clash-comes-out-resolved
|
||||
(t/deftest a-conflict-without-conflicting-properties-comes-out-resolved
|
||||
(let [conflict (-> card
|
||||
(assoc-in [:main :rx] 8)
|
||||
(update :changed-attrs dissoc :rx))]
|
||||
|
||||
@ -11339,7 +11339,7 @@ msgid "workspace.branches.resolve.row.loses-work"
|
||||
msgstr "Loses work"
|
||||
|
||||
#: src/app/main/ui/workspace/sidebar/branch_review/conflicts.cljs:156
|
||||
msgid "workspace.branches.resolve.tag.clash"
|
||||
msgid "workspace.branches.resolve.tag.conflict"
|
||||
msgstr "Conflict"
|
||||
|
||||
#: src/app/main/ui/workspace/sidebar/branch_review/conflicts.cljs:160
|
||||
@ -11526,8 +11526,8 @@ msgstr[0] "Merge blocked: 1 decision left"
|
||||
msgstr[1] "Merge blocked: %s decisions left"
|
||||
|
||||
#: src/app/main/ui/workspace/sidebar/branch_review/conflicts.cljs:297
|
||||
msgid "workspace.branches.resolve.tag.clashes"
|
||||
msgid_plural "workspace.branches.resolve.tag.clashes"
|
||||
msgid "workspace.branches.resolve.tag.conflicts"
|
||||
msgid_plural "workspace.branches.resolve.tag.conflicts"
|
||||
msgstr[0] "1 conflict"
|
||||
msgstr[1] "%s conflicts"
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user