feat: infinite scroll on blabsy and pictique#921
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds cursor-based pagination and infinite-scroll to chats: client scroll detection, loading older messages with scroll-position restoration, SSE initial payload metadata (type/hasMore/total), new server endpoints and service methods for paginated retrieval, and client-side dedupe/grouping for streamed and historical messages. Changes
Sequence DiagramsequenceDiagram
participant User
participant ClientUI as Client UI
participant ChatContext as Chat Context / Client Service
participant API
participant DB as Database
User->>ClientUI: Scroll to top
ClientUI->>ClientUI: handleScroll (near top)
ClientUI->>ChatContext: loadOlderMessages(chatId, beforeId)
ChatContext->>ChatContext: set loadingOlderMessages = true
ChatContext->>API: GET /api/chats/:id/messages/before?before=beforeId&limit=30
API->>DB: Query messages before cursor (createdAt < cursor OR equal with id <)
DB-->>API: Return messages (limit+1) + total info
API-->>ChatContext: { messages, hasMore, total }
ChatContext->>ClientUI: prepend olderMessages + savedScrollInfo
ClientUI->>ClientUI: restore scroll position (no auto-scroll)
ClientUI->>User: Render older messages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
platforms/blabsy/client/src/components/chat/chat-window.tsx (1)
396-405: Minor UX consideration: "No more messages" shown at top of empty conversations.When
hasMoreMessagesisfalsebut the chat has few or no messages, the "No more messages" text still appears. This is technically correct but might be confusing UX for new conversations. Consider conditionally hiding it when there are no messages at all:-{!hasMoreMessages && ( +{!hasMoreMessages && messages?.length > 0 && ( <p className='text-center text-xs text-gray-400 dark:text-gray-500 my-2'> No more messages </p> )}This is optional polish.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/blabsy/client/src/components/chat/chat-window.tsx` around lines 396 - 405, The "No more messages" line is shown even for empty conversations; update the conditional that renders it to also check the messages array length (e.g., only show when hasMoreMessages is false AND messages.length > 0) so new/empty chats don't display that text; modify the JSX around hasMoreMessages (in chat-window.tsx where loadingOlderMessages and the Loading component are rendered) to include this additional messages check.platforms/pictique/api/src/services/ChatService.ts (1)
270-302: Potential edge case with identicalcreatedAttimestamps.The cursor-based pagination uses
LessThan(cursorMessage.createdAt)which could skip messages if multiple messages share the samecreatedAttimestamp. This is unlikely but possible in high-throughput scenarios.Consider adding a secondary sort criterion (e.g., message ID) to ensure deterministic ordering:
const messages = await this.messageRepository.find({ where: { chat: { id: chatId }, createdAt: LessThan(cursorMessage.createdAt), }, relations: ["sender", "readStatuses", "readStatuses.user"], - order: { createdAt: "DESC" }, + order: { createdAt: "DESC", id: "DESC" }, take: limit + 1, });This is a minor edge case given typical message rates, but worth noting for robustness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/pictique/api/src/services/ChatService.ts` around lines 270 - 302, In getChatMessagesBefore, the current WHERE uses only LessThan(cursorMessage.createdAt) which can skip messages with identical createdAt; update the query used in messageRepository.find (and the cursor lookup via cursorMessage) to implement a deterministic tie-breaker: include a compound condition such as (createdAt < cursorCreatedAt) OR (createdAt = cursorCreatedAt AND id < beforeId), add a secondary order by id DESC to the existing order by createdAt DESC, and keep the same take/hasMore logic so pagination remains correct when timestamps are identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/blabsy/client/src/lib/context/chat-context.tsx`:
- Line 42: The ChatContext type for createNewChat is inconsistent with its
implementation: update the type signature in chat-context to match the
implementation by changing createNewChat: (participants: string[], name?:
string) => Promise<string> to include the optional description parameter
(createNewChat: (participants: string[], name?: string, description?: string) =>
Promise<string>), or alternatively remove the description?: string parameter
from the implementation (the function around the createNewChat definition) so
both declaration and implementation match; ensure you update any related usages
to the new signature if needed.
In `@platforms/pictique/api/src/controllers/MessageController.ts`:
- Around line 219-246: getMessagesBefore currently skips verifying the
requesting user is a chat participant; add the same participant check used in
getChat so only members can fetch messages. Before calling
this.chatService.getChatMessagesBefore, retrieve the chat (or call a service
method like this.chatService.isParticipant / this.chatService.getChat) and
ensure req.user.id is listed as a participant; if not, return
res.status(403).json({ error: "Forbidden" }) and only proceed to call
this.chatService.getChatMessagesBefore when the user is authorized.
In `@platforms/pictique/client/src/routes/`(protected)/messages/[id]/+page.svelte:
- Around line 68-69: The variable isOwn is currently set with inverted logic
(const isOwn = sender.id !== userId) causing messages from other users to be
treated as "own"; change the comparison to check equality (sender.id === userId)
so isOwn is true only when the message sender matches userId; update the
assignment near m.sender and review any usage of isOwn in the component
rendering to ensure alignment with the corrected boolean.
- Line 157: The post to mark messages read is currently invoked on every SSE
event via apiClient.post(`/api/chats/${id}/messages/read`); change this so the
read-receipt is sent once after processing the initial batch and thereafter only
once per batch (e.g., debounce/throttle or send after state update that merges
multiple events) inside the SSE event handler, and add error handling (try/catch
or promise .catch) around apiClient.post to log or handle failures; locate the
SSE handler where apiClient.post is called and replace the unconditional
per-event call with a single-call-after-processing strategy that references the
same id param and preserves existing state-updating logic.
- Around line 116-162: watchEventStream opens an EventSource but never closes it
on component unmount; modify watchEventStream to expose or return a cleanup that
closes the EventSource and removes handlers (eventSource.close() and nullifying
onmessage/onopen/onerror/onclose) and then call that cleanup from Svelte's
onDestroy; alternatively store eventSource in a component-scoped variable and in
onDestroy(() => { eventSource?.close(); eventSource = null; }) ensure you close
the SSE and detach handlers used in watchEventStream (references:
watchEventStream, eventSource, onmessage, onopen, onclose, onDestroy).
---
Nitpick comments:
In `@platforms/blabsy/client/src/components/chat/chat-window.tsx`:
- Around line 396-405: The "No more messages" line is shown even for empty
conversations; update the conditional that renders it to also check the messages
array length (e.g., only show when hasMoreMessages is false AND messages.length
> 0) so new/empty chats don't display that text; modify the JSX around
hasMoreMessages (in chat-window.tsx where loadingOlderMessages and the Loading
component are rendered) to include this additional messages check.
In `@platforms/pictique/api/src/services/ChatService.ts`:
- Around line 270-302: In getChatMessagesBefore, the current WHERE uses only
LessThan(cursorMessage.createdAt) which can skip messages with identical
createdAt; update the query used in messageRepository.find (and the cursor
lookup via cursorMessage) to implement a deterministic tie-breaker: include a
compound condition such as (createdAt < cursorCreatedAt) OR (createdAt =
cursorCreatedAt AND id < beforeId), add a secondary order by id DESC to the
existing order by createdAt DESC, and keep the same take/hasMore logic so
pagination remains correct when timestamps are identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f71f54d-e3ed-46ff-8aaf-c40de0cd776d
📒 Files selected for processing (6)
platforms/blabsy/client/src/components/chat/chat-window.tsxplatforms/blabsy/client/src/lib/context/chat-context.tsxplatforms/pictique/api/src/controllers/MessageController.tsplatforms/pictique/api/src/index.tsplatforms/pictique/api/src/services/ChatService.tsplatforms/pictique/client/src/routes/(protected)/messages/[id]/+page.svelte
platforms/pictique/client/src/routes/(protected)/messages/[id]/+page.svelte
Show resolved
Hide resolved
platforms/pictique/client/src/routes/(protected)/messages/[id]/+page.svelte
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
platforms/blabsy/client/src/components/chat/chat-window.tsx (1)
401-410: Consider reordering "No more messages" indicator for better UX.The "No more messages" indicator appears above the loading spinner when both conditions could theoretically be true during state transitions. Consider placing the loading indicator first to ensure it's always visible at the very top during loads.
💡 Suggested reorder
-{!hasMoreMessages && messages.length > 0 && ( - <p className='text-center text-xs text-gray-400 dark:text-gray-500 my-2'> - No more messages - </p> -)} {loadingOlderMessages && ( <div className='flex justify-center my-2'> <Loading className='h-6 w-6' /> </div> )} +{!hasMoreMessages && messages.length > 0 && ( + <p className='text-center text-xs text-gray-400 dark:text-gray-500 my-2'> + No more messages + </p> +)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/blabsy/client/src/components/chat/chat-window.tsx` around lines 401 - 410, The "No more messages" notice can be obscured during state transitions because it is rendered before the loading spinner; in ChatWindow component reorder the conditional rendering so that loadingOlderMessages (and the Loading component) is rendered before the hasMoreMessages && messages check, i.e. move the loading block above the "No more messages" block so the Loading spinner is always shown at the top during loads; adjust render order around the loadingOlderMessages, hasMoreMessages, and messages checks accordingly.platforms/pictique/api/src/services/ChatService.ts (1)
6-6: Unused import:LessThanis imported but not used.The
LessThanoperator is imported but the cursor-based pagination usesBracketswith raw comparisons instead.🧹 Proposed fix to remove unused import
-import { In, LessThan, Brackets } from "typeorm"; +import { In, Brackets } from "typeorm";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/pictique/api/src/services/ChatService.ts` at line 6, Remove the unused import "LessThan" from the import list in ChatService.ts; update the import line that currently reads "import { In, LessThan, Brackets } from 'typeorm';" to only import the used symbols (e.g., "In" and "Brackets") so there are no unused imports remaining.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@platforms/blabsy/client/src/components/chat/chat-window.tsx`:
- Around line 401-410: The "No more messages" notice can be obscured during
state transitions because it is rendered before the loading spinner; in
ChatWindow component reorder the conditional rendering so that
loadingOlderMessages (and the Loading component) is rendered before the
hasMoreMessages && messages check, i.e. move the loading block above the "No
more messages" block so the Loading spinner is always shown at the top during
loads; adjust render order around the loadingOlderMessages, hasMoreMessages, and
messages checks accordingly.
In `@platforms/pictique/api/src/services/ChatService.ts`:
- Line 6: Remove the unused import "LessThan" from the import list in
ChatService.ts; update the import line that currently reads "import { In,
LessThan, Brackets } from 'typeorm';" to only import the used symbols (e.g.,
"In" and "Brackets") so there are no unused imports remaining.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 366349e4-0894-4d00-84dd-674470459294
📒 Files selected for processing (5)
platforms/blabsy/client/src/components/chat/chat-window.tsxplatforms/blabsy/client/src/lib/context/chat-context.tsxplatforms/pictique/api/src/controllers/MessageController.tsplatforms/pictique/api/src/services/ChatService.tsplatforms/pictique/client/src/routes/(protected)/messages/[id]/+page.svelte
ananyayaya129
approved these changes
Mar 15, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
platforms/pictique/client/src/routes/(protected)/messages/[id]/+page.svelte (1)
79-80:⚠️ Potential issue | 🔴 CriticalInverted
isOwnlogic will cause messages to display on the wrong side.The condition
sender.id !== userIdsetsisOwntotruewhen the message is not from the current user. This is semantically inverted—isOwnshould betruewhen the sender is the current user.🐛 Proposed fix
const sender = m.sender as Record<string, string>; -const isOwn = sender.id !== userId; +const isOwn = sender.id === userId;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/pictique/client/src/routes/`(protected)/messages/[id]/+page.svelte around lines 79 - 80, The isOwn flag is inverted: currently it uses sender.id !== userId which marks other users as "own"; change the condition in the message rendering code (the sender variable and isOwn assignment in +page.svelte) to compare equality (sender.id === userId) so isOwn is true when the message sender matches userId; update any places that rely on isOwn (render logic or CSS class toggles) to use the corrected boolean.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/pictique/client/src/routes/`(protected)/messages/[id]/+page.svelte:
- Around line 176-181: The handleSend function clears messageValue regardless of
POST success and gives no user feedback on failure; wrap the apiClient.post call
in a try/catch, introduce a sending state (e.g., isSending) to prevent duplicate
sends, only clear messageValue after a successful response, and surface errors
to the user (e.g., set an errorMessage state or call the app's
toast/notification helper) and re-enable input on failure; reference the
handleSend function, apiClient.post(`/api/chats/${id}/messages`), and
messageValue when making these changes.
- Around line 127-174: The SSE connection in watchEventStream currently lacks an
onerror handler so failures are silent; add eventSource.onerror to log the
error, set a local connection state (e.g., connectionError or sseError) so the
UI can stop any loading spinner, and gracefully close/cleanup eventSourceRef on
error; implement simple retry logic that closes the current EventSource, waits
with exponential backoff (or fixed delay) and re-invokes watchEventStream to
reconnect (ensure you clear previous handlers to avoid leaks), and call
markMessagesRead/update historyLoaded or other UI flags as appropriate so users
see feedback when the stream drops.
---
Duplicate comments:
In `@platforms/pictique/client/src/routes/`(protected)/messages/[id]/+page.svelte:
- Around line 79-80: The isOwn flag is inverted: currently it uses sender.id !==
userId which marks other users as "own"; change the condition in the message
rendering code (the sender variable and isOwn assignment in +page.svelte) to
compare equality (sender.id === userId) so isOwn is true when the message sender
matches userId; update any places that rely on isOwn (render logic or CSS class
toggles) to use the corrected boolean.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3a923bb0-f93e-4aba-bba8-bd4bc97e64a1
📒 Files selected for processing (1)
platforms/pictique/client/src/routes/(protected)/messages/[id]/+page.svelte
Comment on lines
+127
to
+174
| async function watchEventStream() { | ||
| const sseUrl = new URL( | ||
| `/api/chats/${id}/events?token=${getAuthToken()}`, | ||
| PUBLIC_PICTIQUE_BASE_URL | ||
| ).toString(); | ||
| const eventSource = new EventSource(sseUrl); | ||
| eventSourceRef = eventSource; | ||
|
|
||
| eventSource.onopen = () => { | ||
| console.log('Successfully connected.'); | ||
| }; | ||
|
|
||
| eventSource.onmessage = (e) => { | ||
| try { | ||
| const data = JSON.parse(e.data); | ||
|
|
||
| // Handle initial payload with metadata vs real-time new messages | ||
| if (data.type === 'initial') { | ||
| // Initial batch from SSE connection | ||
| const transformed = transformMessages(data.messages); | ||
| messages = computeGroupFlags(removeDuplicateMessages(transformed)); | ||
| hasMore = data.hasMore; | ||
| if (data.messages.length > 0) { | ||
| oldestMessageId = data.messages[0].id; | ||
| } | ||
| historyLoaded = true; | ||
| shouldScrollToBottom = true; | ||
| } else { | ||
| // Real-time new message(s) — could be array or wrapped | ||
| const rawMessages = Array.isArray(data) ? data : [data]; | ||
| const transformed = transformMessages(rawMessages); | ||
|
|
||
| const existingIds = new Set(messages.map((msg) => msg.id)); | ||
| const uniqueNew = transformed.filter((msg) => !existingIds.has(msg.id)); | ||
|
|
||
| if (uniqueNew.length > 0) { | ||
| const merged = removeDuplicateMessages([...messages, ...uniqueNew]); | ||
| messages = computeGroupFlags(merged); | ||
| shouldScrollToBottom = true; | ||
| } | ||
| } | ||
|
|
||
| markMessagesRead(); | ||
| } catch (error) { | ||
| console.error('Error parsing SSE message:', error); | ||
| } | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
Missing onerror handler leaves SSE failures silent.
If the SSE connection fails or drops, there's no error handling or retry logic. Users will see an infinite loading spinner with no feedback.
🛠️ Proposed fix: Add error handler with user feedback
eventSource.onopen = () => {
console.log('Successfully connected.');
};
+eventSource.onerror = (err) => {
+ console.error('SSE connection error:', err);
+ // Optionally set an error state to display to the user
+ // or implement reconnection logic
+};
+
eventSource.onmessage = (e) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function watchEventStream() { | |
| const sseUrl = new URL( | |
| `/api/chats/${id}/events?token=${getAuthToken()}`, | |
| PUBLIC_PICTIQUE_BASE_URL | |
| ).toString(); | |
| const eventSource = new EventSource(sseUrl); | |
| eventSourceRef = eventSource; | |
| eventSource.onopen = () => { | |
| console.log('Successfully connected.'); | |
| }; | |
| eventSource.onmessage = (e) => { | |
| try { | |
| const data = JSON.parse(e.data); | |
| // Handle initial payload with metadata vs real-time new messages | |
| if (data.type === 'initial') { | |
| // Initial batch from SSE connection | |
| const transformed = transformMessages(data.messages); | |
| messages = computeGroupFlags(removeDuplicateMessages(transformed)); | |
| hasMore = data.hasMore; | |
| if (data.messages.length > 0) { | |
| oldestMessageId = data.messages[0].id; | |
| } | |
| historyLoaded = true; | |
| shouldScrollToBottom = true; | |
| } else { | |
| // Real-time new message(s) — could be array or wrapped | |
| const rawMessages = Array.isArray(data) ? data : [data]; | |
| const transformed = transformMessages(rawMessages); | |
| const existingIds = new Set(messages.map((msg) => msg.id)); | |
| const uniqueNew = transformed.filter((msg) => !existingIds.has(msg.id)); | |
| if (uniqueNew.length > 0) { | |
| const merged = removeDuplicateMessages([...messages, ...uniqueNew]); | |
| messages = computeGroupFlags(merged); | |
| shouldScrollToBottom = true; | |
| } | |
| } | |
| markMessagesRead(); | |
| } catch (error) { | |
| console.error('Error parsing SSE message:', error); | |
| } | |
| }; | |
| } | |
| async function watchEventStream() { | |
| const sseUrl = new URL( | |
| `/api/chats/${id}/events?token=${getAuthToken()}`, | |
| PUBLIC_PICTIQUE_BASE_URL | |
| ).toString(); | |
| const eventSource = new EventSource(sseUrl); | |
| eventSourceRef = eventSource; | |
| eventSource.onopen = () => { | |
| console.log('Successfully connected.'); | |
| }; | |
| eventSource.onerror = (err) => { | |
| console.error('SSE connection error:', err); | |
| // Optionally set an error state to display to the user | |
| // or implement reconnection logic | |
| }; | |
| eventSource.onmessage = (e) => { | |
| try { | |
| const data = JSON.parse(e.data); | |
| // Handle initial payload with metadata vs real-time new messages | |
| if (data.type === 'initial') { | |
| // Initial batch from SSE connection | |
| const transformed = transformMessages(data.messages); | |
| messages = computeGroupFlags(removeDuplicateMessages(transformed)); | |
| hasMore = data.hasMore; | |
| if (data.messages.length > 0) { | |
| oldestMessageId = data.messages[0].id; | |
| } | |
| historyLoaded = true; | |
| shouldScrollToBottom = true; | |
| } else { | |
| // Real-time new message(s) — could be array or wrapped | |
| const rawMessages = Array.isArray(data) ? data : [data]; | |
| const transformed = transformMessages(rawMessages); | |
| const existingIds = new Set(messages.map((msg) => msg.id)); | |
| const uniqueNew = transformed.filter((msg) => !existingIds.has(msg.id)); | |
| if (uniqueNew.length > 0) { | |
| const merged = removeDuplicateMessages([...messages, ...uniqueNew]); | |
| messages = computeGroupFlags(merged); | |
| shouldScrollToBottom = true; | |
| } | |
| } | |
| markMessagesRead(); | |
| } catch (error) { | |
| console.error('Error parsing SSE message:', error); | |
| } | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/pictique/client/src/routes/`(protected)/messages/[id]/+page.svelte
around lines 127 - 174, The SSE connection in watchEventStream currently lacks
an onerror handler so failures are silent; add eventSource.onerror to log the
error, set a local connection state (e.g., connectionError or sseError) so the
UI can stop any loading spinner, and gracefully close/cleanup eventSourceRef on
error; implement simple retry logic that closes the current EventSource, waits
with exponential backoff (or fixed delay) and re-invokes watchEventStream to
reconnect (ensure you clear previous handlers to avoid leaks), and call
markMessagesRead/update historyLoaded or other UI flags as appropriate so users
see feedback when the stream drops.
Comment on lines
+176
to
+181
| async function handleSend() { | ||
| await apiClient.post(`/api/chats/${id}/messages`, { | ||
| text: messageValue | ||
| }); | ||
| messageValue = ''; | ||
| } |
Contributor
There was a problem hiding this comment.
Consider adding error handling for message send failures.
If the POST fails, messageValue is still cleared but the user receives no feedback that their message wasn't sent.
🛠️ Proposed improvement
async function handleSend() {
- await apiClient.post(`/api/chats/${id}/messages`, {
- text: messageValue
- });
- messageValue = '';
+ try {
+ await apiClient.post(`/api/chats/${id}/messages`, {
+ text: messageValue
+ });
+ messageValue = '';
+ } catch (err) {
+ console.error('Failed to send message:', err);
+ // Optionally show user feedback
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function handleSend() { | |
| await apiClient.post(`/api/chats/${id}/messages`, { | |
| text: messageValue | |
| }); | |
| messageValue = ''; | |
| } | |
| async function handleSend() { | |
| try { | |
| await apiClient.post(`/api/chats/${id}/messages`, { | |
| text: messageValue | |
| }); | |
| messageValue = ''; | |
| } catch (err) { | |
| console.error('Failed to send message:', err); | |
| // Optionally show user feedback | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/pictique/client/src/routes/`(protected)/messages/[id]/+page.svelte
around lines 176 - 181, The handleSend function clears messageValue regardless
of POST success and gives no user feedback on failure; wrap the apiClient.post
call in a try/catch, introduce a sending state (e.g., isSending) to prevent
duplicate sends, only clear messageValue after a successful response, and
surface errors to the user (e.g., set an errorMessage state or call the app's
toast/notification helper) and re-enable input on failure; reference the
handleSend function, apiClient.post(`/api/chats/${id}/messages`), and
messageValue when making these changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of change
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
UX Improvements
Performance