From c3c22e467404e5edacb5ca8f98122a614102ae0e Mon Sep 17 00:00:00 2001 From: Waleed Date: Thu, 19 Mar 2026 12:57:10 -0700 Subject: [PATCH] improvement(react): replace unnecessary useEffect patterns with better React primitives (#3675) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * improvement(react): replace unnecessary useEffect patterns with better React primitives * fix(react): revert unsafe render-time side effects to useEffect * fix(react): restore useEffect for modals, scroll, and env sync - Modals (create-workspace, rename-document, edit-knowledge-base): restore useEffect watching `open` prop for form reset on programmatic open, since Radix onOpenChange doesn't fire for parent-driven prop changes - Popover: add useEffect watching `open` for programmatic close reset - Chat scroll: restore useEffect watching `isStreamingResponse` so the 1s suppression timer starts when streaming begins, not before the fetch - Credentials manager: revert render-time pattern to useEffect for initial sync from cached React Query data (useRef captures initial value, making the !== check always false on mount) * fix(react): restore useEffect for help/invite modals, combobox index reset - Help modal: restore useEffect watching `open` for form reset on programmatic open (same Radix onOpenChange pattern as other modals) - Invite modal: restore useEffect watching `open` to clear error on programmatic open - Combobox: restore useEffect to reset highlightedIndex when filtered options shrink (prevents stale index from reappearing when options grow) - Remove no-op handleOpenChange wrappers in rename-document and edit-knowledge-base modals (now pure pass-throughs after useEffect fix) * fix(context-menu): use requestAnimationFrame for ColorGrid focus, remove no-op wrapper in create-workspace-modal - ColorGrid: replaced setTimeout with requestAnimationFrame for initial focus to wait for submenu paint completion - create-workspace-modal: removed handleOpenChange pass-through wrapper, use onOpenChange directly * fix(files): restore filesRef pattern to prevent preview mode reset on refetch The useEffect that sets previewMode should only run when selectedFileId changes, not when files array reference changes from React Query refetch. Restores the filesRef pattern to read latest files without triggering the effect — prevents overriding user's manual mode selection. * fix(add-documents-modal, combobox): restore useEffect for modal reset, fix combobox dep array - add-documents-modal: handleOpenChange(true) is dead code in Radix controlled mode — restored useEffect watching open for reset-on-open - combobox: depend on filteredOptions array (not .length) so highlight resets when items change even with same count --- apps/sim/app/(auth)/login/login-form.tsx | 43 ++++++---------- .../reset-password/reset-password-content.tsx | 17 +++---- apps/sim/app/(auth)/signup/signup-form.tsx | 45 ++++++----------- apps/sim/app/chat/components/input/input.tsx | 21 +++----- .../voice-interface/voice-interface.tsx | 50 ++++++++++--------- .../add-documents-modal.tsx | 30 +++++++---- .../credentials/credentials-manager.tsx | 13 +++-- .../components/help-modal/help-modal.tsx | 26 ++++++---- .../components/context-menu/context-menu.tsx | 35 +++++++------ .../create-workspace-modal.tsx | 30 +++++------ .../components/permissions-table.tsx | 19 ++++--- .../w/components/sidebar/sidebar.tsx | 31 +++++++++--- .../emcn/components/combobox/combobox.tsx | 34 ++++++------- .../components/date-picker/date-picker.tsx | 9 ++-- .../emcn/components/popover/popover.tsx | 41 +++++++++------ .../components/time-picker/time-picker.tsx | 6 ++- 16 files changed, 231 insertions(+), 219 deletions(-) diff --git a/apps/sim/app/(auth)/login/login-form.tsx b/apps/sim/app/(auth)/login/login-form.tsx index 85e924fd32..f6e842a602 100644 --- a/apps/sim/app/(auth)/login/login-form.tsx +++ b/apps/sim/app/(auth)/login/login-form.tsx @@ -1,6 +1,6 @@ 'use client' -import { useEffect, useState } from 'react' +import { useEffect, useRef, useState } from 'react' import { createLogger } from '@sim/logger' import { Eye, EyeOff } from 'lucide-react' import Link from 'next/link' @@ -99,15 +99,21 @@ export default function LoginPage({ const router = useRouter() const searchParams = useSearchParams() const [isLoading, setIsLoading] = useState(false) - const [_mounted, setMounted] = useState(false) const [showPassword, setShowPassword] = useState(false) const [password, setPassword] = useState('') const [passwordErrors, setPasswordErrors] = useState([]) const [showValidationError, setShowValidationError] = useState(false) const buttonClass = useBrandedButtonClass() - const [callbackUrl, setCallbackUrl] = useState('/workspace') - const [isInviteFlow, setIsInviteFlow] = useState(false) + const callbackUrlParam = searchParams?.get('callbackUrl') + const invalidCallbackRef = useRef(false) + if (callbackUrlParam && !validateCallbackUrl(callbackUrlParam) && !invalidCallbackRef.current) { + invalidCallbackRef.current = true + logger.warn('Invalid callback URL detected and blocked:', { url: callbackUrlParam }) + } + const callbackUrl = + callbackUrlParam && validateCallbackUrl(callbackUrlParam) ? callbackUrlParam : '/workspace' + const isInviteFlow = searchParams?.get('invite_flow') === 'true' const [forgotPasswordOpen, setForgotPasswordOpen] = useState(false) const [forgotPasswordEmail, setForgotPasswordEmail] = useState('') @@ -120,30 +126,11 @@ export default function LoginPage({ const [email, setEmail] = useState('') const [emailErrors, setEmailErrors] = useState([]) const [showEmailValidationError, setShowEmailValidationError] = useState(false) - const [resetSuccessMessage, setResetSuccessMessage] = useState(null) - - useEffect(() => { - setMounted(true) - - if (searchParams) { - const callback = searchParams.get('callbackUrl') - if (callback) { - if (validateCallbackUrl(callback)) { - setCallbackUrl(callback) - } else { - logger.warn('Invalid callback URL detected and blocked:', { url: callback }) - } - } - - const inviteFlow = searchParams.get('invite_flow') === 'true' - setIsInviteFlow(inviteFlow) - - const resetSuccess = searchParams.get('resetSuccess') === 'true' - if (resetSuccess) { - setResetSuccessMessage('Password reset successful. Please sign in with your new password.') - } - } - }, [searchParams]) + const [resetSuccessMessage, setResetSuccessMessage] = useState(() => + searchParams?.get('resetSuccess') === 'true' + ? 'Password reset successful. Please sign in with your new password.' + : null + ) useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { diff --git a/apps/sim/app/(auth)/reset-password/reset-password-content.tsx b/apps/sim/app/(auth)/reset-password/reset-password-content.tsx index 9127c6e0b4..a48eedc5f8 100644 --- a/apps/sim/app/(auth)/reset-password/reset-password-content.tsx +++ b/apps/sim/app/(auth)/reset-password/reset-password-content.tsx @@ -1,6 +1,6 @@ 'use client' -import { Suspense, useEffect, useState } from 'react' +import { Suspense, useState } from 'react' import { createLogger } from '@sim/logger' import Link from 'next/link' import { useRouter, useSearchParams } from 'next/navigation' @@ -22,14 +22,9 @@ function ResetPasswordContent() { text: '', }) - useEffect(() => { - if (!token) { - setStatusMessage({ - type: 'error', - text: 'Invalid or missing reset token. Please request a new password reset link.', - }) - } - }, [token]) + const tokenError = !token + ? 'Invalid or missing reset token. Please request a new password reset link.' + : null const handleResetPassword = async (password: string) => { try { @@ -87,8 +82,8 @@ function ResetPasswordContent() { token={token} onSubmit={handleResetPassword} isSubmitting={isSubmitting} - statusType={statusMessage.type} - statusMessage={statusMessage.text} + statusType={tokenError ? 'error' : statusMessage.type} + statusMessage={tokenError ?? statusMessage.text} /> diff --git a/apps/sim/app/(auth)/signup/signup-form.tsx b/apps/sim/app/(auth)/signup/signup-form.tsx index b04ad8af4c..0a8138053a 100644 --- a/apps/sim/app/(auth)/signup/signup-form.tsx +++ b/apps/sim/app/(auth)/signup/signup-form.tsx @@ -1,6 +1,6 @@ 'use client' -import { Suspense, useEffect, useState } from 'react' +import { Suspense, useMemo, useState } from 'react' import { createLogger } from '@sim/logger' import { Eye, EyeOff } from 'lucide-react' import Link from 'next/link' @@ -82,49 +82,32 @@ function SignupFormContent({ const searchParams = useSearchParams() const { refetch: refetchSession } = useSession() const [isLoading, setIsLoading] = useState(false) - const [, setMounted] = useState(false) const [showPassword, setShowPassword] = useState(false) const [password, setPassword] = useState('') const [passwordErrors, setPasswordErrors] = useState([]) const [showValidationError, setShowValidationError] = useState(false) - const [email, setEmail] = useState('') + const [email, setEmail] = useState(() => searchParams.get('email') ?? '') const [emailError, setEmailError] = useState('') const [emailErrors, setEmailErrors] = useState([]) const [showEmailValidationError, setShowEmailValidationError] = useState(false) - const [redirectUrl, setRedirectUrl] = useState('') - const [isInviteFlow, setIsInviteFlow] = useState(false) const buttonClass = useBrandedButtonClass() + const redirectUrl = useMemo( + () => searchParams.get('redirect') || searchParams.get('callbackUrl') || '', + [searchParams] + ) + const isInviteFlow = useMemo( + () => + searchParams.get('invite_flow') === 'true' || + redirectUrl.startsWith('/invite/') || + redirectUrl.startsWith('/credential-account/'), + [searchParams, redirectUrl] + ) + const [name, setName] = useState('') const [nameErrors, setNameErrors] = useState([]) const [showNameValidationError, setShowNameValidationError] = useState(false) - useEffect(() => { - setMounted(true) - const emailParam = searchParams.get('email') - if (emailParam) { - setEmail(emailParam) - } - - // Check both 'redirect' and 'callbackUrl' params (login page uses callbackUrl) - const redirectParam = searchParams.get('redirect') || searchParams.get('callbackUrl') - if (redirectParam) { - setRedirectUrl(redirectParam) - - if ( - redirectParam.startsWith('/invite/') || - redirectParam.startsWith('/credential-account/') - ) { - setIsInviteFlow(true) - } - } - - const inviteFlowParam = searchParams.get('invite_flow') - if (inviteFlowParam === 'true') { - setIsInviteFlow(true) - } - }, [searchParams]) - const validatePassword = (passwordValue: string): string[] => { const errors: string[] = [] diff --git a/apps/sim/app/chat/components/input/input.tsx b/apps/sim/app/chat/components/input/input.tsx index 5c9bfea95b..25402cf475 100644 --- a/apps/sim/app/chat/components/input/input.tsx +++ b/apps/sim/app/chat/components/input/input.tsx @@ -71,11 +71,6 @@ export const ChatInput: React.FC<{ } } - // Adjust height on input change - useEffect(() => { - adjustTextareaHeight() - }, [inputValue]) - // Close the input when clicking outside (only when empty) useEffect(() => { const handleClickOutside = (event: MouseEvent) => { @@ -94,17 +89,14 @@ export const ChatInput: React.FC<{ return () => document.removeEventListener('mousedown', handleClickOutside) }, [inputValue]) - // Handle focus and initial height when activated - useEffect(() => { - if (isActive && textareaRef.current) { - textareaRef.current.focus() - adjustTextareaHeight() // Adjust height when becoming active - } - }, [isActive]) - const handleActivate = () => { setIsActive(true) - // Focus is now handled by the useEffect above + requestAnimationFrame(() => { + if (textareaRef.current) { + textareaRef.current.focus() + adjustTextareaHeight() + } + }) } // Handle file selection @@ -186,6 +178,7 @@ export const ChatInput: React.FC<{ const handleInputChange = (e: React.ChangeEvent) => { setInputValue(e.target.value) + adjustTextareaHeight() } // Handle voice start with smooth transition to voice-first mode diff --git a/apps/sim/app/chat/components/voice-interface/voice-interface.tsx b/apps/sim/app/chat/components/voice-interface/voice-interface.tsx index fd7f291c31..9c9cc26539 100644 --- a/apps/sim/app/chat/components/voice-interface/voice-interface.tsx +++ b/apps/sim/app/chat/components/voice-interface/voice-interface.tsx @@ -78,9 +78,10 @@ export function VoiceInterface({ const currentStateRef = useRef<'idle' | 'listening' | 'agent_speaking'>('idle') const isCallEndedRef = useRef(false) - useEffect(() => { - currentStateRef.current = state - }, [state]) + const updateState = useCallback((next: 'idle' | 'listening' | 'agent_speaking') => { + setState(next) + currentStateRef.current = next + }, []) const recognitionRef = useRef(null) const mediaStreamRef = useRef(null) @@ -97,9 +98,10 @@ export function VoiceInterface({ (window as WindowWithSpeech).webkitSpeechRecognition ) - useEffect(() => { - isMutedRef.current = isMuted - }, [isMuted]) + const updateIsMuted = useCallback((next: boolean) => { + setIsMuted(next) + isMutedRef.current = next + }, []) const setResponseTimeout = useCallback(() => { if (responseTimeoutRef.current) { @@ -108,7 +110,7 @@ export function VoiceInterface({ responseTimeoutRef.current = setTimeout(() => { if (currentStateRef.current === 'listening') { - setState('idle') + updateState('idle') } }, 5000) }, []) @@ -123,10 +125,10 @@ export function VoiceInterface({ useEffect(() => { if (isPlayingAudio && state !== 'agent_speaking') { clearResponseTimeout() - setState('agent_speaking') + updateState('agent_speaking') setCurrentTranscript('') - setIsMuted(true) + updateIsMuted(true) if (mediaStreamRef.current) { mediaStreamRef.current.getAudioTracks().forEach((track) => { track.enabled = false @@ -141,17 +143,17 @@ export function VoiceInterface({ } } } else if (!isPlayingAudio && state === 'agent_speaking') { - setState('idle') + updateState('idle') setCurrentTranscript('') - setIsMuted(false) + updateIsMuted(false) if (mediaStreamRef.current) { mediaStreamRef.current.getAudioTracks().forEach((track) => { track.enabled = true }) } } - }, [isPlayingAudio, state, clearResponseTimeout]) + }, [isPlayingAudio, state, clearResponseTimeout, updateState, updateIsMuted]) const setupAudio = useCallback(async () => { try { @@ -310,7 +312,7 @@ export function VoiceInterface({ return } - setState('listening') + updateState('listening') setCurrentTranscript('') if (recognitionRef.current) { @@ -320,10 +322,10 @@ export function VoiceInterface({ logger.error('Error starting recognition:', error) } } - }, [isInitialized, isMuted, state]) + }, [isInitialized, isMuted, state, updateState]) const stopListening = useCallback(() => { - setState('idle') + updateState('idle') setCurrentTranscript('') if (recognitionRef.current) { @@ -333,15 +335,15 @@ export function VoiceInterface({ // Ignore } } - }, []) + }, [updateState]) const handleInterrupt = useCallback(() => { if (state === 'agent_speaking') { onInterrupt?.() - setState('listening') + updateState('listening') setCurrentTranscript('') - setIsMuted(false) + updateIsMuted(false) if (mediaStreamRef.current) { mediaStreamRef.current.getAudioTracks().forEach((track) => { track.enabled = true @@ -356,14 +358,14 @@ export function VoiceInterface({ } } } - }, [state, onInterrupt]) + }, [state, onInterrupt, updateState, updateIsMuted]) const handleCallEnd = useCallback(() => { isCallEndedRef.current = true - setState('idle') + updateState('idle') setCurrentTranscript('') - setIsMuted(false) + updateIsMuted(false) if (recognitionRef.current) { try { @@ -376,7 +378,7 @@ export function VoiceInterface({ clearResponseTimeout() onInterrupt?.() onCallEnd?.() - }, [onCallEnd, onInterrupt, clearResponseTimeout]) + }, [onCallEnd, onInterrupt, clearResponseTimeout, updateState, updateIsMuted]) useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { @@ -397,7 +399,7 @@ export function VoiceInterface({ } const newMutedState = !isMuted - setIsMuted(newMutedState) + updateIsMuted(newMutedState) if (mediaStreamRef.current) { mediaStreamRef.current.getAudioTracks().forEach((track) => { @@ -410,7 +412,7 @@ export function VoiceInterface({ } else if (state === 'idle') { startListening() } - }, [isMuted, state, handleInterrupt, stopListening, startListening]) + }, [isMuted, state, handleInterrupt, stopListening, startListening, updateIsMuted]) useEffect(() => { if (isSupported) { diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-documents-modal/add-documents-modal.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-documents-modal/add-documents-modal.tsx index 56463b60f7..61b6f258fb 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-documents-modal/add-documents-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-documents-modal/add-documents-modal.tsx @@ -1,6 +1,6 @@ 'use client' -import { useEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import { createLogger } from '@sim/logger' import { Loader2, RotateCcw, X } from 'lucide-react' import { useParams } from 'next/navigation' @@ -75,15 +75,25 @@ export function AddDocumentsModal({ } }, [open, clearError]) + /** Handles close with upload guard */ + const handleOpenChange = useCallback( + (newOpen: boolean) => { + if (!newOpen) { + if (isUploading) return + setFiles([]) + setFileError(null) + clearError() + setIsDragging(false) + setDragCounter(0) + setRetryingIndexes(new Set()) + } + onOpenChange(newOpen) + }, + [isUploading, clearError, onOpenChange] + ) + const handleClose = () => { - if (isUploading) return - setFiles([]) - setFileError(null) - clearError() - setIsDragging(false) - setDragCounter(0) - setRetryingIndexes(new Set()) - onOpenChange(false) + handleOpenChange(false) } const processFiles = async (fileList: FileList | File[]) => { @@ -220,7 +230,7 @@ export function AddDocumentsModal({ } return ( - + New Documents diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/credentials/credentials-manager.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/credentials/credentials-manager.tsx index 15ff5a1907..ee553726c1 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/credentials/credentials-manager.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/credentials/credentials-manager.tsx @@ -494,13 +494,12 @@ export function CredentialsManager() { }, [variables]) useEffect(() => { - if (workspaceEnvData) { - if (hasSavedRef.current) { - hasSavedRef.current = false - } else { - setWorkspaceVars(workspaceEnvData?.workspace || {}) - initialWorkspaceVarsRef.current = workspaceEnvData?.workspace || {} - } + if (!workspaceEnvData) return + if (hasSavedRef.current) { + hasSavedRef.current = false + } else { + setWorkspaceVars(workspaceEnvData.workspace || {}) + initialWorkspaceVarsRef.current = workspaceEnvData.workspace || {} } }, [workspaceEnvData]) diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/help-modal/help-modal.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/help-modal/help-modal.tsx index a51cef4be2..c017e6162e 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/help-modal/help-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/help-modal/help-modal.tsx @@ -89,21 +89,25 @@ export function HelpModal({ open, onOpenChange, workflowId, workspaceId }: HelpM }) /** - * Reset all state when modal opens/closes + * Reset all form and UI state to prepare for a fresh modal session */ + const resetModalState = useCallback(() => { + setSubmitStatus(null) + setImages([]) + setIsDragging(false) + setIsProcessing(false) + reset({ + subject: '', + message: '', + type: DEFAULT_REQUEST_TYPE, + }) + }, [reset]) + useEffect(() => { if (open) { - setSubmitStatus(null) - setImages([]) - setIsDragging(false) - setIsProcessing(false) - reset({ - subject: '', - message: '', - type: DEFAULT_REQUEST_TYPE, - }) + resetModalState() } - }, [open, reset]) + }, [open, resetModalState]) /** * Fix z-index for popover/dropdown when inside modal diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.tsx index a6b8d5df91..ae179d5d79 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.tsx @@ -1,6 +1,6 @@ 'use client' -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { type RefObject, useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Button, DropdownMenu, @@ -39,29 +39,27 @@ function ColorGrid({ hexInput, setHexInput, onColorChange, - isOpen, + buttonRefs, }: { hexInput: string setHexInput: (color: string) => void onColorChange?: (color: string) => void - isOpen: boolean + buttonRefs: RefObject<(HTMLButtonElement | null)[]> }) { const [focusedIndex, setFocusedIndex] = useState(-1) const gridRef = useRef(null) - const buttonRefs = useRef<(HTMLButtonElement | null)[]>([]) useEffect(() => { - if (isOpen && gridRef.current) { - const selectedIndex = WORKFLOW_COLORS.findIndex( - ({ color }) => color.toLowerCase() === hexInput.toLowerCase() - ) - const initialIndex = selectedIndex >= 0 ? selectedIndex : 0 - setFocusedIndex(initialIndex) - setTimeout(() => { - buttonRefs.current[initialIndex]?.focus() - }, 50) - } - }, [isOpen, hexInput]) + const selectedIndex = WORKFLOW_COLORS.findIndex( + ({ color }) => color.toLowerCase() === hexInput.toLowerCase() + ) + const idx = selectedIndex >= 0 ? selectedIndex : 0 + setFocusedIndex(idx) + requestAnimationFrame(() => { + buttonRefs.current[idx]?.focus() + }) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []) const handleKeyDown = useCallback( (e: React.KeyboardEvent, index: number) => { @@ -176,10 +174,10 @@ function ColorPickerSubmenu({ handleHexFocus: (e: React.FocusEvent) => void disabled?: boolean }) { - const [isSubOpen, setIsSubOpen] = useState(false) + const buttonRefs = useRef<(HTMLButtonElement | null)[]>([]) return ( - + Change color @@ -190,7 +188,7 @@ function ColorPickerSubmenu({ hexInput={hexInput} setHexInput={setHexInput} onColorChange={onColorChange} - isOpen={isSubOpen} + buttonRefs={buttonRefs} />
e.preventDefault()} > {showOpenInNewTab && onOpenInNewTab && ( diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/create-workspace-modal/create-workspace-modal.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/create-workspace-modal/create-workspace-modal.tsx index 1f775d13b3..5193680eb8 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/create-workspace-modal/create-workspace-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/create-workspace-modal/create-workspace-modal.tsx @@ -1,6 +1,6 @@ 'use client' -import { useCallback, useEffect, useRef, useState } from 'react' +import { useEffect, useRef, useState } from 'react' import { Button, Input, @@ -33,29 +33,31 @@ export function CreateWorkspaceModal({ useEffect(() => { if (open) { setName('') - requestAnimationFrame(() => inputRef.current?.focus()) } }, [open]) - const handleSubmit = useCallback(async () => { + const handleSubmit = async () => { const trimmed = name.trim() if (!trimmed || isCreating) return await onConfirm(trimmed) - }, [name, isCreating, onConfirm]) + } - const handleKeyDown = useCallback( - (e: React.KeyboardEvent) => { - if (e.key === 'Enter') { - e.preventDefault() - void handleSubmit() - } - }, - [handleSubmit] - ) + const handleKeyDown = (e: React.KeyboardEvent) => { + if (e.key === 'Enter') { + e.preventDefault() + void handleSubmit() + } + } return ( - + { + e.preventDefault() + inputRef.current?.focus() + }} + > Create Workspace { const { data: session } = useSession() const userPerms = useUserPermissionsContext() - const [hasLoadedOnce, setHasLoadedOnce] = useState(false) + const hasLoadedOnceRef = useRef(false) - useEffect(() => { - if (!permissionsLoading && !userPerms.isLoading && !isPendingInvitationsLoading) { - setHasLoadedOnce(true) - } - }, [permissionsLoading, userPerms.isLoading, isPendingInvitationsLoading]) + if ( + !hasLoadedOnceRef.current && + !permissionsLoading && + !userPerms.isLoading && + !isPendingInvitationsLoading + ) { + hasLoadedOnceRef.current = true + } + + const hasLoadedOnce = hasLoadedOnceRef.current const existingUsers: UserPermissions[] = useMemo( () => diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx index 1dc9064122..ae32f81e32 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx @@ -591,10 +591,15 @@ export const Sidebar = memo(function Sidebar() { id: 'settings', label: 'Settings', icon: Settings, - onClick: () => navigateToSettings(), + onClick: () => { + if (!isCollapsed) { + setSidebarWidth(SIDEBAR_WIDTH.MIN) + } + navigateToSettings() + }, }, ], - [workspaceId, navigateToSettings] + [workspaceId, navigateToSettings, isCollapsed, setSidebarWidth] ) const { data: fetchedTasks = [], isLoading: tasksLoading } = useTasks(workspaceId) @@ -636,6 +641,16 @@ export const Sidebar = memo(function Sidebar() { setIsTaskDeleteModalOpen(true) }, [tasks]) + const navigateToPage = useCallback( + (path: string) => { + if (!isCollapsed) { + setSidebarWidth(SIDEBAR_WIDTH.MIN) + } + router.push(path) + }, + [isCollapsed, setSidebarWidth, router] + ) + const handleConfirmDeleteTasks = useCallback(() => { const { taskIds: taskIdsToDelete } = contextMenuSelectionRef.current if (taskIdsToDelete.length === 0) return @@ -648,7 +663,7 @@ export const Sidebar = memo(function Sidebar() { const onDeleteSuccess = () => { useFolderStore.getState().clearTaskSelection() if (isViewingDeletedTask) { - router.push(`/workspace/${workspaceId}/home`) + navigateToPage(`/workspace/${workspaceId}/home`) } } @@ -658,7 +673,7 @@ export const Sidebar = memo(function Sidebar() { deleteTasksMutation.mutate(taskIdsToDelete, { onSuccess: onDeleteSuccess }) } setIsTaskDeleteModalOpen(false) - }, [pathname, workspaceId, deleteTaskMutation, deleteTasksMutation, router]) + }, [pathname, workspaceId, deleteTaskMutation, deleteTasksMutation, navigateToPage]) const [visibleTaskCount, setVisibleTaskCount] = useState(5) const [renamingTaskId, setRenamingTaskId] = useState(null) @@ -910,7 +925,7 @@ export const Sidebar = memo(function Sidebar() { try { const pathWorkspaceId = resolveWorkspaceIdFromPath() if (pathWorkspaceId) { - router.push(`/workspace/${pathWorkspaceId}/templates`) + navigateToPage(`/workspace/${pathWorkspaceId}/templates`) logger.info('Navigated to templates', { workspaceId: pathWorkspaceId }) } else { logger.warn('No workspace ID found, cannot navigate to templates') @@ -926,7 +941,7 @@ export const Sidebar = memo(function Sidebar() { try { const pathWorkspaceId = resolveWorkspaceIdFromPath() if (pathWorkspaceId) { - router.push(`/workspace/${pathWorkspaceId}/logs`) + navigateToPage(`/workspace/${pathWorkspaceId}/logs`) logger.info('Navigated to logs', { workspaceId: pathWorkspaceId }) } else { logger.warn('No workspace ID found, cannot navigate to logs') @@ -1113,7 +1128,7 @@ export const Sidebar = memo(function Sidebar() { @@ -1131,7 +1146,7 @@ export const Sidebar = memo(function Sidebar() { } hover={tasksHover} - onClick={() => router.push(`/workspace/${workspaceId}/home`)} + onClick={() => navigateToPage(`/workspace/${workspaceId}/home`)} ariaLabel='Tasks' className='mt-[6px]' > diff --git a/apps/sim/components/emcn/components/combobox/combobox.tsx b/apps/sim/components/emcn/components/combobox/combobox.tsx index 4b922ae811..e9cbeebd85 100644 --- a/apps/sim/components/emcn/components/combobox/combobox.tsx +++ b/apps/sim/components/emcn/components/combobox/combobox.tsx @@ -462,13 +462,25 @@ const Combobox = memo( [disabled, editable, inputRef] ) + const effectiveHighlightedIndex = + highlightedIndex >= 0 && highlightedIndex < filteredOptions.length ? highlightedIndex : -1 + + /** + * Reset highlighted index when filtered options change and index is out of bounds + */ + useEffect(() => { + if (highlightedIndex >= 0 && highlightedIndex >= filteredOptions.length) { + setHighlightedIndex(-1) + } + }, [filteredOptions, highlightedIndex]) + /** * Scroll highlighted option into view */ useEffect(() => { - if (highlightedIndex >= 0 && dropdownRef.current) { + if (effectiveHighlightedIndex >= 0 && dropdownRef.current) { const highlightedElement = dropdownRef.current.querySelector( - `[data-option-index="${highlightedIndex}"]` + `[data-option-index="${effectiveHighlightedIndex}"]` ) if (highlightedElement) { highlightedElement.scrollIntoView({ @@ -477,19 +489,7 @@ const Combobox = memo( }) } } - }, [highlightedIndex]) - - /** - * Adjust highlighted index when filtered options change - */ - useEffect(() => { - setHighlightedIndex((prev) => { - if (prev >= 0 && prev < filteredOptions.length) { - return prev - } - return -1 - }) - }, [filteredOptions]) + }, [effectiveHighlightedIndex]) const SelectedIcon = selectedOption?.icon @@ -713,7 +713,7 @@ const Combobox = memo( const globalIndex = filteredOptions.findIndex( (o) => o.value === option.value ) - const isHighlighted = globalIndex === highlightedIndex + const isHighlighted = globalIndex === effectiveHighlightedIndex const OptionIcon = option.icon return ( @@ -789,7 +789,7 @@ const Combobox = memo( const isSelected = multiSelect ? multiSelectValues?.includes(option.value) : effectiveSelectedValue === option.value - const isHighlighted = index === highlightedIndex + const isHighlighted = index === effectiveHighlightedIndex const OptionIcon = option.icon return ( diff --git a/apps/sim/components/emcn/components/date-picker/date-picker.tsx b/apps/sim/components/emcn/components/date-picker/date-picker.tsx index 67fa14d273..0a597dec58 100644 --- a/apps/sim/components/emcn/components/date-picker/date-picker.tsx +++ b/apps/sim/components/emcn/components/date-picker/date-picker.tsx @@ -559,12 +559,15 @@ const DatePicker = React.forwardRef((props, ref } }, [open, isRangeMode, initialStart, initialEnd]) - React.useEffect(() => { - if (!isRangeMode && selectedDate) { + const singleValueKey = !isRangeMode && selectedDate ? selectedDate.getTime() : undefined + const [prevSingleValueKey, setPrevSingleValueKey] = React.useState(singleValueKey) + if (singleValueKey !== prevSingleValueKey) { + setPrevSingleValueKey(singleValueKey) + if (selectedDate) { setViewMonth(selectedDate.getMonth()) setViewYear(selectedDate.getFullYear()) } - }, [isRangeMode, selectedDate]) + } /** * Handles selection of a specific day in single mode. diff --git a/apps/sim/components/emcn/components/popover/popover.tsx b/apps/sim/components/emcn/components/popover/popover.tsx index 561f041a6c..8702c41a5a 100644 --- a/apps/sim/components/emcn/components/popover/popover.tsx +++ b/apps/sim/components/emcn/components/popover/popover.tsx @@ -226,6 +226,7 @@ const Popover: React.FC = ({ size = 'md', colorScheme = 'default', open, + onOpenChange, ...props }) => { const [currentFolder, setCurrentFolder] = React.useState(null) @@ -251,21 +252,33 @@ const Popover: React.FC = ({ } }, []) + /** Resets all navigation state to initial values */ + const resetState = React.useCallback(() => { + setCurrentFolder(null) + setFolderTitle(null) + setOnFolderSelect(null) + setSearchQuery('') + setLastHoveredItem(null) + setIsKeyboardNav(false) + setSelectedIndex(-1) + registeredItemsRef.current = [] + }, []) + React.useEffect(() => { - if (open === false) { - setCurrentFolder(null) - setFolderTitle(null) - setOnFolderSelect(null) - setSearchQuery('') - setLastHoveredItem(null) - setIsKeyboardNav(false) - setSelectedIndex(-1) - registeredItemsRef.current = [] - } else { - // Reset hover state when opening to prevent stale submenu from previous menu - setLastHoveredItem(null) + if (!open) { + resetState() } - }, [open]) + }, [open, resetState]) + + const handleOpenChange = React.useCallback( + (nextOpen: boolean) => { + if (nextOpen) { + setLastHoveredItem(null) + } + onOpenChange?.(nextOpen) + }, + [onOpenChange] + ) const openFolder = React.useCallback( (id: string, title: string, onLoad?: () => void | Promise, onSelect?: () => void) => { @@ -336,7 +349,7 @@ const Popover: React.FC = ({ return ( - + {children} diff --git a/apps/sim/components/emcn/components/time-picker/time-picker.tsx b/apps/sim/components/emcn/components/time-picker/time-picker.tsx index 1bd45418b1..4bc776b347 100644 --- a/apps/sim/components/emcn/components/time-picker/time-picker.tsx +++ b/apps/sim/components/emcn/components/time-picker/time-picker.tsx @@ -135,13 +135,15 @@ const TimePicker = React.forwardRef( const [hour, setHour] = React.useState(parsed.hour) const [minute, setMinute] = React.useState(parsed.minute) const [ampm, setAmpm] = React.useState<'AM' | 'PM'>(parsed.ampm) + const [prevValue, setPrevValue] = React.useState(value) - React.useEffect(() => { + if (value !== prevValue) { + setPrevValue(value) const newParsed = parseTime(value || '') setHour(newParsed.hour) setMinute(newParsed.minute) setAmpm(newParsed.ampm) - }, [value]) + } React.useEffect(() => { if (open) {