diff --git a/docs/creating-in.md b/docs/creating-in.md index ad03dca..8d5a8dd 100644 --- a/docs/creating-in.md +++ b/docs/creating-in.md @@ -72,6 +72,12 @@ Once resolved, every creation command follows the destination kind. - Selecting an existing cel changes the destination from the lane to the symbol placed by that cel; subsequent symbols and shapes become children there. +Double-clicking a lane creates an empty cel at the playhead; beginning a drawing +creates a drawing cel there. These are the same lane-creation operation with +different payloads. The pointer chooses the lane, never a second creation time. +The resulting cel is selected, so it immediately becomes the preferred target: +drawing again enters that cel's symbol instead of replacing it. + Thus no separate "new cel" versus "add inside" mode is needed. Selecting the lane header says new cel; selecting a cel says add inside. diff --git a/frontend/src/arthur/domain/creation.cljs b/frontend/src/arthur/domain/creation.cljs index ec1f3ca..49660b0 100644 --- a/frontend/src/arthur/domain/creation.cljs +++ b/frontend/src/arthur/domain/creation.cljs @@ -6,20 +6,39 @@ is actually available. See `docs/creating-in.md`." (:require [arthur.domain.clip :as clip] [arthur.domain.nest :as nest] + [arthur.domain.node :as node] [arthur.domain.symbol :as symbol])) +(defn- path-node + "The node at the end of an occurrence `path`, walked from `open`. + + Selection also carries an owner sid and node id for commands that edit the + node directly. Those are deliberately not used here: the path is the address + of the row as seen from the open symbol, and is the only part that describes + every enclosing occurrence at arbitrary depth." + [document open path] + (loop [sid open [id & more] (seq path) found nil] + (if-not id + found + (when-let [n (get-in document [:symbols sid :nodes id])] + (if (seq more) + (when-let [inner (and (= :instance (:kind n)) (node/source n))] + (recur inner more n)) + n))))) + (defn preferred-path "The container path structurally implied by `selection`. Selecting an instance means inside it. Selecting any other node means its containing symbol. An empty or non-node selection means the open symbol." - [document selection] - (let [[kind sid id selected-path] selection + [document open selection] + (let [[kind _sid id selected-path] selection path (when (= :node kind) - (vec (or (seq selected-path) (when id [id]))))] + (vec (or (seq selected-path) (when id [id])))) + selected (path-node document open path)] (cond (empty? path) [] - (= :instance (get-in document [:symbols sid :nodes id :kind])) path + (= :instance (:kind selected)) path :else (vec (butlast path))))) (defn target @@ -33,7 +52,7 @@ Returns `{:kind :lane|:symbol :sid :path :frame :matrix :time}`." [document store open selection frame] - (let [preferred (preferred-path document selection)] + (let [preferred (preferred-path document open selection)] (some (fn [path] (when-let [inside (nest/inside document store open path frame)] (when-let [sid (:sid inside)] diff --git a/frontend/src/arthur/events/ui.cljs b/frontend/src/arthur/events/ui.cljs index a48b1fe..b61f2d2 100644 --- a/frontend/src/arthur/events/ui.cljs +++ b/frontend/src/arthur/events/ui.cljs @@ -345,9 +345,16 @@ (rf/reg-event-db - ::toggle-row - (fn [db [_ path]] - (update-in db [:ui :expanded] #(if (contains? % path) (disj % path) (conj % path))))) + ::set-row-expanded + (fn [db [_ path expanded?]] + ;; The disclosure control says what the next state is instead of asking us + ;; to invert whatever happens to be in app-db by the time its event runs. + ;; A structural drop may reveal this path between pointer-down and click; + ;; blindly toggling then immediately closed the row whose triangle was still + ;; painted as closed. + (update-in db [:ui :expanded] + (fn [paths] + ((if expanded? conj disj) (set paths) path))))) (rf/reg-event-db ::solo @@ -800,8 +807,11 @@ ;; One gesture and one creation path for every lane. The destination decides ;; the kind: an ordinary lane gets a blank symbol; the synthetic palette row ;; gets a blank palette symbol whose placement starts by inheriting. - (fn [db [_ frame target]] + (fn [db [_ _pointer-frame target]] (let [{document :clip st :store} (store/entry (:clip/current db)) + ;; Creation always happens at the playhead. The double-click only + ;; names the lane; it is not a second, pointer-based time cursor. + frame (editing-frame db document) palette? (= :arthur.ui.timeline/palette-track (first target)) root-sid (second target) root (when palette? (clip/symbol document root-sid)) diff --git a/frontend/src/arthur/ui/timeline.cljs b/frontend/src/arthur/ui/timeline.cljs index 8c1a730..3772771 100644 --- a/frontend/src/arthur/ui/timeline.cljs +++ b/frontend/src/arthur/ui/timeline.cljs @@ -656,11 +656,14 @@ :else [::ui/group [from path]])))))})) [:button.tl-twist {:disabled (not expandable?) + ;; This button lives inside a draggable row label. Do not let a tiny + ;; pointer movement turn disclosure into a native row drag (whose click + ;; is then suppressed by the browser). + :draggable false + :on-pointer-down (fn [^js e] (.stopPropagation e)) :on-click (fn [^js e] - (println "heyyy") (.stopPropagation e) - (rf/dispatch [::ui/toggle-row path])) - :class (println expandable?)} + (rf/dispatch [::ui/set-row-expanded path (not expanded?)]))} (when expandable? (if expanded? "▾" "▸"))] ;; A LANE IS NOT AN INSTANCE WEARING A DIFFERENT HAT, to read. Its one row ;; holds blocks that follow one another in time, where every other row diff --git a/frontend/test/arthur/events/lane_test.cljs b/frontend/test/arthur/events/lane_test.cljs index 2830ee4..b4b3783 100644 --- a/frontend/test/arthur/events/lane_test.cljs +++ b/frontend/test/arthur/events/lane_test.cljs @@ -72,26 +72,30 @@ (vals (get-in saved [:symbols :main :nodes])))) "claiming the frame leaves no overlapping cel")))) -(deftest double-click-creation-uses-the-lane-and-pointer-frame +(deftest double-click-creation-uses-the-lane-and-playhead (let [doc (fixture/document) id (store/install! {:clip doc :store {}} "double-click-new-symbol")] (reset! rf-db/app-db {:clip/current id :paint/revision 0 - :ui {:open :main} :playback {:frame 0}}) + :ui {:open :main} :playback {:frame 5}}) (rf/dispatch-sync [::ui/new-symbol-at 5 [:node :main nil []]]) (let [saved (:clip (store/entry id)) [_ sid instance-id] (get-in @rf-db/app-db [:ui :selection]) + selection (get-in @rf-db/app-db [:ui :selection]) instance (get-in saved [:symbols sid :nodes instance-id])] (is (= :main sid)) (is (= [5 6] (node/placed-span instance))) (is (= 1 (clip/frames saved (node/source instance)))) - (is (zero? (get-in @rf-db/app-db [:playback :frame])) - "the pointer frame, not the playhead, chooses the new cel's time")))) + (is (= (node/source instance) + (:sid (creation/target saved {} :main selection 5))) + "the new cel can immediately be selected as the creation target") + (is (= 5 (get-in @rf-db/app-db [:playback :frame])) + "the playhead chooses the new cel's time")))) (deftest the-same-double-click-command-creates-a-palette-symbol-on-the-palette-row (let [doc (clip/blank) id (store/install! {:clip doc :store {}} "double-click-palette-symbol")] (reset! rf-db/app-db {:clip/current id :paint/revision 0 - :ui {:open :main} :playback {:frame 0}}) + :ui {:open :main} :playback {:frame 5}}) (rf/dispatch-sync [::ui/new-symbol-at 5 [:arthur.ui.timeline/palette-track :main]]) (let [saved (:clip (store/entry id)) @@ -213,6 +217,36 @@ (at 25)) "a lane behind an inactive parent occurrence is not valid"))) +(deftest creation-uses-the-occurrence-path-at-arbitrary-depth + (let [instance (fn [id source span] + {:id id :kind :instance :z "a" :span span + :time {:mode :map :at 0 :rate 1} + :source {:symbol source} + :playback {:in 0 :speed 1 :end :stop}}) + doc {:fps 24 :width 20 :height 20 + :symbols + {:main {:id :main :frames 20 + :nodes {:outer (instance :outer :ordinary [0 20])}} + :ordinary {:id :ordinary :frames 20 + :nodes {:lane (instance :lane :symbol-7 [0 20])}} + :symbol-7 {:id :symbol-7 :name "symbol-7" :display :lane :frames 20 + :nodes {:cel (instance :cel :symbol-8 [4 10])}} + :symbol-8 {:id :symbol-8 :name "symbol-8" :frames 6 :nodes {}}}} + lane-selection [:node :ordinary :lane [:outer :lane]] + ;; The path, rather than these redundant owner fields, is authoritative + ;; for creation. A row address may name the occurrence from an outer + ;; view; resolution must still enter the terminal cel. + cel-selection [:node :ordinary :lane [:outer :lane :cel]] + pick #(select-keys (creation/target doc nil :main % 5) + [:kind :sid :path :frame])] + (is (= {:kind :lane :sid :symbol-7 :path [:outer :lane] :frame 5} + (pick lane-selection)) + "the nested lane header remains the lane insertion surface") + (is (= {:kind :symbol :sid :symbol-8 + :path [:outer :lane :cel] :frame 5} + (pick cel-selection)) + "the cel enters its symbol no matter how deeply the lane is nested"))) + (deftest creation-walks-out-of-an-instance-past-its-window (let [doc {:fps 24 :width 20 :height 20 :symbols @@ -360,6 +394,16 @@ (is (= #{[:already-open]} (get-in after [:ui :expanded])) "disclosure is changed only by the twist control"))) +(deftest disclosure-sets-the-state-painted-by-the-button + ;; A move can reveal a destination after the old, closed button has already + ;; received pointer-down. Its eventual click must keep the row open instead + ;; of toggling the newer state closed again. + (reset! rf-db/app-db {:ui {:expanded #{[:destination]}}}) + (rf/dispatch-sync [::ui/set-row-expanded [:destination] true]) + (is (= #{[:destination]} (get-in @rf-db/app-db [:ui :expanded]))) + (rf/dispatch-sync [::ui/set-row-expanded [:destination] false]) + (is (empty? (get-in @rf-db/app-db [:ui :expanded])))) + (deftest finishing-a-polygon-opens-no-rows ;; Expansion is the twist triangle's business. Finishing a shape used to open ;; every row down to it, which inside a lane meant tearing its one row into a