From a86b64521611bf41827451a4fd0170fdc7441bc0 Mon Sep 17 00:00:00 2001 From: Fredrik Rombach Ekelund Date: Tue, 14 Jan 2025 15:50:24 +0100 Subject: [PATCH 1/3] Assistant: Fix scroll glitch on initial render --- src/components/ai-clear-history-reminder.tsx | 37 ++++++++------------ src/components/content-tab-assistant.tsx | 12 ++++--- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/components/ai-clear-history-reminder.tsx b/src/components/ai-clear-history-reminder.tsx index 8d350a033..db7206927 100644 --- a/src/components/ai-clear-history-reminder.tsx +++ b/src/components/ai-clear-history-reminder.tsx @@ -6,6 +6,13 @@ import { Message as MessageType } from '../hooks/use-assistant'; import { getIpcApi } from '../lib/get-ipc-api'; import Button from './button'; +function shouldShowReminder( lastMessage?: MessageType ) { + return ( + lastMessage?.role === 'assistant' && + Date.now() - lastMessage?.createdAt > CLEAR_HISTORY_REMINDER_TIME + ); +} + function AIClearHistoryReminder( { lastMessage, clearConversation, @@ -13,36 +20,22 @@ function AIClearHistoryReminder( { lastMessage?: MessageType; clearConversation: () => void; } ) { - const [ showReminder, setShowReminder ] = useState( false ); - const timeoutId = useRef< NodeJS.Timeout >(); - const currentMessage = useRef< number >(); + const [ showReminder, setShowReminder ] = useState( shouldShowReminder( lastMessage ) ); const elementRef = useRef< HTMLDivElement >( null ); useEffect( () => { - if ( ! lastMessage || lastMessage.role === 'user' ) { - clearTimeout( timeoutId.current ); - setShowReminder( false ); - return; - } - - if ( lastMessage.id === currentMessage.current ) { - return; - } + let timeoutId: NodeJS.Timeout; + const nextValue = shouldShowReminder( lastMessage ); + setShowReminder( nextValue ); - const messageTime = Date.now() - lastMessage?.createdAt; - const timeToRemind = CLEAR_HISTORY_REMINDER_TIME - messageTime; - if ( timeToRemind > 0 ) { - clearTimeout( timeoutId.current ); - setShowReminder( false ); - timeoutId.current = setTimeout( () => { + if ( nextValue && lastMessage ) { + timeoutId = setTimeout( () => { setShowReminder( true ); - }, timeToRemind ); - } else { - setShowReminder( true ); + }, Date.now() - lastMessage.createdAt ); } return () => { - clearTimeout( timeoutId.current ); + clearTimeout( timeoutId ); }; }, [ lastMessage ] ); diff --git a/src/components/content-tab-assistant.tsx b/src/components/content-tab-assistant.tsx index b6747c9d5..74da79949 100644 --- a/src/components/content-tab-assistant.tsx +++ b/src/components/content-tab-assistant.tsx @@ -112,6 +112,7 @@ interface AuthenticatedViewProps { markMessageAsFeedbackReceived: ReturnType< typeof useAssistant >[ 'markMessageAsFeedbackReceived' ]; + wrapperRef: React.RefObject< HTMLDivElement >; } const AuthenticatedView = memo( @@ -122,8 +123,8 @@ const AuthenticatedView = memo( siteId, submitPrompt, markMessageAsFeedbackReceived, + wrapperRef, }: AuthenticatedViewProps ) => { - const endOfMessagesRef = useRef< HTMLDivElement >( null ); const lastMessageRef = useRef< HTMLDivElement >( null ); const [ showThinking, setShowThinking ] = useState( isAssistantThinking ); const lastMessage = useMemo( @@ -151,7 +152,7 @@ const AuthenticatedView = memo( let timer: NodeJS.Timeout; // Scroll to the end of the messages when the tab is opened or site ID changes if ( isInitialRenderRef.current ) { - endOfMessagesRef.current?.scrollIntoView( { behavior: 'instant' } ); + wrapperRef.current?.scrollIntoView( { block: 'end', behavior: 'instant' } ); isInitialRenderRef.current = false; } // Scroll when a new message is added @@ -166,7 +167,7 @@ const AuthenticatedView = memo( } // For user messages, scroll to the end of the messages else { - endOfMessagesRef.current?.scrollIntoView( { behavior: 'smooth' } ); + wrapperRef.current?.scrollIntoView( { block: 'end', behavior: 'smooth' } ); } } @@ -300,7 +301,6 @@ const AuthenticatedView = memo( ) } -
); } @@ -351,6 +351,7 @@ const UnauthenticatedView = ( { onAuthenticate }: { onAuthenticate: () => void } export function ContentTabAssistant( { selectedSite }: ContentTabAssistantProps ) { const inputRef = useRef< HTMLTextAreaElement >( null ); + const wrapperRef = useRef< HTMLDivElement >( null ); const chatContext = useChatContext(); const { isAuthenticated, authenticate, user } = useAuth(); const instanceId = user?.id ? `${ user.id }_${ selectedSite.id }` : selectedSite.id; @@ -467,7 +468,7 @@ export function ContentTabAssistant( { selectedSite }: ContentTabAssistantProps const disabled = isOffline || ! isAuthenticated || ! userCanSendMessage || hasFailedMessage; return ( -
+
) : ( From be133992a2ab292724a53377868b50e98f0523d0 Mon Sep 17 00:00:00 2001 From: Fredrik Rombach Ekelund Date: Tue, 14 Jan 2025 16:55:48 +0100 Subject: [PATCH 2/3] Fix tests --- src/components/ai-clear-history-reminder.tsx | 2 +- .../tests/ai-clear-history-reminder.test.tsx | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/components/ai-clear-history-reminder.tsx b/src/components/ai-clear-history-reminder.tsx index db7206927..3bf2a2a43 100644 --- a/src/components/ai-clear-history-reminder.tsx +++ b/src/components/ai-clear-history-reminder.tsx @@ -9,7 +9,7 @@ import Button from './button'; function shouldShowReminder( lastMessage?: MessageType ) { return ( lastMessage?.role === 'assistant' && - Date.now() - lastMessage?.createdAt > CLEAR_HISTORY_REMINDER_TIME + Date.now() - ( lastMessage?.createdAt ?? 0 ) > CLEAR_HISTORY_REMINDER_TIME ); } diff --git a/src/components/tests/ai-clear-history-reminder.test.tsx b/src/components/tests/ai-clear-history-reminder.test.tsx index 818f658bd..ba5ce59e1 100644 --- a/src/components/tests/ai-clear-history-reminder.test.tsx +++ b/src/components/tests/ai-clear-history-reminder.test.tsx @@ -1,4 +1,5 @@ -import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import { CLEAR_HISTORY_REMINDER_TIME } from '../../constants'; import { getIpcApi } from '../../lib/get-ipc-api'; import AIClearHistoryReminder from '../ai-clear-history-reminder'; import type { Message } from '../../hooks/use-assistant'; @@ -7,15 +8,15 @@ jest.mock( '../../lib/get-ipc-api' ); describe( 'AIClearHistoryReminder', () => { let clearConversation: jest.Mock; - const MOCKED_TIME = 1718882159928; - const TWO_HOURS_DIFF = 2 * 60 * 60 * 1000; + const MOCKED_CURRENT_TIME = 1718882159928; + const OLD_MESSAGE_TIME = MOCKED_CURRENT_TIME - CLEAR_HISTORY_REMINDER_TIME - 1; beforeEach( () => { window.HTMLElement.prototype.scrollIntoView = jest.fn(); clearConversation = jest.fn(); jest.clearAllMocks(); jest.useFakeTimers(); - jest.setSystemTime( MOCKED_TIME ); + jest.setSystemTime( MOCKED_CURRENT_TIME ); } ); afterEach( () => { @@ -25,7 +26,7 @@ describe( 'AIClearHistoryReminder', () => { it( 'should display a reminder when the conversation is stale', () => { const message: Message = { id: 0, - createdAt: MOCKED_TIME - TWO_HOURS_DIFF, + createdAt: OLD_MESSAGE_TIME, content: '', role: 'assistant', }; @@ -42,7 +43,7 @@ describe( 'AIClearHistoryReminder', () => { } ); const message: Message = { id: 0, - createdAt: MOCKED_TIME - TWO_HOURS_DIFF, + createdAt: OLD_MESSAGE_TIME, content: '', role: 'assistant', }; @@ -65,7 +66,7 @@ describe( 'AIClearHistoryReminder', () => { } ); const message: Message = { id: 0, - createdAt: MOCKED_TIME - TWO_HOURS_DIFF, + createdAt: OLD_MESSAGE_TIME, content: '', role: 'assistant', }; From 8dc1bad0a6d6abb5cad70a121ce1182da65ecc56 Mon Sep 17 00:00:00 2001 From: Fredrik Rombach Ekelund Date: Wed, 15 Jan 2025 09:22:11 +0100 Subject: [PATCH 3/3] Fixed last test --- src/components/tests/content-tab-assistant.test.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/tests/content-tab-assistant.test.tsx b/src/components/tests/content-tab-assistant.test.tsx index 07a597278..4ac17b186 100644 --- a/src/components/tests/content-tab-assistant.test.tsx +++ b/src/components/tests/content-tab-assistant.test.tsx @@ -1,5 +1,6 @@ import { render, screen, fireEvent, waitFor, act } from '@testing-library/react'; import { userEvent } from '@testing-library/user-event'; +import { CLEAR_HISTORY_REMINDER_TIME } from '../../constants'; import { useAuth } from '../../hooks/use-auth'; import { ChatProvider } from '../../hooks/use-chat-context'; import { useGetWpVersion } from '../../hooks/use-get-wp-version'; @@ -338,10 +339,10 @@ describe( 'ContentTabAssistant', () => { } ); test( 'clears history via reminder when last message is two hours old', async () => { - const MOCKED_TIME = 1718882159928; - const TWO_HOURS_DIFF = 2 * 60 * 60 * 1000; + const MOCKED_CURRENT_TIME = 1718882159928; + const OLD_MESSAGE_TIME = MOCKED_CURRENT_TIME - CLEAR_HISTORY_REMINDER_TIME - 1; jest.useFakeTimers(); - jest.setSystemTime( MOCKED_TIME ); + jest.setSystemTime( MOCKED_CURRENT_TIME ); const storageKey = 'ai_chat_messages'; localStorage.setItem( @@ -353,7 +354,7 @@ describe( 'ContentTabAssistant', () => { id: 1, content: 'Initial message 2', role: 'assistant', - createdAt: MOCKED_TIME - TWO_HOURS_DIFF, + createdAt: OLD_MESSAGE_TIME, }, ], } )