From 7adac6df40f853de5a2fb2d8e796c20d09717a6a Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Mon, 23 Mar 2026 16:06:23 +0100 Subject: [PATCH] :bug: Fix review comments (#8708) * :bug: Fix focus option only on arrowdown not at open * :bug: Fix focus on input when visible focus should be on options * :recycle: Improve nativation, adding tab control and moving throught options is now cyclic * :sparkles: Add selected option when inside cursor is inside option * :bug: Dropdown is positioned nex to the input alwais --- .../ds/controls/shared/options_dropdown.cljs | 23 +++++----- .../ui/ds/controls/utilities/input_field.cljs | 6 ++- .../ui/ds/controls/utilities/input_field.scss | 5 +++ .../management/forms/controls/combobox.cljs | 29 ++++++++++-- .../forms/controls/combobox_navigation.cljs | 45 +++++++++++-------- .../forms/controls/floating_dropdown.cljs | 44 +++++++++++------- .../forms/controls/token_parsing.cljs | 12 +++++ .../management/forms/controls/utils.cljs | 2 +- 8 files changed, 116 insertions(+), 50 deletions(-) diff --git a/frontend/src/app/main/ui/ds/controls/shared/options_dropdown.cljs b/frontend/src/app/main/ui/ds/controls/shared/options_dropdown.cljs index 7d566f7d63..60de5830a0 100644 --- a/frontend/src/app/main/ui/ds/controls/shared/options_dropdown.cljs +++ b/frontend/src/app/main/ui/ds/controls/shared/options_dropdown.cljs @@ -33,17 +33,7 @@ [:label {:optional true} :string] [:aria-label {:optional true} :string]]) -(def ^:private schema:options-dropdown - [:map - [:ref {:optional true} fn?] - [:class {:optional true} :string] - [:wrapper-ref {:optional true} :any] - [:on-click fn?] - [:options [:vector schema:option]] - [:selected {:optional true} :any] - [:focused {:optional true} :any] - [:empty-to-end {:optional true} [:maybe :boolean]] - [:align {:optional true} [:maybe [:enum :left :right]]]]) + (def ^:private xf:filter-blank-id @@ -104,6 +94,17 @@ :dimmed (true? (:dimmed option)) :on-click on-click}])))) +(def ^:private schema:options-dropdown + [:map + [:ref {:optional true} fn?] + [:class {:optional true} :string] + [:wrapper-ref {:optional true} :any] + [:on-click fn?] + [:options [:vector schema:option]] + [:selected {:optional true} :any] + [:focused {:optional true} :any] + [:empty-to-end {:optional true} [:maybe :boolean]] + [:align {:optional true} [:maybe [:enum :left :right]]]]) (mf/defc options-dropdown* {::mf/schema schema:options-dropdown} diff --git a/frontend/src/app/main/ui/ds/controls/utilities/input_field.cljs b/frontend/src/app/main/ui/ds/controls/utilities/input_field.cljs index 58a3202c80..c0ee6f245e 100644 --- a/frontend/src/app/main/ui/ds/controls/utilities/input_field.cljs +++ b/frontend/src/app/main/ui/ds/controls/utilities/input_field.cljs @@ -37,6 +37,8 @@ has-hint hint-type max-length variant slot-start slot-end + data-option-focused + input-wrapper-ref aria-label] :rest props} ref] (let [input-ref (mf/use-ref) type (d/nilv type "text") @@ -74,7 +76,9 @@ (dom/select-node input-node) (dom/focus! input-node))))] - [:div {:class [inside-class class]} + [:div {:class [inside-class class] + :ref input-wrapper-ref + :data-option-focused data-option-focused} (when (some? slot-start) slot-start) (when (some? icon) diff --git a/frontend/src/app/main/ui/ds/controls/utilities/input_field.scss b/frontend/src/app/main/ui/ds/controls/utilities/input_field.scss index 80068f0c2b..20533664ef 100644 --- a/frontend/src/app/main/ui/ds/controls/utilities/input_field.scss +++ b/frontend/src/app/main/ui/ds/controls/utilities/input_field.scss @@ -41,6 +41,11 @@ --input-bg-color: var(--color-background-primary); --input-outline-color: var(--color-background-quaternary); } + + &[data-option-focused="true"]:has(*:focus-visible) { + --input-bg-color: var(--color-background-tertiary); + --input-outline-color: none; + } } .variant-dense, diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox.cljs index d53b8d0d60..f1ef466929 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox.cljs @@ -84,11 +84,15 @@ filter-term* (mf/use-state "") filter-term (deref filter-term*) + selected-id* (mf/use-state nil) + selected-id (deref selected-id*) + options-ref (mf/use-ref nil) dropdown-ref (mf/use-ref nil) internal-ref (mf/use-ref nil) nodes-ref (mf/use-ref nil) wrapper-ref (mf/use-ref nil) + input-wrapper-ref (mf/use-ref nil) icon-button-ref (mf/use-ref nil) ref (or ref internal-ref) @@ -117,12 +121,28 @@ state (obj/set! state id node)] (mf/set-ref-val! nodes-ref state)))) + get-selected-id + (mf/use-fn + (mf/deps dropdown-options) + (fn [] + (let [input-node (mf/ref-val ref) + value (dom/get-input-value input-node) + cursor (dom/selection-start input-node) + token-name (tp/token-at-cursor value cursor) + options (if (delay? dropdown-options) @dropdown-options dropdown-options)] + (when token-name + (->> options + (filter #(= (:name %) token-name)) + first + :id))))) + toggle-dropdown (mf/use-fn (mf/deps is-open) (fn [event] (dom/prevent-default event) (swap! is-open* not) + (reset! selected-id* (get-selected-id)) (let [input-node (mf/ref-val ref)] (dom/focus! input-node)))) @@ -157,7 +177,8 @@ :options dropdown-options :toggle-dropdown toggle-dropdown :is-open* is-open* - :on-enter on-option-enter}) + :on-enter on-option-enter + :get-selected-id get-selected-id}) on-change (mf/use-fn @@ -216,11 +237,13 @@ :hint-message (:message hint) :on-key-down on-key-down :hint-type (:type hint) + :input-wrapper-ref input-wrapper-ref :ref ref :role "combobox" :aria-activedescendant focused-id :aria-controls listbox-id :aria-expanded is-open + :data-option-focused (boolean focused-id) :slot-end (when (some? @filtered-tokens-by-type) (mf/html @@ -241,7 +264,7 @@ props) - {:keys [style ready?]} (use-floating-dropdown is-open wrapper-ref dropdown-ref)] + {:keys [style ready?]} (use-floating-dropdown is-open input-wrapper-ref wrapper-ref dropdown-ref)] (mf/with-effect [resolve-stream tokens token name token-name] (let [subs (->> resolve-stream @@ -300,7 +323,7 @@ :id listbox-id :options options :focused focused-id - :selected nil + :selected selected-id :align :right :empty-to-end empty-to-end :wrapper-ref dropdown-ref diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox_navigation.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox_navigation.cljs index b8be6dad81..3508833797 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox_navigation.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox_navigation.cljs @@ -26,14 +26,13 @@ [focusables focused-id direction] (let [ids (vec (map :id focusables)) idx (.indexOf (clj->js ids) focused-id) - idx (if (= idx -1) -1 idx) - next-idx (case direction - :down (min (dec (count ids)) (inc idx)) - :up (max 0 (dec (if (= idx -1) 0 idx))))] - (nth ids next-idx nil))) + count (count ids)] + (case direction + :down (nth ids (mod (inc idx) count) nil) + :up (nth ids (mod (if (= idx -1) 0 (dec idx)) count) nil)))) (defn use-navigation - [{:keys [is-open options nodes-ref is-open* toggle-dropdown on-enter]}] + [{:keys [is-open options nodes-ref is-open* toggle-dropdown on-enter get-selected-id]}] (let [focused-id* (mf/use-state nil) focused-id (deref focused-id*) @@ -46,6 +45,7 @@ down? (kbd/down-arrow? event) enter? (kbd/enter? event) esc? (kbd/esc? event) + tab? (kbd/tab? event) open-dropdown (kbd/is-key? event "{") close-dropdown (kbd/is-key? event "}") options (if (delay? options) @options options)] @@ -56,18 +56,21 @@ (dom/prevent-default event) (let [focusables (focusable-options options)] (cond + ;; Dropdown open: move focus to next option is-open (when (seq focusables) (let [next-id (next-focus-id focusables focused-id :down)] (reset! focused-id* next-id))) + ;; Dropdown closed with options: open and focus first (seq focusables) (do (toggle-dropdown event) + (when get-selected-id + (get-selected-id)) (reset! focused-id* (first-focusable-id focusables))) - :else - nil))) + :else nil))) up? (when is-open @@ -77,7 +80,9 @@ (reset! focused-id* next-id))) open-dropdown - (reset! is-open* true) + (do + (reset! is-open* true) + (reset! focused-id* nil)) close-dropdown (reset! is-open* false) @@ -89,21 +94,23 @@ (dom/prevent-default event) (when (some #(= (:id %) focused-id) focusables) (on-enter focused-id))))) + esc? - (do + (when is-open (dom/prevent-default event) + (dom/stop-propagation event) (reset! is-open* false)) + + tab? + (when is-open + (reset! is-open* false) + (reset! focused-id* nil)) + :else nil))))] - ;; Initial focus on first option - (mf/with-effect [is-open options] - (when is-open - (let [opts (if (delay? options) @options options) - focusables (focusable-options opts) - ids (set (map :id focusables))] - (when (and (seq focusables) - (not (contains? ids focused-id))) - (reset! focused-id* (:id (first focusables))))))) + (mf/with-effect [is-open] + (when (not is-open) + (reset! focused-id* nil))) ;; auto scroll when key down (mf/with-effect [focused-id nodes-ref] diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/floating_dropdown.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/floating_dropdown.cljs index 739ca0628c..d7cf90a3f3 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/floating_dropdown.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/floating_dropdown.cljs @@ -9,7 +9,7 @@ [app.util.dom :as dom] [rumext.v2 :as mf])) -(defn use-floating-dropdown [is-open wrapper-ref dropdown-ref] +(defn use-floating-dropdown [is-open input-wrapper-ref outer-wrapper-ref dropdown-ref] (let [position* (mf/use-state nil) position (deref position*) ready* (mf/use-state false) @@ -32,7 +32,7 @@ (> dropdown-height space-below)) position (if open-up? - {:bottom (str (- windows-height (:top combobox-rect) 12) "px") + {:bottom (str (- windows-height (:top combobox-rect) -8) "px") :left (str (:left combobox-rect) "px") :width (str (:width combobox-rect) "px") :placement :top} @@ -44,27 +44,41 @@ (reset! ready* true) (reset! position* position)))] - (mf/with-effect [is-open dropdown-ref wrapper-ref] + (mf/with-effect [is-open dropdown-ref input-wrapper-ref outer-wrapper-ref] (when is-open - (let [handler (fn [event] - (let [dropdown-node (mf/ref-val dropdown-ref) - target (dom/get-target event)] - (when (or (nil? dropdown-node) - (not (instance? js/Node target)) - (not (.contains dropdown-node target))) - (js/requestAnimationFrame - (fn [] - (let [wrapper-node (mf/ref-val wrapper-ref)] - (reset! ready* true) - (calculate-position wrapper-node)))))))] + (let [recalculate + (fn [] + (js/requestAnimationFrame + (fn [] + (let [input-node (mf/ref-val input-wrapper-ref)] + (calculate-position input-node))))) + + handler + (fn [event] + (let [dropdown-node (mf/ref-val dropdown-ref) + target (dom/get-target event)] + (when (or (nil? dropdown-node) + (not (instance? js/Node target)) + (not (.contains dropdown-node target))) + (recalculate)))) + + resize-observer (js/ResizeObserver. (fn [_] (recalculate))) + outer-node (mf/ref-val outer-wrapper-ref) + dropdown-node (mf/ref-val dropdown-ref)] + (handler nil) (.addEventListener js/window "resize" handler) (.addEventListener js/window "scroll" handler true) + (when outer-node + (.observe resize-observer outer-node)) + (when dropdown-node + (.observe resize-observer dropdown-node)) (fn [] (.removeEventListener js/window "resize" handler) - (.removeEventListener js/window "scroll" handler true))))) + (.removeEventListener js/window "scroll" handler true) + (.disconnect resize-observer))))) {:style position :ready? ready diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/token_parsing.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/token_parsing.cljs index 2308bef9c7..1c317ff3e3 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/token_parsing.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/token_parsing.cljs @@ -22,6 +22,18 @@ :end (or (str/index-of value "}" last-open) cursor) :partial (subs text-before (inc last-open))}))) +(defn token-at-cursor + "Returns the full token name at the cursor position if cursor is + inside a complete {token-name} reference, nil otherwise." + [value cursor] + (let [last-open (str/last-index-of (subs value 0 cursor) "{") + last-close (str/index-of value "}" (or last-open 0))] + (when (and last-open last-close (> last-close last-open)) + (let [token-name (subs value (inc last-open) last-close)] + (when (and (seq token-name) + (not (str/includes? token-name " "))) + token-name))))) + (defn active-token [value input-node] (let [cursor (dom/selection-start input-node)] diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/utils.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/utils.cljs index e75989cdc8..8127dc2f1e 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/utils.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/utils.cljs @@ -9,7 +9,7 @@ [token] {:id (str (get token :id)) :type :token - :resolved-value (get token :resolved-value) + :resolved-value (get token :value) :name (get token :name)}) (defn- generate-dropdown-options