mirror of
https://github.com/penpot/penpot.git
synced 2026-07-28 17:06:21 +00:00
🐛 Fix leaked focus timers in dashboard sidebar navigation (#10715)
* 🐛 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 * 🐛 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 * ♻️ 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 * ♻️ 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 * 📎 Add pr feedback fix
This commit is contained in:
parent
6c2b61e1ad
commit
4b994d20aa
@ -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
|
||||
|
||||
@ -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))
|
||||
|
||||
@ -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]
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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]
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user