From 4cd5bfc1024d296cb82d9b37b8ca674ca07544c3 Mon Sep 17 00:00:00 2001 From: Mega Yu Date: Wed, 11 Feb 2026 14:37:25 +0800 Subject: [PATCH] feat: implement scroll helpers for Cmd+K focus visibility and wheel anchoring to improve ux --- src/main/frontend/components/cmdk/core.cljs | 211 ++++++++++++------ src/main/frontend/components/cmdk/scroll.cljs | 101 +++++++++ .../frontend/components/cmdk/scroll_test.cljs | 101 +++++++++ 3 files changed, 349 insertions(+), 64 deletions(-) create mode 100644 src/main/frontend/components/cmdk/scroll.cljs create mode 100644 src/test/frontend/components/cmdk/scroll_test.cljs diff --git a/src/main/frontend/components/cmdk/core.cljs b/src/main/frontend/components/cmdk/core.cljs index 09e375613b..1b20c6ffde 100644 --- a/src/main/frontend/components/cmdk/core.cljs +++ b/src/main/frontend/components/cmdk/core.cljs @@ -3,6 +3,7 @@ [clojure.string :as string] [frontend.components.block :as block] [frontend.components.cmdk.list-item :as list-item] + [frontend.components.cmdk.scroll :as scroll] [frontend.components.cmdk.state :as cmdk-state] [frontend.components.icon :as icon-component] [frontend.config :as config] @@ -622,25 +623,45 @@ (when-let [action (state->action state)] (handle-action action state event))) -(defn- scroll-into-view-when-invisible +(defn- ensure-focus-visible! [state target] - (let [*container-ref (::scroll-container-ref state) - container-rect (.getBoundingClientRect @*container-ref) - t1 (.-top container-rect) - b1 (.-bottom container-rect) - target-rect (.getBoundingClientRect target) - t2 (.-top target-rect) - b2 (.-bottom target-rect)] - (when-not (<= t1 t2 b2 b1) ; not visible - (.scrollIntoView target - #js {:inline "nearest" - :behavior "smooth"})))) + (let [container @(::scroll-container-ref state) + rect (scroll/focus-row-visible-rect container target)] + (when rect + (let [next-scroll-top (scroll/ensure-focus-visible-scroll-top rect)] + (when (not= next-scroll-top (.-scrollTop container)) + (set! (.-scrollTop container) next-scroll-top)))))) + +(defn- apply-anchored-wheel-scroll! + [state e] + (when (and @(::wheel-focus-anchor? state) + (= :keyboard @(::focus-source state))) + (let [container @(::scroll-container-ref state) + target @(::highlighted-row-el state) + rect (scroll/focus-row-visible-rect container target) + delta-y (some-> e .-deltaY)] + (when (and rect (number? delta-y) (not (zero? delta-y))) + (let [next-scroll-top (scroll/anchored-scroll-top (assoc rect :delta-y delta-y))] + (.preventDefault e) + (when (not= next-scroll-top (.-scrollTop container)) + (set! (.-scrollTop container) next-scroll-top))))))) + +(defn- handle-results-wheel! + [state e] + ;; Wheel input means user wants free scrolling, so exit keyboard-anchored mode first. + (when (= :keyboard @(::focus-source state)) + (reset! (::focus-source state) :mouse) + (reset! (::highlighted-row-el state) nil)) + (apply-anchored-wheel-scroll! state e)) (rum/defcs result-group < rum/reactive [state' state title group visible-items first-item sidebar?] (let [{:keys [show items]} (some-> state ::results deref group) - highlighted-item (or @(::highlighted-item state) first-item) + focus-source @(::focus-source state) + highlighted-item (or @(::highlighted-item state) + (when (= :keyboard focus-source) first-item)) + disable-lazy? @(::disable-lazy? state) *mouse-active? (::mouse-active? state) filter' @(::filter state) can-show-less? (< (get-group-limit group) (count visible-items)) @@ -650,8 +671,14 @@ [:div {:class (if (= title "Create") "border-b border-gray-06 last:border-b-0" "border-b border-gray-06 pb-1 last:border-b-0") - :on-mouse-move #(reset! *mouse-active? true) - :on-mouse-enter #(reset! *mouse-active? true)} + :on-mouse-move (fn [e] + (let [dx (or (.-movementX e) 0) + dy (or (.-movementY e) 0) + real-pointer-move? (or (not (zero? dx)) + (not (zero? dy)))] + (when real-pointer-move? + (when-not @*mouse-active? + (reset! *mouse-active? true)))))} (when-not (= title "Create") [:div {:class "text-xs py-1.5 px-3 flex justify-between items-center gap-2 text-gray-11 bg-gray-02 h-8"} [:div {:class "font-bold text-gray-11 pl-0.5 cursor-pointer select-none" @@ -664,7 +691,7 @@ [:div {:class "pl-1.5 text-gray-12 rounded-full" :style {:font-size "0.7rem"}} (if (<= 100 (count items)) - (str "99+") + "99+" (count items))]) [:div {:class "flex-1"}] @@ -673,7 +700,12 @@ (empty? filter') (not sidebar?)) [:a.text-link.select-node.opacity-50.hover:opacity-90 - {:on-click (if (= show :more) show-less show-more)} + {:on-click (fn [e] + (util/stop e) + (reset! (::focus-source state) :mouse) + (when-let [input-el @(::input-ref state)] + (.focus input-el)) + ((if (= show :more) show-less show-more)))} (if (= show :more) [:div.flex.flex-row.gap-1.items-center "Show less" @@ -709,28 +741,64 @@ (handle-action :default state item) (when-let [on-click (:on-click item)] (on-click e))) - :on-mouse-enter - (fn [_e] - (when (not= item @(::highlighted-item state)) - (reset! (::highlighted-item state) item))) - :on-highlight (fn [ref] - (reset! (::highlighted-group state) group) - (when (and ref (.-current ref) - (not (:mouse-enter-triggered-highlight @(::highlighted-item state)))) - (scroll-into-view-when-invisible state (.-current ref))))) - nil)] - (if (= group :nodes) + :on-mouse-enter + (fn [_e] + (when (and @*mouse-active? + (= :mouse @(::focus-source state))) + (when (not= item @(::highlighted-item state)) + (reset! (::highlighted-item state) item)))) + :component-opts + {:on-mouse-move + (fn [e] + (let [dx (or (.-movementX e) 0) + dy (or (.-movementY e) 0) + real-pointer-move? (or (not (zero? dx)) + (not (zero? dy)))] + (when real-pointer-move? + (when-not @*mouse-active? + (reset! *mouse-active? true)) + (when-not (= :mouse @(::focus-source state)) + (reset! (::focus-source state) :mouse)) + (when (not= item @(::highlighted-item state)) + (reset! (::highlighted-item state) item)))))} + :on-highlight (fn [ref] + (reset! (::highlighted-group state) group) + (when (and ref (.-current ref)) + (let [row-el (.-current ref)] + (reset! (::highlighted-row-el state) row-el) + (when (= :keyboard @(::focus-source state)) + (ensure-focus-visible! state row-el)))))) + nil)] + (if (and (= group :nodes) (not disable-lazy?)) (ui/lazy-visible (fn [] item) {:trigger-once? true}) item)))]])) (defn move-highlight [state n] (let [items (mapcat last (state->results-ordered state (:search/mode @state/state))) + focus-source @(::focus-source state) highlighted-item (some-> state ::highlighted-item deref (dissoc :mouse-enter-triggered-highlight)) - current-item-index (some->> highlighted-item (.indexOf items)) - next-item-index (some-> (or current-item-index 0) (+ n) (mod (count items)))] - (if-let [next-highlighted-item (nth items next-item-index nil)] - (reset! (::highlighted-item state) next-highlighted-item) - (reset! (::highlighted-item state) nil)))) + fallback-highlighted? (and (nil? highlighted-item) + (= :keyboard focus-source) + (seq items)) + current-item-index (cond + highlighted-item (.indexOf items highlighted-item) + fallback-highlighted? 0 + :else nil) + items-count (count items)] + (if (pos? items-count) + (let [base-index (if (some? current-item-index) + current-item-index + (if (pos? n) -1 0)) + next-item-index (mod (+ base-index n) items-count) + next-highlighted-item (nth items next-item-index nil)] + (if next-highlighted-item + (reset! (::highlighted-item state) next-highlighted-item) + (do + (reset! (::highlighted-item state) nil) + (reset! (::highlighted-row-el state) nil)))) + (do + (reset! (::highlighted-item state) nil) + (reset! (::highlighted-row-el state) nil))))) (defn handle-input-change ([state e] (handle-input-change state e (.. e -target -value))) @@ -808,14 +876,24 @@ (shui/dialog-close! :ls-dialog-cmdk) (state/sidebar-add-block! repo input :search)) as-keydown? (if meta? - (show-more) (do + (reset! (::disable-lazy? state) true) + (show-more)) + (do + (reset! (::disable-lazy? state) true) (reset! (::mouse-active? state) false) + (reset! (::focus-source state) :keyboard) + (reset! (::highlighted-row-el state) nil) (move-highlight state 1))) as-keyup? (if meta? - (show-less) (do + (reset! (::disable-lazy? state) true) + (show-less)) + (do + (reset! (::disable-lazy? state) true) (reset! (::mouse-active? state) false) + (reset! (::focus-source state) :keyboard) + (reset! (::highlighted-row-el state) nil) (move-highlight state -1))) (and enter? (not composing?)) (do (handle-action :default state e) @@ -888,7 +966,8 @@ ;; This was moved to a functional component (hooks/use-effect! (fn [] (when (and highlighted-item (= -1 (.indexOf all-items (dissoc highlighted-item :mouse-enter-triggered-highlight)))) - (reset! (::highlighted-item state) nil))) + (reset! (::highlighted-item state) nil) + (reset! (::highlighted-row-el state) nil))) [all-items]) (hooks/use-effect! (fn [] @@ -1072,7 +1151,11 @@ (rum/local false ::meta?) (rum/local nil ::highlighted-group) (rum/local nil ::highlighted-item) + (rum/local nil ::highlighted-row-el) + (rum/local false ::disable-lazy?) + (rum/local :keyboard ::focus-source) (rum/local false ::mouse-active?) + (rum/local true ::wheel-focus-anchor?) (rum/local default-results ::results) (rum/local nil ::scroll-container-ref) (rum/local nil ::input-ref) @@ -1090,36 +1173,36 @@ :class (cond-> "w-full h-full relative flex flex-col justify-start" (not sidebar?) (str " rounded-lg"))} (input-row state all-items opts) - [:div {:class (cond-> "w-full flex-1 overflow-y-auto min-h-[65dvh] max-h-[65dvh]" - (not sidebar?) (str " pb-14")) - :ref #(let [*ref (::scroll-container-ref state)] - (when-not @*ref (reset! *ref %))) - :on-mouse-enter #(reset! (::mouse-active? state) true) - :on-mouse-leave #(reset! (::mouse-active? state) false) - :style {:background "var(--lx-gray-02)" - :scroll-padding-block 32}} + [:div {:class (cond-> "w-full flex-1 overflow-y-auto min-h-[65dvh] max-h-[65dvh]" + (not sidebar?) (str " pb-14")) + :ref #(let [*ref (::scroll-container-ref state)] + (when-not @*ref (reset! *ref %))) + :on-mouse-leave #(reset! (::mouse-active? state) false) + :on-wheel #(handle-results-wheel! state %) + :style {:background "var(--lx-gray-02)" + :scroll-padding-block 32}} - (when group-filter - [:div.flex.flex-col.px-3.py-1.opacity-70.text-sm - (search-only state (string/capitalize (name group-filter)))]) + (when group-filter + [:div.flex.flex-col.px-3.py-1.opacity-70.text-sm + (search-only state (string/capitalize (name group-filter)))]) - (let [items (filter - (fn [[_group-name group-key group-count _group-items]] - (and (not= 0 group-count) - (if-not group-filter true - (or (= group-filter group-key) - (and (= group-filter :nodes) - (= group-key :current-page)) - (and (contains? #{:create} group-filter) - (= group-key :create)))))) - results-ordered)] - (if (seq items) - (for [[group-name group-key _group-count group-items] items] - (let [title (string/capitalize group-name)] - (result-group state title group-key group-items first-item sidebar?))) - [:div.flex.flex-col.p-4.opacity-50 - (when-not (string/blank? @*input) - "No matched results")]))] + (let [items (filter + (fn [[_group-name group-key group-count _group-items]] + (and (not= 0 group-count) + (if-not group-filter true + (or (= group-filter group-key) + (and (= group-filter :nodes) + (= group-key :current-page)) + (and (contains? #{:create} group-filter) + (= group-key :create)))))) + results-ordered)] + (if (seq items) + (for [[group-name group-key _group-count group-items] items] + (let [title (string/capitalize group-name)] + (result-group state title group-key group-items first-item sidebar?))) + [:div.flex.flex-col.p-4.opacity-50 + (when-not (string/blank? @*input) + "No matched results")]))] (when-not sidebar? (hints state))])) (rum/defc cmdk-modal [props] diff --git a/src/main/frontend/components/cmdk/scroll.cljs b/src/main/frontend/components/cmdk/scroll.cljs new file mode 100644 index 0000000000..70bbd03d30 --- /dev/null +++ b/src/main/frontend/components/cmdk/scroll.cljs @@ -0,0 +1,101 @@ +(ns frontend.components.cmdk.scroll + "Scroll geometry helpers for Cmd+K focus visibility and wheel anchoring.") + +(defn focus-row-visible-rect + "Builds normalized focus geometry from `container` and `target` DOM elements. + + Input: + - `container`: scroll container element. + - `target`: focused row element. + + Output: + - `nil` if either input is missing. + - A map with keys: + `:scroll-top`, `:viewport-height`, `:scroll-height`, `:focus-top`, `:focus-height`." + [container target] + (when (and container target) + (let [container-rect (.getBoundingClientRect container) + target-rect (.getBoundingClientRect target) + scroll-top (.-scrollTop container) + focus-top (+ scroll-top (- (.-top target-rect) (.-top container-rect))) + focus-height (.-height target-rect)] + {:scroll-top scroll-top + :viewport-height (.-clientHeight container) + :scroll-height (.-scrollHeight container) + :focus-top focus-top + :focus-height focus-height}))) + +(defn- max-scroll-top + "Returns the maximum valid scroll top for geometry map `data`." + [{:keys [scroll-height viewport-height]}] + (max 0 (- (or scroll-height 0) (or viewport-height 0)))) + +(defn- clamp-scroll-top + "Clamps numeric `scroll-top` into valid range based on geometry map `data`." + [scroll-top data] + (let [max-top (max-scroll-top data)] + (-> scroll-top + (max 0) + (min max-top)))) + +(defn ensure-focus-visible-scroll-top + "Returns a corrected `scroll-top` that keeps the focus row visible in viewport. + + Input map keys: + + | key | description | + |--------------------|-----------------------------------------------| + | `:scroll-top` | Current container scroll top | + | `:viewport-height` | Container viewport height | + | `:focus-top` | Focus row top in container scroll coordinates | + | `:focus-height` | Focus row height | + | `:scroll-height` | Full scroll height used for clamping | + + Output: + - A clamped numeric `scroll-top`." + [{:keys [scroll-top viewport-height focus-top focus-height] :as data}] + (let [focus-bottom (+ (or focus-top 0) (or focus-height 0)) + viewport-bottom (+ (or scroll-top 0) (or viewport-height 0)) + target-top (cond + (nil? focus-top) scroll-top + (< focus-top scroll-top) focus-top + (> focus-bottom viewport-bottom) (- focus-bottom viewport-height) + :else scroll-top)] + (clamp-scroll-top target-top data))) + +(defn anchored-scroll-top + "Returns anchored `scroll-top` after applying wheel delta while keeping focus in view. + + Input map keys: + + | key | description | + |--------------------|-----------------------------------------------| + | `:scroll-top` | Current container scroll top | + | `:delta-y` | Requested wheel delta | + | `:viewport-height` | Container viewport height | + | `:scroll-height` | Full scroll height | + | `:focus-top` | Focus row top in container scroll coordinates | + | `:focus-height` | Focus row height | + + Behavior: + - If focus geometry is missing, behaves like normal clamped scrolling. + - If focus geometry is present, constrains result so focus row remains visible. + + Output: + - A clamped numeric `scroll-top`." + [{:keys [scroll-top delta-y viewport-height focus-top focus-height] :as data}] + (let [base-scroll-top (or scroll-top 0) + desired-scroll-top (clamp-scroll-top (+ base-scroll-top (or delta-y 0)) data)] + (if (or (nil? focus-top) (nil? focus-height) (<= (or viewport-height 0) 0)) + desired-scroll-top + (let [focus-bottom (+ focus-top focus-height) + min-visible-top (inc (- focus-top viewport-height)) + max-visible-top (dec focus-bottom) + min-top (max 0 min-visible-top) + max-top (min (max-scroll-top data) max-visible-top) + constrained-top (if (> min-top max-top) + desired-scroll-top + (-> desired-scroll-top + (max min-top) + (min max-top)))] + (clamp-scroll-top constrained-top data))))) diff --git a/src/test/frontend/components/cmdk/scroll_test.cljs b/src/test/frontend/components/cmdk/scroll_test.cljs new file mode 100644 index 0000000000..9d8f9073ea --- /dev/null +++ b/src/test/frontend/components/cmdk/scroll_test.cljs @@ -0,0 +1,101 @@ +(ns frontend.components.cmdk.scroll-test + (:require [cljs.test :refer [deftest is testing]] + [frontend.components.cmdk.scroll :as scroll])) + +(deftest focus-row-visible-rect-normalization + (testing "returns normalized geometry from container and target DOM data" + (let [container #js {:scrollTop 100 + :clientHeight 240 + :scrollHeight 1200 + :getBoundingClientRect (fn [] #js {:top 40 :height 240})} + target #js {:getBoundingClientRect (fn [] #js {:top 90 :height 30})}] + (is (= {:scroll-top 100 + :viewport-height 240 + :scroll-height 1200 + :focus-top 150 + :focus-height 30} + (scroll/focus-row-visible-rect container target))))) + (testing "returns nil when container or target is missing" + (is (nil? (scroll/focus-row-visible-rect nil #js {}))) + (is (nil? (scroll/focus-row-visible-rect #js {} nil))))) + +(deftest keyboard-navigation-still-moves-focus-and-keeps-visible + (testing "keyboard-driven focus visibility correction keeps focused row in viewport" + (is (= 170 + (scroll/ensure-focus-visible-scroll-top + {:scroll-top 0 + :viewport-height 200 + :scroll-height 2000 + :focus-top 350 + :focus-height 20}))) + (is (= 120 + (scroll/ensure-focus-visible-scroll-top + {:scroll-top 120 + :viewport-height 200 + :scroll-height 2000 + :focus-top 180 + :focus-height 20}))))) + +(deftest wheel-scroll-cannot-pass-focused-row-downward + (testing "downward wheel scrolling is clamped when it would move past the focused row" + (is (= 59 + (scroll/anchored-scroll-top + {:scroll-top 0 + :delta-y 300 + :viewport-height 200 + :scroll-height 2000 + :focus-top 40 + :focus-height 20}))))) + +(deftest wheel-scroll-cannot-pass-focused-row-upward + (testing "upward wheel scrolling is clamped when it would move past the focused row" + (is (= 351 + (scroll/anchored-scroll-top + {:scroll-top 400 + :delta-y -300 + :viewport-height 200 + :scroll-height 2000 + :focus-top 550 + :focus-height 20}))))) + +(deftest no-focus-falls-back-to-normal-wheel + (testing "without focused row anchoring falls back to regular scrolling bounds" + (is (= 340 + (scroll/anchored-scroll-top + {:scroll-top 100 + :delta-y 240 + :viewport-height 200 + :scroll-height 2000}))) + (is (= 0 + (scroll/anchored-scroll-top + {:scroll-top 10 + :delta-y -200 + :viewport-height 200 + :scroll-height 2000}))))) + +(deftest anchored-scroll-top-boundary-branches + (testing "viewport-height <= 0 uses regular clamped scroll result" + (is (= 100 + (scroll/anchored-scroll-top + {:scroll-top 10 + :delta-y 500 + :viewport-height 0 + :scroll-height 100 + :focus-top 10 + :focus-height 10})))) + (testing "min-top > max-top falls back to desired clamped scroll result" + (is (= 5 + (scroll/anchored-scroll-top + {:scroll-top 0 + :delta-y 5 + :viewport-height 200 + :scroll-height 220 + :focus-top 500 + :focus-height 20})))) + (testing "without focus geometry positive overscroll is clamped to max" + (is (= 100 + (scroll/anchored-scroll-top + {:scroll-top 50 + :delta-y 1000 + :viewport-height 200 + :scroll-height 300})))))