🐛 Fix review comments (#8708)

* 🐛 Fix focus option only on arrowdown not at open

* 🐛 Fix focus on input when visible focus should be on options

* ♻️ Improve nativation, adding tab control and moving throught options is now cyclic

*  Add selected option when inside cursor is inside option

* 🐛 Dropdown is positioned nex to the input alwais
This commit is contained in:
Eva Marco 2026-03-23 16:06:23 +01:00 committed by GitHub
parent 11ed09f431
commit 7adac6df40
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 116 additions and 50 deletions

View File

@ -33,17 +33,7 @@
[:label {:optional true} :string] [:label {:optional true} :string]
[:aria-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 (def ^:private
xf:filter-blank-id xf:filter-blank-id
@ -104,6 +94,17 @@
:dimmed (true? (:dimmed option)) :dimmed (true? (:dimmed option))
:on-click on-click}])))) :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/defc options-dropdown*
{::mf/schema schema:options-dropdown} {::mf/schema schema:options-dropdown}

View File

@ -37,6 +37,8 @@
has-hint hint-type has-hint hint-type
max-length variant max-length variant
slot-start slot-end slot-start slot-end
data-option-focused
input-wrapper-ref
aria-label] :rest props} ref] aria-label] :rest props} ref]
(let [input-ref (mf/use-ref) (let [input-ref (mf/use-ref)
type (d/nilv type "text") type (d/nilv type "text")
@ -74,7 +76,9 @@
(dom/select-node input-node) (dom/select-node input-node)
(dom/focus! 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) (when (some? slot-start)
slot-start) slot-start)
(when (some? icon) (when (some? icon)

View File

@ -41,6 +41,11 @@
--input-bg-color: var(--color-background-primary); --input-bg-color: var(--color-background-primary);
--input-outline-color: var(--color-background-quaternary); --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, .variant-dense,

View File

@ -84,11 +84,15 @@
filter-term* (mf/use-state "") filter-term* (mf/use-state "")
filter-term (deref filter-term*) filter-term (deref filter-term*)
selected-id* (mf/use-state nil)
selected-id (deref selected-id*)
options-ref (mf/use-ref nil) options-ref (mf/use-ref nil)
dropdown-ref (mf/use-ref nil) dropdown-ref (mf/use-ref nil)
internal-ref (mf/use-ref nil) internal-ref (mf/use-ref nil)
nodes-ref (mf/use-ref nil) nodes-ref (mf/use-ref nil)
wrapper-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) icon-button-ref (mf/use-ref nil)
ref (or ref internal-ref) ref (or ref internal-ref)
@ -117,12 +121,28 @@
state (obj/set! state id node)] state (obj/set! state id node)]
(mf/set-ref-val! nodes-ref state)))) (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 toggle-dropdown
(mf/use-fn (mf/use-fn
(mf/deps is-open) (mf/deps is-open)
(fn [event] (fn [event]
(dom/prevent-default event) (dom/prevent-default event)
(swap! is-open* not) (swap! is-open* not)
(reset! selected-id* (get-selected-id))
(let [input-node (mf/ref-val ref)] (let [input-node (mf/ref-val ref)]
(dom/focus! input-node)))) (dom/focus! input-node))))
@ -157,7 +177,8 @@
:options dropdown-options :options dropdown-options
:toggle-dropdown toggle-dropdown :toggle-dropdown toggle-dropdown
:is-open* is-open* :is-open* is-open*
:on-enter on-option-enter}) :on-enter on-option-enter
:get-selected-id get-selected-id})
on-change on-change
(mf/use-fn (mf/use-fn
@ -216,11 +237,13 @@
:hint-message (:message hint) :hint-message (:message hint)
:on-key-down on-key-down :on-key-down on-key-down
:hint-type (:type hint) :hint-type (:type hint)
:input-wrapper-ref input-wrapper-ref
:ref ref :ref ref
:role "combobox" :role "combobox"
:aria-activedescendant focused-id :aria-activedescendant focused-id
:aria-controls listbox-id :aria-controls listbox-id
:aria-expanded is-open :aria-expanded is-open
:data-option-focused (boolean focused-id)
:slot-end :slot-end
(when (some? @filtered-tokens-by-type) (when (some? @filtered-tokens-by-type)
(mf/html (mf/html
@ -241,7 +264,7 @@
props) 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] (mf/with-effect [resolve-stream tokens token name token-name]
(let [subs (->> resolve-stream (let [subs (->> resolve-stream
@ -300,7 +323,7 @@
:id listbox-id :id listbox-id
:options options :options options
:focused focused-id :focused focused-id
:selected nil :selected selected-id
:align :right :align :right
:empty-to-end empty-to-end :empty-to-end empty-to-end
:wrapper-ref dropdown-ref :wrapper-ref dropdown-ref

View File

@ -26,14 +26,13 @@
[focusables focused-id direction] [focusables focused-id direction]
(let [ids (vec (map :id focusables)) (let [ids (vec (map :id focusables))
idx (.indexOf (clj->js ids) focused-id) idx (.indexOf (clj->js ids) focused-id)
idx (if (= idx -1) -1 idx) count (count ids)]
next-idx (case direction (case direction
:down (min (dec (count ids)) (inc idx)) :down (nth ids (mod (inc idx) count) nil)
:up (max 0 (dec (if (= idx -1) 0 idx))))] :up (nth ids (mod (if (= idx -1) 0 (dec idx)) count) nil))))
(nth ids next-idx nil)))
(defn use-navigation (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) (let [focused-id* (mf/use-state nil)
focused-id (deref focused-id*) focused-id (deref focused-id*)
@ -46,6 +45,7 @@
down? (kbd/down-arrow? event) down? (kbd/down-arrow? event)
enter? (kbd/enter? event) enter? (kbd/enter? event)
esc? (kbd/esc? event) esc? (kbd/esc? event)
tab? (kbd/tab? event)
open-dropdown (kbd/is-key? event "{") open-dropdown (kbd/is-key? event "{")
close-dropdown (kbd/is-key? event "}") close-dropdown (kbd/is-key? event "}")
options (if (delay? options) @options options)] options (if (delay? options) @options options)]
@ -56,18 +56,21 @@
(dom/prevent-default event) (dom/prevent-default event)
(let [focusables (focusable-options options)] (let [focusables (focusable-options options)]
(cond (cond
;; Dropdown open: move focus to next option
is-open is-open
(when (seq focusables) (when (seq focusables)
(let [next-id (next-focus-id focusables focused-id :down)] (let [next-id (next-focus-id focusables focused-id :down)]
(reset! focused-id* next-id))) (reset! focused-id* next-id)))
;; Dropdown closed with options: open and focus first
(seq focusables) (seq focusables)
(do (do
(toggle-dropdown event) (toggle-dropdown event)
(when get-selected-id
(get-selected-id))
(reset! focused-id* (first-focusable-id focusables))) (reset! focused-id* (first-focusable-id focusables)))
:else :else nil)))
nil)))
up? up?
(when is-open (when is-open
@ -77,7 +80,9 @@
(reset! focused-id* next-id))) (reset! focused-id* next-id)))
open-dropdown open-dropdown
(reset! is-open* true) (do
(reset! is-open* true)
(reset! focused-id* nil))
close-dropdown close-dropdown
(reset! is-open* false) (reset! is-open* false)
@ -89,21 +94,23 @@
(dom/prevent-default event) (dom/prevent-default event)
(when (some #(= (:id %) focused-id) focusables) (when (some #(= (:id %) focused-id) focusables)
(on-enter focused-id))))) (on-enter focused-id)))))
esc? esc?
(do (when is-open
(dom/prevent-default event) (dom/prevent-default event)
(dom/stop-propagation event)
(reset! is-open* false)) (reset! is-open* false))
tab?
(when is-open
(reset! is-open* false)
(reset! focused-id* nil))
:else nil))))] :else nil))))]
;; Initial focus on first option (mf/with-effect [is-open]
(mf/with-effect [is-open options] (when (not is-open)
(when is-open (reset! focused-id* nil)))
(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)))))))
;; auto scroll when key down ;; auto scroll when key down
(mf/with-effect [focused-id nodes-ref] (mf/with-effect [focused-id nodes-ref]

View File

@ -9,7 +9,7 @@
[app.util.dom :as dom] [app.util.dom :as dom]
[rumext.v2 :as mf])) [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) (let [position* (mf/use-state nil)
position (deref position*) position (deref position*)
ready* (mf/use-state false) ready* (mf/use-state false)
@ -32,7 +32,7 @@
(> dropdown-height space-below)) (> dropdown-height space-below))
position (if open-up? 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") :left (str (:left combobox-rect) "px")
:width (str (:width combobox-rect) "px") :width (str (:width combobox-rect) "px")
:placement :top} :placement :top}
@ -44,27 +44,41 @@
(reset! ready* true) (reset! ready* true)
(reset! position* position)))] (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 (when is-open
(let [handler (fn [event] (let [recalculate
(let [dropdown-node (mf/ref-val dropdown-ref) (fn []
target (dom/get-target event)] (js/requestAnimationFrame
(when (or (nil? dropdown-node) (fn []
(not (instance? js/Node target)) (let [input-node (mf/ref-val input-wrapper-ref)]
(not (.contains dropdown-node target))) (calculate-position input-node)))))
(js/requestAnimationFrame
(fn [] handler
(let [wrapper-node (mf/ref-val wrapper-ref)] (fn [event]
(reset! ready* true) (let [dropdown-node (mf/ref-val dropdown-ref)
(calculate-position wrapper-node)))))))] 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) (handler nil)
(.addEventListener js/window "resize" handler) (.addEventListener js/window "resize" handler)
(.addEventListener js/window "scroll" handler true) (.addEventListener js/window "scroll" handler true)
(when outer-node
(.observe resize-observer outer-node))
(when dropdown-node
(.observe resize-observer dropdown-node))
(fn [] (fn []
(.removeEventListener js/window "resize" handler) (.removeEventListener js/window "resize" handler)
(.removeEventListener js/window "scroll" handler true))))) (.removeEventListener js/window "scroll" handler true)
(.disconnect resize-observer)))))
{:style position {:style position
:ready? ready :ready? ready

View File

@ -22,6 +22,18 @@
:end (or (str/index-of value "}" last-open) cursor) :end (or (str/index-of value "}" last-open) cursor)
:partial (subs text-before (inc last-open))}))) :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] (defn active-token [value input-node]
(let [cursor (dom/selection-start input-node)] (let [cursor (dom/selection-start input-node)]

View File

@ -9,7 +9,7 @@
[token] [token]
{:id (str (get token :id)) {:id (str (get token :id))
:type :token :type :token
:resolved-value (get token :resolved-value) :resolved-value (get token :value)
:name (get token :name)}) :name (get token :name)})
(defn- generate-dropdown-options (defn- generate-dropdown-options