Skip to content

Fix chat scroll and charts message persistance #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
+62 −29
Draft

Fix chat scroll and charts message persistance #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3277c7
chore: fix chat scroll
epetrow Nov 27, 2025
bfd2368
chore: fix chat charts persistance
epetrow Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions client/src/hooks/useChatBot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface StreamingResponse {
json?: Record<string, unknown>; // For chart data or other structured responses
error?: string;
incomplete?: boolean;
messageId?: string | number; // ID of the message associated with this response
}

export interface UseChatBotReturn {
Expand Down Expand Up @@ -174,25 +175,27 @@ export const useChatBot = (config: ChatBotConfig): UseChatBotReturn => {
setMessages(prev => prev.filter(msg => msg.id !== typingMessageId));

// Then add complete response after a tiny delay
const botMessageId = Date.now() + 2;
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Date.now() + 2 for message ID generation can lead to collisions if multiple messages are created in rapid succession. Consider using a more robust ID generation strategy like incrementing a counter or a UUID library.

Copilot uses AI. Check for mistakes.
setTimeout(() => {
const botMessage: Message = {
id: Date.now() + 2,
id: botMessageId,
author: bot,
timestamp: new Date(),
text: currentAnswer
};

setMessages(prev => [...prev, botMessage]);
}, 10);

// Set the final response with the message ID for any additional processing
if (finalResponse) {
finalResponse.messageId = botMessageId;
setLatestResponse(finalResponse);
}
} else {
// If no response, just remove typing indicator
setMessages(prev => prev.filter(msg => msg.id !== typingMessageId));
}

// Set the final response for any additional processing
if (finalResponse) {
setLatestResponse(finalResponse);
}
} catch (error) {
const errorMessage = error instanceof Error ? error.message : 'Network error';

Expand Down
43 changes: 29 additions & 14 deletions client/src/pages/FinanceAnalysis.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ export default function FinanceAnalysis() {

const [selectedCharts, setSelectedCharts] = React.useState<BarChartDef[]>([]);
const [isChartsExpanded, setIsChartsExpanded] = React.useState(false);
// Map message IDs to their associated charts
const [messageCharts, setMessageCharts] = React.useState<Map<string | number, BarChartDef[]>>(new Map());

// Predefined suggestions related to financial data
const financialSuggestions: ChatSuggestion[] = [
Expand Down Expand Up @@ -178,36 +180,48 @@ export default function FinanceAnalysis() {
suggestions: financialSuggestions,
});

// Watch for changes in the latest response to update charts
// Associate charts with the latest bot message when response arrives
React.useEffect(() => {
if (
chatBot.latestResponse?.json?.charts &&
Array.isArray(chatBot.latestResponse.json.charts)
Array.isArray(chatBot.latestResponse.json.charts) &&
chatBot.latestResponse.messageId
) {
const validCharts: BarChartDef[] = chatBot.latestResponse.json.charts
.filter(isBarChartDef)
.slice(0, 3);
setSelectedCharts(validCharts);
} else {
setSelectedCharts([]);

if (validCharts.length > 0) {
setMessageCharts(prev => {
const newMap = new Map(prev);
newMap.set(chatBot.latestResponse!.messageId!, validCharts);
Comment on lines +195 to +197
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using non-null assertions (!) on potentially undefined values is unsafe. While the outer condition checks for messageId, the non-null assertion could mask potential runtime errors. Consider using optional chaining or a guard clause instead.

Suggested change
setMessageCharts(prev => {
const newMap = new Map(prev);
newMap.set(chatBot.latestResponse!.messageId!, validCharts);
const messageId = chatBot.latestResponse.messageId;
setMessageCharts(prev => {
const newMap = new Map(prev);
if (messageId) {
newMap.set(messageId, validCharts);
}

Copilot uses AI. Check for mistakes.
return newMap;
});
setSelectedCharts(validCharts);
}
}
}, [chatBot.latestResponse, isBarChartDef]);

// Custom message template that includes thumbnail when charts are available
const customMessageTemplate = React.useCallback((props: ChatMessageTemplateProps) => {
const isBot = props.item.author.id !== chatBot.user.id;
const isLatestBotMessage = isBot && props.item.id === chatBot.messages[chatBot.messages.length - 1]?.id;
const hasCharts = selectedCharts.length > 0;
const chartsForThisMessage = messageCharts.get(props.item.id);

return (
<div>
<ChatMessage {...props} />
{isLatestBotMessage && hasCharts && (
<ChartThumbnail onClick={() => setIsChartsExpanded(true)} />
{isBot && chartsForThisMessage && chartsForThisMessage.length > 0 && (
<ChartThumbnail onClick={() => {
const charts = messageCharts.get(props.item.id);
if (charts) {
setSelectedCharts(charts);
setIsChartsExpanded(true);
}
Comment on lines +215 to +219
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chart retrieval logic is duplicated - it's already retrieved as chartsForThisMessage on line 208. Consider reusing that variable instead of calling messageCharts.get(props.item.id) again.

Copilot uses AI. Check for mistakes.
}} />
)}
</div>
);
}, [chatBot.user.id, chatBot.messages, selectedCharts]);
}, [chatBot.user.id, messageCharts]);

// Memoized callback for sending messages
const handleSendMessage = React.useCallback((text: string) => {
Expand Down Expand Up @@ -285,9 +299,10 @@ export default function FinanceAnalysis() {
{/* Page Header */}
{chatBot.messages.length > 1 && <ChatHeaderTemplate messages={chatBot.messages} />}
{/* Conversation Area */}
<div className={`finance-analysis-chat-wrapper k-d-flex k-flex-column k-flex-1 k-p-6 k-gap-8 chat-wrapper ${chatBot.messages.length <= 1 ? 'show-gradient' : ''}`}>
<div className={`finance-analysis-chat-wrapper k-d-flex k-flex-column k-flex-1 k-p-6 k-gap-8 chat-wrapper ${chatBot.messages.length <= 1 ? 'show-gradient' : ''}`} style={{ minHeight: 0 }}>
<div
className={`k-d-flex k-flex-column k-align-items-center k-justify-content-flex-end k-h-full ${chatBot.messages.length > 1 ? "finance-analysis-chat-wrapper-conversation" : ""}`}
className={`k-d-flex k-flex-column k-align-items-center k-h-full ${chatBot.messages.length > 1 ? "finance-analysis-chat-wrapper-conversation" : ""}`}
style={chatBot.messages.length > 1 ? { minHeight: 0 } : { justifyContent: 'flex-end', paddingBottom: '64px' }}
>
{renderChat()}
</div>
Expand All @@ -299,9 +314,9 @@ export default function FinanceAnalysis() {
<div className="finance-preview-wrapper k-overflow-auto preview-wrapper k-d-flex k-flex-col">
{/* Page Header */}
<ChatHeaderTemplate messages={chatBot.messages} />
<div className="k-d-grid k-grid-cols-1 k-grid-cols-xl-2 preview k-flex-1">
<div className="k-d-grid k-grid-cols-1 k-grid-cols-xl-2 preview k-flex-1" style={{ minHeight: 0 }}>
{/* Left Panel - Chat (393px) */}
<div className="k-align-self-end k-d-none k-d-xl-flex chat-preview k-pos-relative">
<div className="k-d-none k-d-xl-flex chat-preview k-pos-relative" style={{ minHeight: 0}}>
{renderChat()}
</div>

Expand Down
4 changes: 2 additions & 2 deletions client/src/pages/KnowledgeAssistant.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,15 @@ const KnowledgeAssistant = () => {
)}
{hasConversationStarted && <ChatHeaderTemplate messages={chatBot.messages} />}
{/* Chat Component */}
<div className={`knowledge-assistant-conversation k-d-flex k-flex-column k-flex-1 k-align-items-center k-w-full conversation-container ${hasConversationStarted ? 'knowledge-assistant-conversation-started' : ''}`}>
<div className={`knowledge-assistant-conversation k-d-flex k-flex-column k-flex-1 k-align-items-center k-w-full conversation-container ${hasConversationStarted ? 'knowledge-assistant-conversation-started' : ''}`} style={hasConversationStarted ? { minHeight: 0 } : { justifyContent: 'flex-end', paddingBottom: '64px' }}>
<div className={`knowledge-assistant-chat-wrapper chat-content-wrapper k-w-full k-d-flex k-flex-column k-pos-relative ${!hasConversationStarted ? 'show-gradient' : ''} ${hasConversationStarted ? 'knowledge-assistant-chat-flex-full' : 'knowledge-assistant-chat-flex-none'}`}>
<Chat
messages={hasConversationStarted ? chatBot.messages.slice(1) : chatBot.messages}
authorId={chatBot.user.id}
onSendMessage={chatBot.addNewMessage}
placeholder="Try a suggestion or ask about KendoReact"
className="k-border-transparent"
height={hasConversationStarted ? "100%" : undefined}
height="100%"
messageTemplate={ChatMessage}
timestampTemplate={() => null }
showUsername={false}
Expand Down
29 changes: 22 additions & 7 deletions client/src/styles/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ html, body, #root {
}

.k-drawer-wrapper::after,
.preview .charts-preview:before {
.preview .chat-preview:before {
content: "";
display: block;
height: 100%;
Expand All @@ -198,11 +198,12 @@ html, body, #root {
border-radius: 815px;
pointer-events: none;
}
.k-drawer-wrapper::after {
.k-drawer-wrapper::after,
.preview .chat-preview:before {
right: 0;
}
.preview .charts-preview:before {
left: -60px;
.preview .chat-preview:before {
right: -80px;
}

.user-selection-wrapper:after,
Expand Down Expand Up @@ -346,12 +347,11 @@ html, body, #root {
display: none;
}

.k-chat .k-message-list {
.k-chat .k-message-list-content {
justify-content: flex-end;

min-height: 100%;
}


.finance-analysis-chat-default .k-message-list {
padding-block: 64px;
}
Expand Down Expand Up @@ -1102,13 +1102,25 @@ html, body, #root {

.finance-preview-wrapper {
height: calc(100vh - 54px);
overflow: hidden;
}

.finance-analysis-chat-expanded {
display: flex;
flex-direction: column;
height: 100%;
overflow-y: auto;
}




.finance-charts-preview {
border: 1px solid #FFF;
background: rgba(255, 255, 255, 0.50);
box-shadow: 0 4px 12px 0 rgba(13, 10, 44, 0.08);
backdrop-filter: blur(2px);
overflow-y: auto;
}

.finance-charts-container {
Expand Down Expand Up @@ -1210,6 +1222,7 @@ html, body, #root {

.knowledge-assistant-conversation {
box-sizing: border-box;
min-height: 0;
}

.knowledge-assistant-conversation-started {
Expand All @@ -1223,6 +1236,8 @@ html, body, #root {

.knowledge-assistant-chat-flex-full {
flex: 1;
min-height: 0;
height: 100%;
}

.knowledge-assistant-chat-flex-none {
Expand Down