fix: websocket message scroll #6503
Conversation
pooja-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
WalkthroughRefactors WebSocket message rendering to use react-virtuoso for virtualization, lifts message open state to the parent, implements auto-scroll and wheel-based user-scroll detection, and removes the sort-order UI/prop from the WS response pane. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WSResponsePane
participant WSMessagesList
participant Virtuoso
participant WSMessageItem
User->>WSMessageItem: Click / toggle
WSMessageItem->>WSMessagesList: onToggle(messageId)
WSMessagesList->>WSMessagesList: update openMessages Set
WSMessagesList->>Virtuoso: request render/update
Virtuoso->>WSMessageItem: render with isOpen state
Note over WSMessagesList,Virtuoso: Wheel events and at-bottom state adjust followOutput
alt User at bottom and no open messages
WSMessagesList->>Virtuoso: enable auto-scroll (followOutput)
else User scrolled away or a message open
WSMessagesList->>Virtuoso: disable auto-scroll
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js (2)
62-95: Consider timestamp uniqueness for message identification.Using
message.timestampas the unique identifier (lines 94, 216) could cause issues if two messages arrive at the exact same millisecond. If message objects have a more reliable unique ID, prefer that instead.
210-217: Minor:shouldAutoScrollrelies on ref, andkeyis redundant.
Line 211:
shouldAutoScrollreads fromopenMessagesRef.currentwhich won't trigger re-evaluation whenopenMessagesstate changes. However, sinceisAtBottomstate change causes re-render, this works in practice. Consider usingopenMessages.sizedirectly for clarity.Line 216: The
keyprop insideitemContentis unnecessary — Virtuoso manages item keys internally viatotalCountand index.🔎 Suggested simplification
- // Auto-scroll enabled when no messages are open AND user is at bottom - const shouldAutoScroll = openMessagesRef.current.size === 0 && isAtBottom; + // Auto-scroll enabled when no messages are open AND user is at bottom + const shouldAutoScroll = openMessages.size === 0 && isAtBottom; const renderItem = useCallback((index) => { const msg = messages[index]; const isOpen = openMessages.has(msg.timestamp); - return <WSMessageItem key={msg.timestamp} message={msg} isOpen={isOpen} onToggle={handleMessageToggle} />; + return <WSMessageItem message={msg} isOpen={isOpen} onToggle={handleMessageToggle} />; }, [messages, openMessages, handleMessageToggle]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
packages/bruno-app/package.jsonpackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/ResponsePane/WsResponsePane/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js
🧠 Learnings (4)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/components/ResponsePane/WsResponsePane/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles
Applied to files:
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component
Applied to files:
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors
Applied to files:
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js
🧬 Code graph analysis (2)
packages/bruno-app/src/components/ResponsePane/WsResponsePane/index.js (1)
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js (1)
WSMessagesList(183-236)
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js (4)
packages/bruno-app/src/components/RequestPane/WsBody/SingleWSMessage/index.js (1)
message(41-41)packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js (1)
StyledWrapper(3-43)packages/bruno-app/src/components/ResponsePane/WsResponsePane/StyledWrapper.js (1)
StyledWrapper(3-57)packages/bruno-app/src/components/RequestPane/WsQueryUrl/StyledWrapper.js (1)
StyledWrapper(3-102)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
🔇 Additional comments (5)
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js (1)
3-5: LGTM!The
flex: 1andmin-height: 0combo is the correct approach for enabling virtualized scrolling within a flex container. Withoutmin-height: 0, the flex item wouldn't shrink below its content size, breaking Virtuoso's height calculation.packages/bruno-app/src/components/ResponsePane/WsResponsePane/index.js (1)
18-20: LGTM — sort order removal is intentional.The
WSResultcomponent now simply passes messages toWSMessagesListwithout sort order. This aligns with the virtualization refactor.packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js (3)
1-8: LGTM!Imports are well-organized and all are utilized in the component.
77-88: LGTM!The "new message" highlight effect is well-implemented with proper cleanup. The
notifiedref prevents re-triggering on re-renders.
222-235: LGTM — Virtuoso configuration is appropriate.The setup correctly handles auto-scroll behavior for a real-time message list. The
initialTopMostItemIndexensures users see the latest messages on load.Note:
virtuosoRef(line 225) is currently unused. Either remove it or keep it for future imperative scroll control.
| "react-player": "^2.16.0", | ||
| "react-redux": "^7.2.9", | ||
| "react-tooltip": "^5.5.2", | ||
| "react-virtuoso": "^4.17.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
react-virtuoso 4.17.0 React 19 compatibility
💡 Result:
Short answer: react-virtuoso v4.17.0 does not officially declare React 19 support in its peerDependencies, so you may see peer-dependency warnings when installing with React 19. It will often work at runtime, but it's not an officially listed compatible React version — use npm/yarn overrides or --legacy-peer-deps if needed, or test thoroughly.
Sources:
- react-virtuoso repo/homepage. [1]
- package.json (example release shows peerDependencies up to React 18). [2]
- Snyk/npm package metadata for v4.17.0. [3]
References:
[1] https://github.com/petyosi/react-virtuoso
[2] https://app.unpkg.com/[email protected]/files/package.json
[3] https://security.snyk.io/package/npm/react-virtuoso
Consider React 19 compatibility for [email protected].
[email protected] does not officially support React 19—peerDependencies are limited to React 18. While it may work at runtime, this creates peer-dependency warnings and potential maintenance risk. Either pin a React 18-compatible version, use an alternative virtualization library with React 19 support, or explicitly handle the peer-dependency conflict with npm/yarn overrides.
🤖 Prompt for AI Agents
In packages/bruno-app/package.json around line 83, react-virtuoso@^4.17.0
declares React 18 peerDependencies and will produce React 19 peer warnings; fix
by either (A) upgrade react-virtuoso to a release that explicitly supports React
19 (if one exists) and update the dependency entry accordingly, or (B) replace
react-virtuoso with an alternative virtualization library that declares React 19
support, or (C) add a package-manager override/peer-dependency resolution in
package.json (npm/yarn/pnpm) to accept React 19 for react-virtuoso and then
reinstall and run the app’s UI tests to verify no runtime issues.
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js
Outdated
Show resolved
Hide resolved
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js
Outdated
Show resolved
Hide resolved
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js (1)
62-95: Add displayName to memoized component for debugging.When wrapping components with
memo, adding adisplayNamehelps with React DevTools debugging.Suggested change
const WSMessageItem = memo(({ message, isOpen, onToggle }) => { // ... component body }); + +WSMessageItem.displayName = 'WSMessageItem';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
packages/bruno-app/package.jsonpackages/bruno-app/src/components/ResponsePane/WsResponsePane/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-app/package.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/ResponsePane/WsResponsePane/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js
🧠 Learnings (5)
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors
Applied to files:
packages/bruno-app/src/components/ResponsePane/WsResponsePane/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component
Applied to files:
packages/bruno-app/src/components/ResponsePane/WsResponsePane/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles
Applied to files:
packages/bruno-app/src/components/ResponsePane/WsResponsePane/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/components/ResponsePane/WsResponsePane/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js
📚 Learning: 2026-01-09T18:25:14.640Z
Learnt from: kanakkholwal
Repo: usebruno/bruno PR: 6767
File: packages/bruno-app/src/components/ResponseExample/index.js:221-226
Timestamp: 2026-01-09T18:25:14.640Z
Learning: In the Bruno Electron renderer code (packages/bruno-app), assume window.ipcRenderer is always available and skip existence checks. Do not guard for ipcRenderer in this Electron context; use window.ipcRenderer directly (e.g., window.ipcRenderer.send(...), window.ipcRenderer.on(...)). If there are non-Electron contexts (such as test environments or non-Electron builds), add guards or mocks to avoid runtime errors there, but for the intended Electron renderer files, this pattern should be applied broadly within packages/bruno-app.
Applied to files:
packages/bruno-app/src/components/ResponsePane/WsResponsePane/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.jspackages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js
🧬 Code graph analysis (2)
packages/bruno-app/src/components/ResponsePane/WsResponsePane/index.js (3)
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js (1)
WSMessagesList(177-258)packages/bruno-app/src/components/ResponsePane/GrpcResponsePane/index.js (1)
response(39-39)packages/bruno-app/src/components/ResponsePane/index.js (1)
response(39-39)
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js (1)
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js (1)
StyledWrapper(3-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (10)
packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/StyledWrapper.js (1)
4-6: LGTM!Correct flex layout for virtualization. The
min-height: 0is essential for the flex child to shrink properly, allowing Virtuoso to manage scrolling internally.packages/bruno-app/src/components/ResponsePane/WsResponsePane/StyledWrapper.js (1)
57-66: LGTM!Clean action-icon styling using theme props. Follows the established pattern and coding guidelines for styled components.
packages/bruno-app/src/components/ResponsePane/WsResponsePane/index.js (1)
18-20: LGTM!Clean simplification. Passing only
messagesaligns with the refactoredWSMessagesListsignature.packages/bruno-app/src/components/ResponsePane/WsResponsePane/WSMessagesList/index.js (7)
1-8: LGTM!Clean imports. Adding
memo,useCallback, andVirtuososupports the virtualization refactor well.
82-88: LGTM!Timer cleanup is properly handled. Good use of cleanup function in useEffect.
184-194: LGTM!Clean toggle implementation using Set. Creating a new Set instance on each update is the correct pattern for React state immutability.
196-211: LGTM!Good use of passive event listener for wheel events. Proper cleanup on unmount.
220-229: LGTM!Smart auto-scroll logic: disables when user scrolls away or has messages expanded. Re-enables smoothly when at bottom.
246-255: LGTM!Solid Virtuoso configuration.
initialTopMostItemIndexset to last item provides the expected chat-like scroll behavior. ThefollowOutputandatBottomStateChangecallbacks work together nicely for auto-scroll management.
236-238: No action needed –seqis always present.The
seqfield is automatically assigned to every WebSocket message by an incrementing counter managed per connection. It's never undefined or missing, making the fallback tomsg.timestampunnecessary in practice. The key generation is safe.
2 participants
Description
JIRA
Contribution Checklist:
Screen.Recording.2026-01-12.at.1.00.45.PM.mov
Summary by CodeRabbit
New Features
Improvements
Changes
✏️ Tip: You can customize this high-level summary in your review settings.