From b9ff74164edc6151d305ff315a9c75e924f0b273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Tejero=20Cantero?= Date: Thu, 17 Sep 2026 13:18:42 +0200 Subject: [PATCH] :bug: Give the exit door a button, name what the counters count, and unswap the update resolver Four defects the first by-hand pass found, and one of them was a lie the copy told. The exit door has a button. The limits dialog and three refusal messages told a user that any branch can become an ordinary file, and nothing under `frontend/src` called the command that does it. The kebab now carries the action between Archive and Delete for an open branch, behind a confirmation whose copy says what is kept and what is lost: the content stays, and the file stops being a branch, so it can no longer be compared with main or merged into main. The counters say what they count. Both spans carry a title naming the direction and the unit, which is entity-level changes across shapes, pages, components, colours, typographies, media, and tokens, and stating that saves are not counted. That reading is what made a jump from 7 to 420 look like a regression when the number was right. The update resolver showed each side's values under the other side's heading, and the cause was not a swapped label. The conflict payload is role-keyed rather than document-keyed: `compute-merge*` runs the update direction with the branch as `theirs`, and `three-way-entities` writes `:main` for `theirs`, so an update hands the branch under the key named `:main`. One binding re-keys each conflict's two sides, and the values inside `:changed-attrs` with them, in update mode only. The headings, the chips, and the resolution keywords then all speak documents, the bulk buttons select the side they name, and the merge mode is unchanged by construction rather than by inspection. The modal's fallback diff fetch now also runs in the mode's own direction, so its key space matches. The two bars keep off each other. The banner publishes the room it takes from the bottom edge as one custom property, measured from its live box, and the palette lifts itself by that value. So the offset follows the collapsed and the expanded state, and no literal is repeated in two stylesheets. Verified in the running frontend rather than by inspection: both namespaces reload with no warnings, the re-keying is an involution on a synthetic update conflict with base, id, reason, and label intact, the real row component renders the new menu for open, archived, and merged rows, and the confirm's accept path reaches the backend command with the branch file id. --- .../src/app/main/data/workspace/branches.cljs | 20 +++ .../src/app/main/ui/workspace/palette.scss | 6 +- .../main/ui/workspace/sidebar/branches.cljs | 125 ++++++++++++++++-- .../main/ui/workspace/sidebar/branches.scss | 9 ++ frontend/translations/en.po | 36 +++++ 5 files changed, 184 insertions(+), 12 deletions(-) diff --git a/frontend/src/app/main/data/workspace/branches.cljs b/frontend/src/app/main/data/workspace/branches.cljs index 320c553257..0e771b1415 100644 --- a/frontend/src/app/main/data/workspace/branches.cljs +++ b/frontend/src/app/main/data/workspace/branches.cljs @@ -214,6 +214,26 @@ (fetch-branches)))) (rx/catch (fn [cause] (rx/of (refusal-notification cause (tr "workspace.branches.lifecycle.error"))))))))) +(defn materialize-branch + "Turn a branch into an ordinary file: `file-id` is the branch's FILE, and + the command persists the branch's derived state as the file's own data + payload, drops the op log and clears the branch marker, so every later + read and write takes the ordinary route. The content stays; what goes is + the branch itself, so the file can no longer be compared with main or + merged into it. The branch row is soft-deleted, so refreshing the panel + drops the row and refreshing the context drops the banner when the file + that left the branch is the open one." + [file-id] + (assert (uuid? file-id) "expected valid uuid for `file-id`") + (ptk/reify ::materialize-branch + ptk/WatchEvent + (watch [_ _ _] + (->> (rp/cmd! :materialize-file-branch {:file-id file-id}) + (rx/mapcat (fn [_] (rx/of (ntf/success (tr "workspace.branches.materialize.success")) + (fetch-branches) + (fetch-branch-context)))) + (rx/catch (fn [cause] (rx/of (refusal-notification cause (tr "workspace.branches.lifecycle.error"))))))))) + (defn create-branch "Create a branch from a file. With no `file-id`, branches the currently open file (force-persisting first so the merge base captures the latest diff --git a/frontend/src/app/main/ui/workspace/palette.scss b/frontend/src/app/main/ui/workspace/palette.scss index b61814d2ba..a128401db5 100644 --- a/frontend/src/app/main/ui/workspace/palette.scss +++ b/frontend/src/app/main/ui/workspace/palette.scss @@ -13,7 +13,11 @@ position: absolute; width: 100vw; left: 0; - bottom: 0; + // The branch banner hangs from this same bottom edge and publishes the room + // it takes from it (`branches.cljs::publish-banner-clearance!`), so the bar + // sits above the banner instead of over its controls. With no banner the + // property is unset and the bar keeps its plain bottom anchor. + bottom: var(--branch-banner-clearance, 0px); padding-bottom: deprecated.$s-4; /** Aligns AI Chat button **/ diff --git a/frontend/src/app/main/ui/workspace/sidebar/branches.cljs b/frontend/src/app/main/ui/workspace/sidebar/branches.cljs index 1d0f0e2ef2..380417e518 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/branches.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/branches.cljs @@ -27,6 +27,7 @@ [app.main.ui.ds.notifications.context-notification :refer [context-notification*]] [app.main.ui.ds.product.avatar :refer [avatar*]] [app.main.ui.ds.product.empty-state :refer [empty-state*]] + [app.main.ui.hooks.resize :as r] [app.main.ui.workspace.sidebar.pull-requests :as prui :refer [pr-state-badge*]] [app.util.color :as uc] [app.util.dom :as dom] @@ -588,9 +589,11 @@ [:div {:class (stl/css :info-row)} [:dt {:class (stl/css :info-key)} (tr "workspace.branches.info.changes")] [:dd {:class (stl/css :info-val :info-counts)} - [:span {:class (stl/css :count-ahead)} + [:span {:class (stl/css :count-ahead) + :title (tr "workspace.branches.counts.ahead")} [:> i/icon* {:icon-id i/arrow-up :size "s"}] (dm/str (:ahead branch 0))] - [:span {:class (stl/css :count-behind)} + [:span {:class (stl/css :count-behind) + :title (tr "workspace.branches.counts.behind")} [:> i/icon* {:icon-id i/arrow-down :size "s"}] (dm/str (:behind branch 0))] (when (pos? (:conflicts branch 0)) [:span {:class (stl/css :item-badge :badge-conflict)} @@ -759,7 +762,23 @@ :message (tr "workspace.branches.delete.message" (:name entry)) :accept-label (tr "labels.delete") :accept-style :danger - :on-accept (fn [_] (st/emit! (dwb/delete-branch (:id entry))))}))))] + :on-accept (fn [_] (st/emit! (dwb/delete-branch (:id entry))))})))) + + ;; the exit door: the branch stops being a branch but keeps its + ;; content, and the confirmation says so, because the branch row and + ;; the link to main do not come back + on-materialize + (mf/use-fn + (mf/deps entry) + (fn [event] + (dom/stop-propagation event) + (on-set-menu false) + (st/emit! (modal/show {:type :confirm + :title (tr "workspace.branches.materialize.title") + :message (tr "workspace.branches.materialize.message" (:name entry)) + :accept-label (tr "workspace.branches.materialize.accept") + :accept-style :danger + :on-accept (fn [_] (st/emit! (dwb/materialize-branch (:branch-file-id entry))))}))))] [:li {:class (stl/css-case :branch-entry true :is-archived archived? @@ -801,10 +820,12 @@ [:div {:class (stl/css :branch-entry-aside)} (when (and (not archived?) (not main?)) [:div {:class (stl/css :branch-entry-counts)} - [:span {:class (stl/css :count-ahead)} + [:span {:class (stl/css :count-ahead) + :title (tr "workspace.branches.counts.ahead")} [:> i/icon* {:icon-id i/arrow-up :size "s"}] (dm/str ahead)] - [:span {:class (stl/css :count-behind)} + [:span {:class (stl/css :count-behind) + :title (tr "workspace.branches.counts.behind")} [:> i/icon* {:icon-id i/arrow-down :size "s"}] (dm/str behind)]]) @@ -854,6 +875,11 @@ (tr (if archived? "workspace.branches.menu.restore" "workspace.branches.menu.archive"))] + ;; only an open branch can leave the branch engine: once merged or + ;; archived there is nothing left to turn into an ordinary file + (when-not archived? + [:> dropdown-menu-item* {:class (stl/css :menu-option) :on-click on-materialize} + (tr "workspace.branches.materialize.action")]) [:> dropdown-menu-item* {:class (stl/css :menu-option) :on-click on-delete} (tr "labels.delete")]]]])) @@ -1489,6 +1515,25 @@ (map? res) (get res attr) :else nil)) +(defn- document-sides + "Re-key one conflict from a `:main->branch` payload's role names to the two + documents the resolution modal names, where `:main` holds the branch and + `:branch` holds main (`branch-merge.cljc::compute-merge*` feeds the branch + as `theirs` and main as `ours` in that direction, and + `branch-merge.cljc::three-way-entities` writes `:main` for `theirs`). The + base side, the id, the reason and the label read the same in both + directions, as do the resolution keywords the commands take, so only the + two sides and the values inside `:changed-attrs` move." + [conflict] + (let [swap (fn [sides] {:main (:branch sides) :branch (:main sides)})] + (cond-> (assoc conflict + :main (:branch conflict) + :branch (:main conflict)) + (map? (:changed-attrs conflict)) + (assoc :changed-attrs (into {} + (map (fn [[attr sides]] [attr (swap sides)])) + (:changed-attrs conflict)))))) + (mf/defc conflict-preview-row* "The BASE · MAIN · BRANCH · RESULT preview strip on top of the detail panel. RESULT updates live as per-property choices change." @@ -1557,6 +1602,19 @@ ;; left the dialog showing 0 of 0 with an Apply that could never ;; enable (vcs:tp-update-conflict-frame). Prefer what the command said. conflicts (if (seq conflicts) conflicts (:conflicts diff)) + + ;; In update mode the payload's two sides arrive role-keyed rather than + ;; document-keyed: `branch-merge.cljc::compute-merge*` runs the + ;; `:main->branch` direction with the branch as `theirs` and main as + ;; `ours`, and `branch-merge.cljc::three-way-entities` names those + ;; `:main` and `:branch`. Every heading, chip and button below names + ;; the two documents, so an update hands the branch where the modal + ;; says main. Re-key the payload once, here: the mode-dependent + ;; mapping lives in this one binding. Resolutions stay in document + ;; terms, which is what both commands expect of them. + conflicts (if (= mode :update) + (mapv document-sides conflicts) + conflicts) resolutions (or resolutions {}) total (count conflicts) resolved (count (filterv #(bm/conflict-resolved? % (get resolutions (:id %))) conflicts)) @@ -1586,8 +1644,13 @@ (dwb/merge-branch branch {:resolutions resolutions :keep-branch keep?}))))] - (mf/with-effect [(:id branch)] - (st/emit! (dwb/fetch-branch-diff (:id branch)))) + ;; the fetched diff backs both the fallback conflict set and the side + ;; subtitles, so it must be computed in the direction this modal resolves: + ;; an update shows main's incoming changes, and the key-space of its + ;; payload has to match the one the normalization above assumes + (mf/with-effect [(:id branch) mode] + (st/emit! (dwb/fetch-branch-diff (:id branch) + (if (= mode :update) :main->branch :branch->main)))) [:div {:class (stl/css :compare-overlay)} [:div {:class (stl/css :compare-container)} @@ -1719,6 +1782,23 @@ ;; --- Branch context banner (shown while editing a branch) +(defn- publish-banner-clearance! + "Publish the room the branch banner takes from the bottom edge of the + canvas as `--branch-banner-clearance` on the app root, which the floating + colour/typography palette bar reads to lift itself clear (`palette.scss`): + the banner and the bar hang from the same edge and the bar draws over the + banner's controls. The room is measured from the banner's live box rather + than written down, so it follows the banner in both the collapsed and the + expanded state, where one fixed offset would be wrong in one of the two." + [] + (when-let [root (dom/get-root)] + (when-let [node (dom/get-element "branch-banner")] + (let [room (- (:bottom (dom/get-bounding-rect root)) + (:top (dom/get-bounding-rect node)))] + (dom/set-css-property! root + "--branch-banner-clearance" + (dm/str (max 0 room) "px")))))) + (mf/defc branch-context-banner* [{:keys [file-id]}] (let [ctx (mf/deref branch-context) @@ -1772,7 +1852,14 @@ on-open-review (mf/use-fn (mf/deps open-pr) - #(st/emit! (dwpr/open-pull-request open-pr)))] + #(st/emit! (dwpr/open-pull-request open-pr))) + + ;; publishes the room the banner takes from the bottom of the canvas + ;; whenever its box changes size, which is what keeps the floating + ;; colour/typography palette bar clear of it in both the collapsed and + ;; the expanded state (see `publish-banner-clearance!`) + banner-ref (r/use-resize-observer + (mf/use-fn (fn [_ _] (publish-banner-clearance!))))] (mf/with-effect [file-id] (when (contains? cf/flags :branching) @@ -1795,8 +1882,22 @@ (st/emit! (dwb/fetch-branch-context)))))] (fn [] (events/unlistenByKey key)))) + ;; it is cleared when there is no banner to clear, so a file that is not a + ;; branch leaves the palette bar on its plain bottom anchor + (mf/with-effect [ctx] + (if (some? ctx) + (publish-banner-clearance!) + (dom/unset-css-property! (dom/get-root) "--branch-banner-clearance"))) + + ;; the same goes when the banner goes away with the component, so nothing + ;; is left behind for the palette bar to clear later + (mf/with-effect [] + (fn [] (dom/unset-css-property! (dom/get-root) "--branch-banner-clearance"))) + (when ctx - [:div {:class (stl/css-case :branch-banner true + [:div {:id "branch-banner" + :ref banner-ref + :class (stl/css-case :branch-banner true :branch-banner-merged merged? :is-collapsed collapsed?)} [:button {:class (stl/css :branch-banner-handle) @@ -1828,9 +1929,11 @@ (let [conflicts (or (:conflicts ctx) 0)] [:div {:class (stl/css :branch-banner-actions)} [:span {:class (stl/css :branch-banner-counts)} - [:span {:class (stl/css :count-ahead)} + [:span {:class (stl/css :count-ahead) + :title (tr "workspace.branches.counts.ahead")} [:> i/icon* {:icon-id i/arrow-up :size "s"}] (dm/str (:ahead ctx))] - [:span {:class (stl/css :count-behind)} + [:span {:class (stl/css :count-behind) + :title (tr "workspace.branches.counts.behind")} [:> i/icon* {:icon-id i/arrow-down :size "s"}] (dm/str (:behind ctx))]] (when (pos? conflicts) diff --git a/frontend/src/app/main/ui/workspace/sidebar/branches.scss b/frontend/src/app/main/ui/workspace/sidebar/branches.scss index c11e539562..95b0c55883 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/branches.scss +++ b/frontend/src/app/main/ui/workspace/sidebar/branches.scss @@ -1332,6 +1332,15 @@ border: deprecated.$s-2 solid var(--panel-border-color); background-color: var(--color-background-primary); + // The floating colour and typography palette bar hangs from the same bottom + // edge of the canvas, and it draws over the banner's controls. The banner + // publishes the room it takes from that edge, measured from its own box, as + // `--branch-banner-clearance` on the app root + // (`branches.cljs::publish-banner-clearance!`), and the palette bar lifts + // itself by that value (`workspace/palette.scss`). One measured value, so + // the bar follows the banner's collapsed and expanded states and the two + // cannot drift apart. + // the kebab dropdown opens upwards, past the banner box: it must not // be clipped (the collapsed state hides its content via block-size + // visibility) and it must beat the viewport controls, which are diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 003509a586..586de5e81a 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -6682,6 +6682,20 @@ msgstr "Use this" msgid "workspace.branches.conflicts.whole-hint" msgstr "This element was added or removed on one side. Choose which version to keep." +#: src/app/main/ui/workspace/sidebar/branches.cljs:593, src/app/main/ui/workspace/sidebar/branches.cljs:824, src/app/main/ui/workspace/sidebar/branches.cljs:1933 +msgid "workspace.branches.counts.ahead" +msgstr "" +"Changes this branch has and main does not. Counts entity-level changes " +"(shapes, pages, components, colours, typographies, media, and tokens), not " +"saves." + +#: src/app/main/ui/workspace/sidebar/branches.cljs:596, src/app/main/ui/workspace/sidebar/branches.cljs:828, src/app/main/ui/workspace/sidebar/branches.cljs:1936 +msgid "workspace.branches.counts.behind" +msgstr "" +"Changes main has and this branch does not. Counts entity-level changes " +"(shapes, pages, components, colours, typographies, media, and tokens), not " +"saves." + #: src/app/main/ui/workspace/sidebar/branches.cljs:299 msgid "workspace.branches.create.derive-from" msgstr "Derive from" @@ -6953,6 +6967,28 @@ msgstr "main has new changes — you can update this branch" msgid "workspace.branches.main-subtitle" msgstr "Default branch" +#: src/app/main/ui/workspace/sidebar/branches.cljs:779 +msgid "workspace.branches.materialize.accept" +msgstr "Turn it into a file" + +#: src/app/main/ui/workspace/sidebar/branches.cljs:882 +msgid "workspace.branches.materialize.action" +msgstr "Turn into an ordinary file" + +#: src/app/main/ui/workspace/sidebar/branches.cljs:778 +msgid "workspace.branches.materialize.message" +msgstr "" +"Branch \"%s\" keeps its content. The file stops being a branch, so it can no " +"longer be compared with main or merged into main." + +#: src/app/main/data/workspace/branches.cljs:232 +msgid "workspace.branches.materialize.success" +msgstr "The branch is now an ordinary file" + +#: src/app/main/ui/workspace/sidebar/branches.cljs:777 +msgid "workspace.branches.materialize.title" +msgstr "Turn this branch into an ordinary file" + #: src/app/main/ui/workspace/main_menu.cljs:736 msgid "workspace.branches.menu" msgstr "Branches"