From 4b994d20aac78723770ae6538bd3372b7936add1 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 28 Jul 2026 11:06:00 +0200 Subject: [PATCH] :bug: Fix leaked focus timers in dashboard sidebar navigation (#10715) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * :bug: Fix leaked deferred DOM ops on dashboard navigation and template clone The React reconciliation "removeChild" error surfaced during rapid dashboard navigation because several effects scheduled deferred DOM operations (focus, CSS positioning) without returning a cleanup that cancelled them. When the component unmounted before the callback fired, it ran against stale DOM and desynchronized React fiber tree from the actual DOM. - context_menu_a11y.cljs: replace tm/schedule-on-idle (30s idle window) with tm/schedule (setTimeout 0) and return a rx/dispose! cleanup. - dropdown.cljs: capture the tm/schedule handle and dispose it in the effect cleanup. - tooltip.cljs: capture the ts/raf handle and cancel it on cleanup. AI-assisted-by: opencode-go/mimo-v2.5-pro * :bug: Fix leaked focus timers in dashboard sidebar navigation Six sidebar navigation handlers scheduled setTimeout callbacks to mutate tabindex/focus on React-managed title elements without cancelling prior pending callbacks. During rapid keyboard navigation (Projects→Fonts→Libs→Drafts) the stale callbacks fired against unmounted DOM, desyncing React fiber tree and triggering "removeChild" NotFoundError. - sidebar-project*: cancel prior timer in on-key-down - sidebar-search*: cancel prior timer in on-key-press - sidebar-content*: cancel prior timer in go-projects-with-key, go-fonts-with-key, go-drafts-with-key, go-libs-with-key Each handler now stores the timer handle in a component-level ref and disposes any pending handle before scheduling a new one. AI-assisted-by: opencode-go/mimo-v2.5-pro * :recycle: Refactor sidebar focus timer handling into helpers Extract the repeated dispose-before-schedule focus idiom into schedule-focus-by-id! (sidebar.cljs) and focus-and-untabbable! (app.util.dom). Replaces the six duplicated blocks and adds mf/use-effect unmount cleanup to dispose any pending timer in the three sidebar components, closing the remaining leak noted in the original fix. AI-assisted-by: opencode/hy3-free * :recycle: Extract use-focus-timer-ref hook for sidebar components Replace the duplicated mf/use-ref + mf/use-effect cleanup pairs in sidebar-project*, sidebar-search*, and sidebar-content* with a shared use-focus-timer-ref hook (app.main.ui.hooks). The hook creates the ref and disposes any pending timer on unmount via mf/with-effect, reading the ref with mf/ref-val instead of deref. mf/use-effect is now a body-level hook call rather than a let binding. AI-assisted-by: opencode/hy3-free * :paperclip: Add pr feedback fix --- .../main/ui/components/context_menu_a11y.cljs | 8 ++- .../src/app/main/ui/components/dropdown.cljs | 8 ++- .../src/app/main/ui/dashboard/sidebar.cljs | 64 +++++++------------ .../src/app/main/ui/ds/tooltip/tooltip.cljs | 37 +++++------ frontend/src/app/main/ui/hooks.cljs | 10 +++ frontend/src/app/util/dom.cljs | 7 ++ 6 files changed, 70 insertions(+), 64 deletions(-) diff --git a/frontend/src/app/main/ui/components/context_menu_a11y.cljs b/frontend/src/app/main/ui/components/context_menu_a11y.cljs index 75c97554ce..e2e824c19e 100644 --- a/frontend/src/app/main/ui/components/context_menu_a11y.cljs +++ b/frontend/src/app/main/ui/components/context_menu_a11y.cljs @@ -18,6 +18,7 @@ [app.util.i18n :as i18n :refer [tr]] [app.util.keyboard :as kbd] [app.util.timers :as tm] + [beicon.v2.core :as rx] [rumext.v2 :as mf])) (def ^:private xf:options @@ -229,8 +230,11 @@ (partial ug/unlisten "penpot:context-menu:open" on-event))) (mf/with-effect [ids] - (tm/schedule-on-idle - #(dom/focus! (dom/get-element (first ids))))) + (let [handle (tm/schedule + (fn [] + (some-> (dom/get-element (first ids)) + (dom/focus!))))] + #(rx/dispose! handle))) (when (some? levels) [:> dropdown-content* props diff --git a/frontend/src/app/main/ui/components/dropdown.cljs b/frontend/src/app/main/ui/components/dropdown.cljs index 894d9ef204..4a0a1b0590 100644 --- a/frontend/src/app/main/ui/components/dropdown.cljs +++ b/frontend/src/app/main/ui/components/dropdown.cljs @@ -11,6 +11,7 @@ [app.util.globals :as globals] [app.util.keyboard :as kbd] [app.util.timers :as tm] + [beicon.v2.core :as rx] [goog.events :as events] [rumext.v2 :as mf]) (:import goog.events.EventType)) @@ -45,9 +46,10 @@ (fn [] (let [keys [(events/listen globals/document EventType.CLICK on-click) (events/listen globals/document EventType.CONTEXTMENU on-click) - (events/listen globals/document EventType.KEYUP on-keyup)]] - (tm/schedule #(mf/set-ref-val! listening-ref true)) - #(run! events/unlistenByKey keys)))] + (events/listen globals/document EventType.KEYUP on-keyup)] + timer (tm/schedule #(mf/set-ref-val! listening-ref true))] + #(do (rx/dispose! timer) + (run! events/unlistenByKey keys))))] (mf/use-effect on-mount) children)) diff --git a/frontend/src/app/main/ui/dashboard/sidebar.cljs b/frontend/src/app/main/ui/dashboard/sidebar.cljs index eeda604490..53d87a8878 100644 --- a/frontend/src/app/main/ui/dashboard/sidebar.cljs +++ b/frontend/src/app/main/ui/dashboard/sidebar.cljs @@ -40,6 +40,7 @@ [app.main.ui.ds.buttons.button :refer [button*]] [app.main.ui.ds.foundations.assets.icon :refer [icon*] :as i] [app.main.ui.ds.foundations.assets.raw-svg :refer [raw-svg*]] + [app.main.ui.hooks :refer [use-focus-timer-ref]] [app.main.ui.icons :as deprecated-icon] [app.main.ui.nitrate.nitrate-form] [app.util.dom :as dom] @@ -94,6 +95,14 @@ (def ^:private ^:svg-id penpot-logo-icon "penpot-logo-icon") (def ^:private ^:svg-id penpot-logo-icon-subtle "penpot-logo-subtle") +(defn schedule-focus-by-id! + [ref element-id] + (when-let [h (mf/ref-val ref)] + (ts/dispose! h)) + (mf/set-ref-val! ref + (ts/schedule + #(dom/focus-and-untabbable! (dom/get-element element-id))))) + (mf/defc sidebar-project* {::mf/private true} [{:keys [item is-selected]}] @@ -112,6 +121,8 @@ project-id (get item :id) + focus-timer-ref (use-focus-timer-ref) + on-click (mf/use-fn (mf/deps project-id) @@ -123,14 +134,9 @@ (mf/deps project-id) (fn [event] (when (kbd/enter? event) - (st/emit! - (dcm/go-to-dashboard-files :project-id project-id)) - (ts/schedule - (fn [] - (when-let [title (dom/get-element (str project-id))] - (dom/set-attribute! title "tabindex" "0") - (dom/focus! title) - (dom/set-attribute! title "tabindex" "-1"))))))) + (schedule-focus-by-id! focus-timer-ref (str project-id)) + (st/emit! (dcm/go-to-dashboard-files :project-id project-id))))) + on-menu-click (mf/use-fn @@ -228,6 +234,8 @@ focused? (mf/use-state false) emit! (mf/use-memo #(f/debounce st/emit! 500)) + focus-timer-ref (use-focus-timer-ref) + on-search-blur (mf/use-fn (fn [_] @@ -254,13 +262,7 @@ (mf/use-fn (fn [e] (when (kbd/enter? e) - (ts/schedule - (fn [] - (let [search-title (dom/get-element (str "dashboard-search-title"))] - (when search-title - (dom/set-attribute! search-title "tabindex" "0") - (dom/focus! search-title) - (dom/set-attribute! search-title "tabindex" "-1"))))) + (schedule-focus-by-id! focus-timer-ref "dashboard-search-title") (dom/prevent-default e) (dom/stop-propagation e)))) @@ -948,6 +950,8 @@ nitrate? (contains? cf/flags :nitrate) + focus-timer-ref (use-focus-timer-ref) + go-projects (mf/use-fn #(st/emit! (dcm/go-to-dashboard-recent))) @@ -957,12 +961,7 @@ (fn [] (st/emit! (dcm/go-to-dashboard-recent :team-id team-id)) - (ts/schedule - (fn [] - (when-let [projects-title (dom/get-element "dashboard-projects-title")] - (dom/set-attribute! projects-title "tabindex" "0") - (dom/focus! projects-title) - (dom/set-attribute! projects-title "tabindex" "-1")))))) + (schedule-focus-by-id! focus-timer-ref "dashboard-projects-title"))) go-fonts (mf/use-fn @@ -975,13 +974,7 @@ (fn [] (st/emit! (dcm/go-to-dashboard-fonts :team-id team-id)) - (ts/schedule - (fn [] - (let [font-title (dom/get-element "dashboard-fonts-title")] - (when font-title - (dom/set-attribute! font-title "tabindex" "0") - (dom/focus! font-title) - (dom/set-attribute! font-title "tabindex" "-1"))))))) + (schedule-focus-by-id! focus-timer-ref "dashboard-fonts-title"))) go-drafts (mf/use-fn @@ -994,12 +987,7 @@ (mf/deps team-id default-project-id) (fn [] (st/emit! (dcm/go-to-dashboard-files :team-id team-id :project-id default-project-id)) - (ts/schedule - (fn [] - (when-let [title (dom/get-element "dashboard-drafts-title")] - (dom/set-attribute! title "tabindex" "0") - (dom/focus! title) - (dom/set-attribute! title "tabindex" "-1")))))) + (schedule-focus-by-id! focus-timer-ref "dashboard-drafts-title"))) go-libs (mf/use-fn @@ -1012,13 +1000,7 @@ (fn [] (st/emit! (dcm/go-to-dashboard-libraries :team-id team-id)) - (ts/schedule - (fn [] - (let [libs-title (dom/get-element "dashboard-libraries-title")] - (when libs-title - (dom/set-attribute! libs-title "tabindex" "0") - (dom/focus! libs-title) - (dom/set-attribute! libs-title "tabindex" "-1"))))))) + (schedule-focus-by-id! focus-timer-ref "dashboard-libraries-title"))) pinned-projects (mf/with-memo [projects] diff --git a/frontend/src/app/main/ui/ds/tooltip/tooltip.cljs b/frontend/src/app/main/ui/ds/tooltip/tooltip.cljs index bc20c517c9..e3d6b153fb 100644 --- a/frontend/src/app/main/ui/ds/tooltip/tooltip.cljs +++ b/frontend/src/app/main/ui/ds/tooltip/tooltip.cljs @@ -322,25 +322,26 @@ (let [trigger-el (mf/ref-val trigger-ref) tooltip-el (mf/ref-val tooltip-ref)] (when (and trigger-el tooltip-el) - (ts/raf - (fn [] - (let [origin-brect (dom/get-bounding-rect trigger-el) - tooltip-brect (dom/get-bounding-rect tooltip-el) - window-size (dom/get-window-size)] - (when-let [[new-placement placement-rect] - (find-matching-placement - placement - tooltip-brect - origin-brect - window-size - offset)] - (dom/set-css-property! tooltip-el "inset-block-start" - (str (:top placement-rect) "px")) - (dom/set-css-property! tooltip-el "inset-inline-start" - (str (:left placement-rect) "px")) + (let [raf-id (ts/raf + (fn [] + (let [origin-brect (dom/get-bounding-rect trigger-el) + tooltip-brect (dom/get-bounding-rect tooltip-el) + window-size (dom/get-window-size)] + (when-let [[new-placement placement-rect] + (find-matching-placement + placement + tooltip-brect + origin-brect + window-size + offset)] + (dom/set-css-property! tooltip-el "inset-block-start" + (str (:top placement-rect) "px")) + (dom/set-css-property! tooltip-el "inset-inline-start" + (str (:left placement-rect) "px")) - (when (not= new-placement placement) - (reset! placement* new-placement))))))))))) + (when (not= new-placement placement) + (reset! placement* new-placement))))))] + #(ts/cancel-af! raf-id))))))) [:> :div props children diff --git a/frontend/src/app/main/ui/hooks.cljs b/frontend/src/app/main/ui/hooks.cljs index ae8ebd30d5..bb541dd8ea 100644 --- a/frontend/src/app/main/ui/hooks.cljs +++ b/frontend/src/app/main/ui/hooks.cljs @@ -281,6 +281,16 @@ (mf/set-ref-val! ref val)) (mf/ref-val ref))) +;; FIXME: replace with rumext +(defn use-focus-timer-ref + "Returns a ref for scheduling focus timers and disposes any pending + timer on component unmount." + [] + (let [ref (mf/use-ref nil)] + (mf/with-effect [] + #(some-> (mf/ref-val ref) ts/dispose!)) + ref)) + ;; FIXME: rename to use-focus-objects (defn with-focus-objects ([objects] diff --git a/frontend/src/app/util/dom.cljs b/frontend/src/app/util/dom.cljs index f4be22b6c9..d885e7771b 100644 --- a/frontend/src/app/util/dom.cljs +++ b/frontend/src/app/util/dom.cljs @@ -699,6 +699,13 @@ (when (some? node) (.setAttribute node attr value))) +(defn focus-and-untabbable! + [^js node] + (when (some? node) + (set-attribute! node "tabindex" "0") + (focus! node) + (set-attribute! node "tabindex" "-1"))) + (defn set-style! [^js node ^string style value] (when (some? node)