fix: Fix mobile scroll bug on live chat #40
Conversation
WalkthroughRefactors LiveChat to delegate auto-scrolling to a new useChatScroll hook, replacing manual end-anchor scrolling. Integrates a Retry button for disconnected state. Adds a new hook file implementing smooth scroll-to-bottom when near-bottom on dependency changes. Minor UI tweaks, standardized string literals, and success handling for super chat flow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant WS as WebSocket
participant LiveChat
participant Hook as useChatScroll
participant DOM as Chat Container
WS->>LiveChat: onConnect
LiveChat->>LiveChat: fetchRecentMessages()
LiveChat->>DOM: Render messages
Note over LiveChat,Hook: chatWindowRef from useChatScroll attached to container
WS-->>LiveChat: onMessage(newMessage)
LiveChat->>LiveChat: append to messages
LiveChat->>Hook: dep change (messages)
Hook->>DOM: if near-bottom, smooth scroll to bottom
User->>LiveChat: Click Retry (disconnected)
LiveChat->>WS: manualReconnect()
User->>LiveChat: Send message / SuperChat
LiveChat->>WS: send("text"/"superchat", payload)
LiveChat->>LiveChat: on superchat success: toast + close modal
LiveChat->>Hook: dep change (messages)
Hook->>DOM: conditional smooth scroll
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
src/hooks/useChatScroll.js (3)
13-16: Avoid overscrolling: compute exact bottom positionUse
scrollHeight - clientHeightfor precision. Some browsers clampscrollTop> max, which can cause redundant work and jank.- node.scrollTo({ - top: node.scrollHeight, - behavior: "smooth", - }); + node.scrollTo({ + top: node.scrollHeight - node.clientHeight, + behavior: "smooth", + });
6-6: PreferuseEffecthere to reduce main-thread blocking on mobile
useLayoutEffectruns before paint and can contribute to scroll jank on low-end/mobile devices. Scrolling after paint is fine for chat.- React.useLayoutEffect(() => { + React.useEffect(() => {
9-15: Make near-bottom threshold and motion behavior adaptableMobile inertial scrolling benefits from a slightly larger threshold (e.g., 48–64px). Also respect reduced-motion preferences.
- const shouldScroll = - node.scrollTop + node.clientHeight + 20 >= node.scrollHeight; + const threshold = 48; // tweakable cushion for mobile inertial scroll + const shouldScroll = + node.scrollTop + node.clientHeight + threshold >= node.scrollHeight;Optionally:
- node.scrollTo({ - top: node.scrollHeight - node.clientHeight, - behavior: "smooth", - }); + const prefersReduced = window.matchMedia?.("(prefers-reduced-motion: reduce)")?.matches; + node.scrollTo({ + top: node.scrollHeight - node.clientHeight, + behavior: prefersReduced ? "auto" : "smooth", + });src/components/Chat/LiveChat.js (4)
24-24: Remove dead code: legacy anchor ref and helper
messagesEndRef,scrollToBottom(), and the commented anchor are no longer used after migrating touseChatScroll. Also drop the unuseduseRefimport.-import { useRef, useState } from "react"; +import { useState } from "react"; @@ - const messagesEndRef = useRef(null); @@ - const scrollToBottom = () => { - setTimeout(() => { - messagesEndRef.current?.scrollIntoView({ behavior: "smooth" }); - }, 100); - }; @@ - {/* <div ref={messagesEndRef} /> */}Also applies to: 117-121, 190-190, 1-1
31-37: Reduce noisy logs in productionPer-message
console.logcan hurt performance on mobile and interfere with smooth scrolling.- console.log("LiveChat received message:", message); - setMessages((prev) => { - console.log("Previous messages:", prev.length); - const updated = [...prev, message]; - console.log("Updated messages:", updated.length); - return updated; - }); + setMessages((prev) => [...prev, message]);If you want to keep diagnostics:
if (process.env.NODE_ENV !== "production") { console.log("LiveChat received message:", message); }
59-67: Avoid clearing messages on connect to prevent flickerImmediately calling
setMessages([])on every connect can cause empty-state flicker during reconnects. Consider clearing only on reconnect, or swap the list atomically after “recent_messages” arrive.
- Track a
isReconnectflag and only clear on reconnect.- Or buffer recent messages and then
setMessages(buffer)once.
100-101: Offer recovery action on send failureInstead of asking users to refresh, trigger
manualReconnect()and surface a retry CTA.- toast.error("Chat connection lost. Please refresh the page."); + toast.error("Chat connection lost. Attempting to reconnect..."); + manualReconnect();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
src/components/Chat/LiveChat.js(5 hunks)src/hooks/useChatScroll.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/Chat/LiveChat.js (4)
src/hooks/useWebSocket.js (5)
useAuth(13-13)useWebSocket(6-176)isConnected(7-7)sendMessage(154-163)connectionStatus(8-8)src/context/AuthContext.js (3)
useAuth(6-12)useAuth(6-12)user(15-15)src/hooks/useChatScroll.js (1)
useChatScroll(3-22)src/components/Chat/ChatMessage.js (1)
ChatMessage(5-54)
| <div | ||
| className="bg-white rounded-lg shadow-lg flex flex-col" | ||
| style={{ height: "600px" }} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix mobile height: avoid fixed 600px to prevent double-scroll and iOS toolbar issues
A fixed pixel height is brittle on mobile and can cause the very scroll bug you’re fixing. Use responsive dvh with a desktop fallback.
- <div
- className="bg-white rounded-lg shadow-lg flex flex-col"
- style={{ height: "600px" }}
- >
+ <div
+ className="bg-white rounded-lg shadow-lg flex flex-col h-[70dvh] md:h-[600px]"
+ >📝 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.
| <div | |
| className="bg-white rounded-lg shadow-lg flex flex-col" | |
| style={{ height: "600px" }} | |
| > | |
| <div | |
| className="bg-white rounded-lg shadow-lg flex flex-col h-[70dvh] md:h-[600px]" | |
| > |
🤖 Prompt for AI Agents
In src/components/Chat/LiveChat.js around lines 124-127, replace the fixed
inline height of "600px" with a responsive dvh-based value and a desktop
fallback to avoid double-scrolling on mobile; update the style to use CSS
min/max (for example height: "min(600px, 100dvh)" or height: "100dvh" with
maxHeight: "600px") so mobile uses 100dvh while desktop is capped, keeping the
element responsive and preventing iOS toolbar scroll issues.
| <span className="font-semibold">ℹ️ Chat Info:</span> All messages are | ||
| completely anonymous. Inappropriate words are censored. Mods will | ||
| remove personal targeted messages. |
There was a problem hiding this comment.
User-facing copy contradicts behavior
UI says “completely anonymous” but messages display username (derived from email). This is misleading.
- Either change copy to reflect display name usage, or
- Send
username: "Anonymous"whenisAnonymousis desired.
Example copy:
-<span className="font-semibold">ℹ️ Chat Info:</span> All messages are completely anonymous. Inappropriate words are censored. Mods will remove personal targeted messages.
+<span className="font-semibold">ℹ️ Chat Info:</span> Your display name is shown with messages. Inappropriate words are censored. Mods will remove personal targeted messages.📝 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.
| <span className="font-semibold">ℹ️ Chat Info:</span> All messages are | |
| completely anonymous. Inappropriate words are censored. Mods will | |
| remove personal targeted messages. | |
| <span className="font-semibold">ℹ️ Chat Info:</span> Your display name is shown with messages. Inappropriate words are censored. Mods will remove personal targeted messages. |
🤖 Prompt for AI Agents
In src/components/Chat/LiveChat.js around lines 167-169, the UI text claims
messages are "completely anonymous" but the app displays a username (derived
from email), which is misleading; either update the copy to accurately state
that a display name/username is shown (e.g., "Messages are pseudonymous; a
display name will be shown") or, if true anonymity is required, ensure when
isAnonymous is set you override the outgoing message payload to set username to
"Anonymous" (and strip any email-derived identifier) before sending; pick one
approach and implement the corresponding change consistently in the message
send/display logic and the copy here.
| <ChatMessage | ||
| key={msg.ID || msg.id || `msg-${index}-${Date.now()}`} | ||
| message={msg} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use stable keys to prevent re-mounts and scroll jumps
Date.now() in keys forces re-mounts every render and can break autoscroll on mobile. Prefer server IDs or timestamps; fall back to index only as last resort.
- <ChatMessage
- key={msg.ID || msg.id || `msg-${index}-${Date.now()}`}
- message={msg}
- />
+ <ChatMessage
+ key={msg.id || msg.ID || msg.created_at || msg.CreatedAt || index}
+ message={msg}
+ />📝 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.
| <ChatMessage | |
| key={msg.ID || msg.id || `msg-${index}-${Date.now()}`} | |
| message={msg} | |
| /> | |
| <ChatMessage | |
| key={msg.id || msg.ID || msg.created_at || msg.CreatedAt || index} | |
| message={msg} | |
| /> |
🤖 Prompt for AI Agents
In src/components/Chat/LiveChat.js around lines 184 to 187, the key for
ChatMessage uses Date.now() which forces re-mounts every render and breaks
autoscroll; replace that unstable key with a stable identifier by using the
server-provided ID or timestamp first (e.g., msg.id, msg.ID, msg.timestamp,
msg.createdAt) and only fall back to the array index as last resort (e.g.,
`msg-${index}`) so keys remain stable across renders.
Pull Request Template for Pollz Frontend
✏️ Summary of Changes
Describe the changes you have made, including any refactoring or feature additions. Attach any relevant screenshots or videos.
📦 Dependencies
List any dependencies or related PRs (e.g., "Depends on #123").
🐛 Related Issues
Link any issues that are resolved or affected by this PR. Use "Fixes #123" or "Closes #123" to automatically close issues when PR is merged.
📋 Checklist
📝 Additional Notes
Any additional context, breaking changes, or notes for reviewers.
Summary by CodeRabbit
New Features
Style