From 2cdb5757122bc14eefb047e8eac01e336d2f29cb Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Thu, 15 Oct 2026 09:37:31 +0200 Subject: [PATCH] :bug: Fix font selector outside click, empty search and tick (#12285) * :bug: Close the font selector when clicking outside it The font selector had no click-outside handling. It only closed with Esc, or when its menu was unmounted, which is why a viewport click (that deselects the text) closed it and a click on the left sidebar, the rulers or the header did not. Listen to click and contextmenu on the document, as the dropdowns do, and close when the target is outside the selector. The listener starts on the next tick so the opening click does not close it, and a target no longer in the document (a font row removed after selecting it) is not taken as an outside click. The token font combobox uses the same component, so it gets the fix too. Closes #12246 AI-assisted-by: claude-opus-5-5 * :bug: Hide the empty Recent section in the font selector The "Recent" heading showed with `(and recent-fonts show-recent)`, but the filtered recent fonts are a vector and an empty one is truthy. So a search with no matches showed the heading over a blank list, and any search showed it over results that are not recent. - Show the "Recent" section only when it has fonts. - Show "No matching fonts" when the search finds nothing. - Square the list's top corners only when it sits under "Recent". Closes #12249 AI-assisted-by: claude-opus-5-5 * :bug: Separate the applied font from the focused one in font selector The tick marked the first search result when the applied font was filtered out, since one value drove both the tick and the font Enter applies. So the tick showed a font that was not applied. The tick and the focus are now two states: - The tick marks the font the text had when the selector opened. - A green border, as in the select dropdowns, marks the font Enter applies. Typing moves it to the first result without previewing it. Arrow keys move it and, once they stop for a moment, preview the font. Previewing every font of a held key flooded the store until React stopped with "Maximum update depth exceeded". - Recent fonts get no border: they are reached with the mouse or by filtering, not with the arrow keys. - Enter applies the focused font. Esc and an outside click undo the preview. Also make the rows fill the list width, so the border is not clipped, and scroll to the focused row only through the virtualized list. Closes #12247 AI-assisted-by: claude-opus-5-5 * :recycle: Refactor typography.scss to the Design System Move typography.scss to the DS tokens and mixins. No visual change intended, except that the rows that took their typography from %asset-element now use the DS body-small (line-height 1.3 instead of 1.4). - Inline the %asset-element and %input-base placeholders, resolving their design-token aliases to DS tokens and using local CSS variables for state colors. - Keep the spacing inputs on %input-element and %button-icon-small, with a TODO: those placeholders style the icon svg and the input elements, and go away when the controls move to the DS numeric-input*, out of scope here. - Use logical properties and DS spacing and size tokens. - Merge the duplicated .typography-sample rule. - Note why the "Ag" samples have no DS typography, and leave TODOs where no DS typography matches exactly. AI-assisted-by: claude-opus-5-5 --- .../sidebar/options/menus/typography.cljs | 156 +++++++++++------- .../sidebar/options/menus/typography.scss | 147 +++++++++++++---- frontend/translations/en.po | 4 + frontend/translations/es.po | 4 + 4 files changed, 222 insertions(+), 89 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs index f4974abc7b..2a6b472977 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs @@ -291,21 +291,14 @@ (mf/defc font-item* {::mf/wrap [mf/memo]} - [{:keys [font is-current on-click style]}] - (let [item-ref (mf/use-ref) - on-click (mf/use-fn (mf/deps font) #(on-click font))] - - (mf/with-effect [is-current] - (when is-current - (let [element (mf/ref-val item-ref)] - (when-not (dom/is-in-viewport? element) - (dom/scroll-into-view! element))))) - + [{:keys [font is-current is-focused on-click style]}] + (let [on-click (mf/use-fn (mf/deps font) #(on-click font))] [:div {:class (stl/css :font-wrapper) :style style - :ref item-ref :on-click on-click} - [:div {:class (stl/css-case :font-item true :selected is-current)} + [:div {:class (stl/css-case :font-item true + :selected is-current + :focused is-focused)} [:> font-item-preview* {:font font}] (when is-current [:> icon* {:icon-id i/tick @@ -326,11 +319,20 @@ (mf/defc font-selector* [{:keys [on-select on-close current-font show-recent full-size]}] - (let [selected (mf/use-state current-font) + (let [;; The tick marks the font the text had when the selector opened. + ;; The focus (green border) marks the font Enter applies: arrows move + ;; it and preview the font, Esc and an outside click undo the preview + initial-font (mf/with-memo [] current-font) + focused* (mf/use-state current-font) + focused (deref focused*) + previewed? (mf/use-ref false) + preview? (mf/use-ref false) + state* (mf/use-state #(do {:term "" :backends #{}})) state (deref state*) + root (mf/use-ref) flist (mf/use-ref) input (mf/use-ref) @@ -353,52 +355,57 @@ (filter #(contains? installed-ids (:id %))) (filter-fonts state))) - ;; When the active font is not in the filtered results, pre-select - ;; the first match visually so the user can confirm it with Enter — - ;; without live-applying it on every keystroke. - effective-selected - (if (and (seq fonts) (not (d/seek #(= (:id %) (:id @selected)) fonts))) - (first fonts) - @selected) - full-size? (boolean (and full-size show-recent)) - select-next - (mf/use-fn - (mf/deps fonts) - (fn [event] - (dom/stop-propagation event) - (dom/prevent-default event) - (swap! selected get-next-font fonts))) + ;; `recent-fonts` is a vector, so check it with `seq`: an empty one + ;; is truthy and would show the heading over nothing + show-recent? (boolean (and show-recent (seq recent-fonts))) - select-prev + ;; Arrows only move the focus, from its latest value: a held key + ;; fires again before the listener sees the new state. The preview + ;; follows in an effect + focus-and-preview (mf/use-fn (mf/deps fonts) - (fn [event] + (fn [event get-font] (dom/stop-propagation event) (dom/prevent-default event) - (swap! selected get-prev-font fonts))) + (when (seq fonts) + (mf/set-ref-val! preview? true) + (swap! focused* get-font fonts)))) + + on-cancel + (mf/use-fn + (mf/deps initial-font on-select on-close) + (fn [] + (when (mf/ref-val previewed?) + (on-select initial-font)) + (on-close))) on-key-down (mf/use-fn - (mf/deps fonts on-select on-close) + (mf/deps focused focus-and-preview on-cancel on-select on-close) (fn [event] (cond - (kbd/up-arrow? event) (select-prev event) - (kbd/down-arrow? event) (select-next event) - (kbd/esc? event) (on-close) - (kbd/enter? event) - (let [first-result (when-not (d/seek #(= (:id %) (:id @selected)) fonts) - (first fonts))] - (if first-result - (do (on-select first-result) (on-close)) - (on-close))) + (kbd/up-arrow? event) (focus-and-preview event get-prev-font) + (kbd/down-arrow? event) (focus-and-preview event get-next-font) + (kbd/esc? event) (on-cancel) + (kbd/enter? event) (do (when (some? focused) + (on-select focused)) + (on-close)) :else (dom/focus! (mf/ref-val input))))) on-filter-change (mf/use-fn - (fn [event] - (swap! state* assoc :term event))) + (mf/deps state all-fonts) + (fn [term] + ;; Move the focus to the first result, without previewing it. + ;; `swap!` on a `use-state` returns nil, so build the new state here + (let [state (assoc state :term term)] + (reset! state* state) + (mf/set-ref-val! preview? false) + (when (seq term) + (reset! focused* (first (filter-fonts state all-fonts))))))) on-select-and-close (mf/use-fn @@ -407,10 +414,29 @@ (on-select font) (on-close)))] - (mf/with-effect [fonts on-key-down] + (mf/with-effect [on-key-down] (let [key (events/listen js/document "keydown" on-key-down)] #(events/unlistenByKey key))) + ;; Close on an outside click. Starts on the next tick to skip the + ;; opening click; detached targets (a just-selected row) are ignored + (mf/with-effect [on-cancel] + (let [listening? (volatile! false) + on-click (fn [event] + (let [target (dom/get-target event) + node (mf/ref-val root)] + (when (and @listening? + (some? node) + (.-isConnected ^js target) + (not (dom/child? target node))) + (on-cancel)))) + task (tm/schedule #(vreset! listening? true)) + keys [(events/listen js/document "click" on-click) + (events/listen js/document "contextmenu" on-click)]] + (fn [] + (tm/dispose! task) + (run! events/unlistenByKey keys)))) + ;; Materialize the preview sprite into the DOM only while the picker is open ;; (markup is prefetched on workspace load), removing it on close so its ;; ~2000 nodes aren't kept around idle. The attachment is deferred so the @@ -427,16 +453,26 @@ (when-some [n @node*] (fonts/detach-preview-sprite! n)))))) - (mf/with-effect [@selected] + ;; Preview the focused font once the arrows stop for a moment. Applying + ;; every font of a held key floods the store and the text editor with + ;; updates, until React stops with "Maximum update depth exceeded" + (mf/with-effect [focused] + (when (mf/ref-val preview?) + (mf/set-ref-val! preview? false) + (let [task (tm/schedule 100 (fn [] + (mf/set-ref-val! previewed? true) + (on-select focused)))] + #(tm/dispose! task)))) + + ;; The only scroll to the focused row: the list is virtualized, so the row + ;; may not be rendered yet + (mf/with-effect [focused fonts] (let [node (mf/ref-val flist) - index (:index @selected)] + index (d/index-of-pred fonts #(= (:id %) (:id focused)))] ;; This is nil safe operation, do nothing if node or index are ;; invalid. (dom/scroll-to-row node index))) - (mf/with-effect [@selected] - (on-select @selected)) - (mf/with-effect [] (st/emit! (dsc/push-shortcuts :typography {} :workspace)) (fn [] @@ -451,7 +487,8 @@ (dom/scroll-to-position node offset))))) [:div {:class [(stl/css-case :font-selector true - :fonts-on-modal (not full-size?))]} + :fonts-on-modal (not full-size?))] + :ref root} [:div {:class (stl/css-case :font-selector-dropdown true :font-selector-dropdown-full-size full-size?)} [:div {:class (stl/css :header)} @@ -459,7 +496,7 @@ :value (:term state) :auto-focus true :placeholder (tr "workspace.options.search-font")}] - (when (and recent-fonts show-recent) + (when show-recent? [:section {:class (stl/css :show-recent)} [:p {:class (stl/css :header-title)} (tr "workspace.options.recent-fonts")] (for [[idx font] (d/enumerate recent-fonts)] @@ -467,15 +504,21 @@ :font font :style {} :on-click on-select-and-close - :is-current (= (:id font) (:id effective-selected))}])])] + ;; Recent fonts are reached with the mouse or by + ;; filtering, never with the arrow keys, so no focus + :is-current (= (:id font) (:id initial-font))}])])] [:div {:class (stl/css-case :fonts-list true - :fonts-list-full-size full-size?)} + :fonts-list-full-size full-size? + :fonts-list-below-recent show-recent?)} + (when (empty? fonts) + [:p {:class (stl/css :fonts-list-empty)} + (tr "workspace.options.no-font-results")]) [:> rvt/AutoSizer {} (fn [props] (let [width (unchecked-get props "width") height (unchecked-get props "height") - render #(row-renderer fonts effective-selected on-select-and-close %)] + render #(row-renderer fonts (:id initial-font) (:id focused) on-select-and-close %)] (mf/html [:> rvt/List #js {:height height :ref flist @@ -485,7 +528,7 @@ :rowRenderer render}])))]]]])) (defn row-renderer - [fonts selected on-select props] + [fonts current-id focused-id on-select props] (let [index (unchecked-get props "index") key (unchecked-get props "key") style (unchecked-get props "style") @@ -495,7 +538,8 @@ :font font :style style :on-click on-select - :is-current (= (:id font) (:id selected))}]))) + :is-current (= (:id font) current-id) + :is-focused (= (:id font) focused-id)}]))) (mf/defc font-options* [{:keys [values on-change on-blur show-recent full-size-selector]}] diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.scss b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.scss index bbfa19734e..ddc47c5b97 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.scss +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.scss @@ -13,6 +13,8 @@ // Must match BOX_HEIGHT in scripts/build-fonts-preview.js (the sprite bakes each // font name into that box at 1 unit == 1px). +// NOTE: kept in px on purpose, not px2rem(): the sprite is baked in pixels, so +// the box must not scale with the root font size. $font-preview-box-height: 28px; .typography-entry { @@ -72,15 +74,6 @@ $font-preview-box-height: 28px; } } -.typography-sample { - display: flex; - justify-content: center; - align-items: center; - min-inline-size: $sz-24; - block-size: $sz-32; - color: var(--color-foreground-secondary); -} - .typography-name { color: var(--color-foreground-primary); } @@ -113,18 +106,23 @@ $font-preview-box-height: 28px; } .typography-sample-input { + // No DS typography on purpose: the "Ag" sample is drawn in the + // typography's own font + font-size: px2rem(16); + line-height: 1; display: flex; justify-content: center; align-items: center; inline-size: $sz-24; block-size: 100%; - font-size: px2rem(16); - line-height: 1; overflow: hidden; color: var(--color-foreground-primary); } .adv-typography-name { + // TODO: No exact DS typography match found. Rename input of a + // typography; only the font size is set. + // Closest match: body-small (12px). font-size: px2rem(12); color: var(--font-name-wrapper-foreground-color); flex-grow: 1; @@ -155,25 +153,30 @@ $font-preview-box-height: 28px; } .typography-name-wrapper { - @extend %asset-element; + @include t.use-typography("body-small"); display: grid; grid-template-columns: $sz-24 auto 1fr $sz-28; flex: 1; + align-items: center; block-size: $sz-32; inline-size: 100%; padding: 0 0 0 var(--sp-m); + border-radius: $br-8; background-color: var(--color-background-quaternary); + color: var(--color-foreground-primary); margin-block-end: var(--sp-xs); } .typography-sample { + // No DS typography on purpose: the "Ag" sample is drawn in the + // typography's own font + font-size: px2rem(16); + line-height: 1; display: flex; justify-content: center; align-items: center; min-inline-size: $sz-24; - font-size: px2rem(16); - line-height: 1; block-size: $sz-32; padding: 0; overflow: hidden; @@ -186,8 +189,8 @@ $font-preview-box-height: 28px; margin: 0; min-inline-size: px2rem(6); - padding: 8px 0 8px 6px; - max-width: 100%; + padding: var(--sp-s) 0 var(--sp-s) px2rem(6); + max-inline-size: 100%; color: var(--color-foreground-primary); } @@ -210,7 +213,7 @@ $font-preview-box-height: 28px; display: grid; grid-template-columns: 50% 50%; block-size: $sz-32; - padding-left: var(--sp-xs); + padding-inline-start: var(--sp-xs); } .info-label, @@ -235,11 +238,22 @@ $font-preview-box-height: 28px; } .font-option { - @include t.use-typography("body-small"); - @extend %asset-element; + --font-option-background-color: var(--color-background-tertiary); + @include t.use-typography("body-small"); + + display: flex; + align-items: center; + block-size: $sz-32; padding: var(--sp-s); + border-radius: $br-8; + background-color: var(--font-option-background-color); + color: var(--color-foreground-primary); cursor: pointer; + + &:hover { + --font-option-background-color: var(--color-background-quaternary); + } } .font-option-name { @@ -255,15 +269,26 @@ $font-preview-box-height: 28px; } .font-size-options { - @extend %asset-element; + --font-size-options-background-color: var(--color-background-tertiary); + @include t.use-typography("body-small"); + display: flex; flex-grow: 1; + align-items: center; + block-size: $sz-32; inline-size: px2rem(60); margin: 0; padding: 0; border: $b-1 solid var(--color-background-tertiary); + border-radius: $br-8; + background-color: var(--font-size-options-background-color); + color: var(--color-foreground-primary); position: relative; + + &:hover { + --font-size-options-background-color: var(--color-background-quaternary); + } } .font-variant-options { @@ -283,6 +308,9 @@ $font-preview-box-height: 28px; gap: var(--sp-xs); } +// TODO: Kept on the deprecated %input-element and %button-icon-small +// placeholders, which style the icon svg and the input elements. They go +// away when these controls move to the DS numeric-input*. .line-height, .letter-spacing { @extend %input-element; @@ -301,11 +329,22 @@ $font-preview-box-height: 28px; } .text-transform { - @extend %asset-element; + --text-transform-background-color: var(--color-background-tertiary); + @include t.use-typography("body-small"); + + display: flex; + align-items: center; + block-size: $sz-32; inline-size: fit-content; padding: 0; - background-color: var(--color-background-tertiary); + border-radius: $br-8; + background-color: var(--text-transform-background-color); + color: var(--color-foreground-primary); + + &:hover { + --text-transform-background-color: var(--color-background-quaternary); + } } .font-size-select { @@ -320,10 +359,24 @@ $font-preview-box-height: 28px; outline: none; .numeric-input { - @extend %input-base; @include t.use-typography("body-small"); + @include text-ellipsis; + block-size: $sz-28; + inline-size: 100%; + flex-grow: 1; + margin: var(--sp-xxs) 0; padding: 0; + border: none; + border-radius: $br-8; + background: none; + outline: none; + color: var(--color-foreground-primary); + + &[disabled] { + opacity: 0.5; + pointer-events: none; + } } } @@ -332,16 +385,15 @@ $font-preview-box-height: 28px; justify-content: center; align-items: center; position: absolute; - top: calc(-1 * #{px2rem(18)}); - left: 0; - right: 0; + inset-block-start: calc(-1 * #{px2rem(18)}); + inset-inline: 0; block-size: 100%; inline-size: 100%; z-index: var(--z-index-dropdown); } .fonts-on-modal { - top: 0; + inset-block-start: 0; } .show-recent { @@ -377,22 +429,39 @@ $font-preview-box-height: 28px; } .font-wrapper { - padding-bottom: var(--sp-xs); + padding-block-end: var(--sp-xs); cursor: pointer; } .font-item { - @extend %asset-element; + --font-item-background-color: var(--color-background-tertiary); + + @include t.use-typography("body-small"); - width: px2rem(280); - margin-bottom: var(--sp-xs); - border-radius: $br-8; display: flex; + block-size: $sz-32; + inline-size: 100%; + margin-block-end: var(--sp-xs); + padding: var(--sp-s) var(--sp-m); + border-radius: $br-8; + background-color: var(--font-item-background-color); + color: var(--color-foreground-primary); + + &:hover { + --font-item-background-color: var(--color-background-quaternary); + } &.selected { color: var(--color-foreground-primary); } + // The font Enter applies, same border as the focused option of the + // select dropdowns + &.focused { + outline: $b-1 solid var(--color-accent-primary); + outline-offset: calc(-1 * #{$b-1}); + } + // Center & clip so a previewed font's own metrics never grow/overflow the row. align-items: center; overflow: hidden; @@ -448,9 +517,21 @@ $font-preview-box-height: 28px; } .fonts-list-full-size { + border: $b-1 solid var(--color-background-quaternary); +} + +// Joins the list to the "Recent" section above it +.fonts-list-below-recent { border-start-start-radius: 0; border-start-end-radius: 0; - border: $b-1 solid var(--color-background-quaternary); +} + +.fonts-list-empty { + @include t.use-typography("body-small"); + + color: var(--color-foreground-secondary); + margin: 0; + padding: var(--sp-m); } .dropdown-icon { diff --git a/frontend/translations/en.po b/frontend/translations/en.po index eb04aa1bf8..b9a3544478 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -7711,6 +7711,10 @@ msgstr "More library colors" msgid "workspace.options.more-token-colors" msgstr "More color tokens" +#: src/app/main/ui/workspace/sidebar/options/menus/typography.cljs:502 +msgid "workspace.options.no-font-results" +msgstr "No matching fonts" + #: src/app/main/ui/workspace/sidebar/options/drawing/frame.cljs:117, src/app/main/ui/workspace/sidebar/options/menus/measures.cljs:518 msgid "workspace.options.no-size-preset-results" msgstr "No matching size preset" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 59bc353113..9adb3cde53 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -7777,6 +7777,10 @@ msgstr "Más colores de la biblioteca" msgid "workspace.options.more-token-colors" msgstr "Más tokens de color" +#: src/app/main/ui/workspace/sidebar/options/menus/typography.cljs:502 +msgid "workspace.options.no-font-results" +msgstr "No hay fuentes coincidentes" + #: src/app/main/ui/workspace/sidebar/options/drawing/frame.cljs:117, src/app/main/ui/workspace/sidebar/options/menus/measures.cljs:518 msgid "workspace.options.no-size-preset-results" msgstr "No hay tamaños predefinidos coincidentes"