From 0a80fc55173eca18f493c3eacee056147618ae51 Mon Sep 17 00:00:00 2001
From: icarus <eurfelux@gmail.com>
Date: Tue, 23 Sep 2025 22:20:35 +0800
Subject: [PATCH] refactor(hooks): improve scroll position hook with timer
 utility

- Replace setTimeout with useTimer utility for better cleanup
- Add throttleWait parameter for configurable scroll throttling
- Update documentation to reflect changes
---
 src/renderer/src/hooks/useScrollPosition.ts   | 26 ++++++------
 .../AgentSettings/AgentToolingSettings.tsx    | 40 +++++++++++--------
 2 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/src/renderer/src/hooks/useScrollPosition.ts b/src/renderer/src/hooks/useScrollPosition.ts
index 872004d58e..acb1bd851b 100644
--- a/src/renderer/src/hooks/useScrollPosition.ts
+++ b/src/renderer/src/hooks/useScrollPosition.ts
@@ -1,30 +1,32 @@
 import { throttle } from 'lodash'
 import { useEffect, useRef } from 'react'
 
-export default function useScrollPosition(key: string) {
+import { useTimer } from './useTimer'
+
+/**
+ * A custom hook that manages scroll position persistence for a container element
+ * @param key - A unique identifier used to store/retrieve the scroll position
+ * @returns An object containing:
+ *  - containerRef: React ref for the scrollable container
+ *  - handleScroll: Throttled scroll event handler that saves scroll position
+ */
+export default function useScrollPosition(key: string, throttleWait?: number) {
   const containerRef = useRef<HTMLDivElement>(null)
   const scrollKey = `scroll:${key}`
-  const scrollTimerRef = useRef<NodeJS.Timeout>(undefined)
+  const { setTimeoutTimer } = useTimer()
 
   const handleScroll = throttle(() => {
     const position = containerRef.current?.scrollTop ?? 0
     window.requestAnimationFrame(() => {
       window.keyv.set(scrollKey, position)
     })
-  }, 100)
+  }, throttleWait ?? 100)
 
   useEffect(() => {
     const scroll = () => containerRef.current?.scrollTo({ top: window.keyv.get(scrollKey) || 0 })
     scroll()
-    clearTimeout(scrollTimerRef.current)
-    scrollTimerRef.current = setTimeout(scroll, 50)
-  }, [scrollKey])
-
-  useEffect(() => {
-    return () => {
-      clearTimeout(scrollTimerRef.current)
-    }
-  }, [])
+    setTimeoutTimer('scrollEffect', scroll, 50)
+  }, [scrollKey, setTimeoutTimer])
 
   return { containerRef, handleScroll }
 }
diff --git a/src/renderer/src/pages/settings/AgentSettings/AgentToolingSettings.tsx b/src/renderer/src/pages/settings/AgentSettings/AgentToolingSettings.tsx
index 1270ebf5a2..53ed29cf12 100644
--- a/src/renderer/src/pages/settings/AgentSettings/AgentToolingSettings.tsx
+++ b/src/renderer/src/pages/settings/AgentSettings/AgentToolingSettings.tsx
@@ -1,6 +1,7 @@
 import { Alert, Card, CardBody, CardHeader, Chip, Input, Switch } from '@heroui/react'
 import { useAgentClient } from '@renderer/hooks/agents/useAgentClient'
 import { useMCPServers } from '@renderer/hooks/useMCPServers'
+import useScrollPosition from '@renderer/hooks/useScrollPosition'
 import {
   AgentConfiguration,
   AgentConfigurationSchema,
@@ -11,7 +12,7 @@ import {
 } from '@renderer/types'
 import { Modal } from 'antd'
 import { ShieldAlert, ShieldCheck, Wrench } from 'lucide-react'
-import { FC, useCallback, useEffect, useMemo, useState } from 'react'
+import { FC, startTransition, useCallback, useEffect, useMemo, useState } from 'react'
 import { useTranslation } from 'react-i18next'
 import { mutate } from 'swr'
 
@@ -105,6 +106,7 @@ const computeModeDefaults = (mode: PermissionMode, tools: Tool[]): string[] => {
 const unique = (values: string[]) => Array.from(new Set(values))
 
 export const AgentToolingSettings: FC<AgentToolingSettingsProps> = ({ agent, updateAgent }) => {
+  const { containerRef, handleScroll } = useScrollPosition('AgentToolingSettings', 100)
   const { t } = useTranslation()
   const client = useAgentClient()
   const { mcpServers: allServers } = useMCPServers()
@@ -252,22 +254,26 @@ export const AgentToolingSettings: FC<AgentToolingSettingsProps> = ({ agent, upd
       if (!agent || isUpdatingTools) {
         return
       }
-      setApprovedToolIds((prev) => {
-        const exists = prev.includes(toolId)
-        if (isApproved === exists) {
-          return prev
-        }
-        const next = isApproved ? [...prev, toolId] : prev.filter((id) => id !== toolId)
-        const sanitized = unique(next.filter((id) => availableTools.some((tool) => tool.id === id)).concat(autoToolIds))
-        setIsUpdatingTools(true)
-        void (async () => {
-          try {
-            await updateAgent({ id: agent.id, allowed_tools: sanitized })
-          } finally {
-            setIsUpdatingTools(false)
+      startTransition(() => {
+        setApprovedToolIds((prev) => {
+          const exists = prev.includes(toolId)
+          if (isApproved === exists) {
+            return prev
           }
-        })()
-        return sanitized
+          const next = isApproved ? [...prev, toolId] : prev.filter((id) => id !== toolId)
+          const sanitized = unique(
+            next.filter((id) => availableTools.some((tool) => tool.id === id)).concat(autoToolIds)
+          )
+          setIsUpdatingTools(true)
+          void (async () => {
+            try {
+              await updateAgent({ id: agent.id, allowed_tools: sanitized })
+            } finally {
+              setIsUpdatingTools(false)
+            }
+          })()
+          return sanitized
+        })
       })
     },
     [agent, isUpdatingTools, availableTools, autoToolIds, updateAgent]
@@ -322,7 +328,7 @@ export const AgentToolingSettings: FC<AgentToolingSettingsProps> = ({ agent, upd
   }
 
   return (
-    <SettingsContainer>
+    <SettingsContainer ref={containerRef} onScroll={handleScroll}>
       {contextHolder}
       <SettingsItem>
         <SettingsTitle>