feat(request-pane): restore body tab scroll position on tab switch#7250
feat(request-pane): restore body tab scroll position on tab switch#7250Nizam-Chaudhary wants to merge 1 commit intousebruno:mainfrom
Conversation
When editing large request bodies (JSON/XML/text/sparql), switching to another tab (params, headers, auth, etc.) and back would reset the CodeMirror editor scroll position to the top. Fix by persisting the scroll position to Redux on editor unmount (via CodeEditor's onScroll prop) and restoring it on mount (via initialScroll), mirroring the existing scroll restoration pattern in QueryResultPreview.
Nizam-Chaudhary
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
WalkthroughAdds per-tab scroll position persistence to the request body editor. Introduces a new Redux reducer Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
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
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 2
🧹 Nitpick comments (2)
packages/bruno-app/src/providers/ReduxStore/slices/tabs.js (1)
80-97: InitializerequestBodyScrollPositionalongsideresponsePaneScrollPositioninaddTab.
responsePaneScrollPositionis explicitly initialized tonull(line 86) in the push path, but the newrequestBodyScrollPositionfield is not, creating inconsistency. Selectors readingtab.requestBodyScrollPositionwill getundefinedinstead ofnullon freshly opened tabs.♻️ Proposed fix
state.tabs.push({ uid, collectionUid, requestPaneWidth: null, requestPaneTab: requestPaneTab || defaultRequestPaneTab, responsePaneTab: 'response', responsePaneScrollPosition: null, + requestBodyScrollPosition: null, responseFormat: null, responseViewTab: null, scriptPaneTab: null, type: type || 'request', ...(uid ? { folderUid: uid } : {}), preview: preview !== undefined ? preview : !nonReplaceableTabTypes.includes(type), ...(exampleUid ? { exampleUid } : {}), ...(itemUid ? { itemUid } : {}) });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/providers/ReduxStore/slices/tabs.js` around lines 80 - 97, The new field requestBodyScrollPosition is missing an explicit initialization in the tab creation path, causing freshly opened tabs to have undefined instead of null; update the object pushed into state.tabs inside the addTab reducer (the state.tabs.push(...) block) to include requestBodyScrollPosition: null alongside responsePaneScrollPosition: null so selectors consistently receive null on new tabs.packages/bruno-app/src/components/RequestPane/RequestBody/index.js (1)
38-45: Throttle theonScrolldispatch to avoid flooding Redux on every scroll tick.
onScrollfires on every scroll event, triggering a Redux state write and re-render cycle at scroll frequency. This is particularly costly for large body editors. A simple trailing throttle (e.g. ~100ms) keeps the persisted position accurate without the hot-path cost.♻️ Proposed fix — debounce with lodash
+import { useMemo } from 'react'; +import debounce from 'lodash/debounce'; - const onScroll = (editor) => { - dispatch( - updateRequestBodyScrollPosition({ - uid: focusedTab.uid, - scrollY: editor.getScrollInfo().top - }) - ); - }; + const onScroll = useMemo( + () => + debounce((editor) => { + if (!focusedTab) return; + dispatch( + updateRequestBodyScrollPosition({ + uid: focusedTab.uid, + scrollY: editor.getScrollInfo().top + }) + ); + }, 100), + [focusedTab, dispatch] + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/RequestBody/index.js` around lines 38 - 45, The onScroll handler calls dispatch(updateRequestBodyScrollPosition(...)) on every scroll tick; wrap that dispatch in a trailing throttle (~100ms) to reduce Redux writes. Create a stable throttled function (e.g. throttledUpdate = useMemo(() => throttle((uid, scrollY) => dispatch(updateRequestBodyScrollPosition({ uid, scrollY })), 100, { trailing: true }), [dispatch]) and then have onScroll call throttledUpdate(focusedTab.uid, editor.doc.scrollTop); ensure you import throttle from lodash (or implement equivalent) and cancel the throttled function on unmount (throttledUpdate.cancel() in a useEffect cleanup) so the throttled updates and focusedTab.uid are handled correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-app/src/components/RequestPane/RequestBody/index.js`:
- Line 41: The object construction uses focusedTab.uid without a null guard
which can throw if focusedTab is undefined; change that usage to safely access
the property (e.g., use focusedTab?.uid or provide a default like
null/undefined) where the object is created (look for the object literal
containing "uid: focusedTab.uid") so it mirrors the safe access pattern used
elsewhere (focusedTab?.requestBodyScrollPosition).
- Around line 38-45: The onScroll handler is reading the editor's internal
scrollTop via editor.doc.scrollTop; change it to use the public API
editor.getScrollInfo().top (e.g., pass editor.getScrollInfo().top as the scrollY
value when dispatching updateRequestBodyScrollPosition for focusedTab.uid) so it
matches other usages (linkAware.js, brunoVarInfo.js) and avoids relying on
undocumented internals.
---
Nitpick comments:
In `@packages/bruno-app/src/components/RequestPane/RequestBody/index.js`:
- Around line 38-45: The onScroll handler calls
dispatch(updateRequestBodyScrollPosition(...)) on every scroll tick; wrap that
dispatch in a trailing throttle (~100ms) to reduce Redux writes. Create a stable
throttled function (e.g. throttledUpdate = useMemo(() => throttle((uid, scrollY)
=> dispatch(updateRequestBodyScrollPosition({ uid, scrollY })), 100, { trailing:
true }), [dispatch]) and then have onScroll call throttledUpdate(focusedTab.uid,
editor.doc.scrollTop); ensure you import throttle from lodash (or implement
equivalent) and cancel the throttled function on unmount
(throttledUpdate.cancel() in a useEffect cleanup) so the throttled updates and
focusedTab.uid are handled correctly.
In `@packages/bruno-app/src/providers/ReduxStore/slices/tabs.js`:
- Around line 80-97: The new field requestBodyScrollPosition is missing an
explicit initialization in the tab creation path, causing freshly opened tabs to
have undefined instead of null; update the object pushed into state.tabs inside
the addTab reducer (the state.tabs.push(...) block) to include
requestBodyScrollPosition: null alongside responsePaneScrollPosition: null so
selectors consistently receive null on new tabs.
| const onScroll = (editor) => { | ||
| dispatch( | ||
| updateRequestBodyScrollPosition({ | ||
| uid: focusedTab.uid, | ||
| scrollY: editor.doc.scrollTop | ||
| }) | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate the file and examine the onScroll handler
find . -name "RequestBody" -type d | head -5Repository: usebruno/bruno
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Search for the RequestBody file
fd "index.js" | grep -i "requestbody"Repository: usebruno/bruno
Length of output: 207
🏁 Script executed:
#!/bin/bash
# Look at the file mentioned in review
cat -n "packages/bruno-app/src/components/RequestPane/RequestBody/index.js" | head -60Repository: usebruno/bruno
Length of output: 2646
🏁 Script executed:
#!/bin/bash
# Search for CodeEditor component and how onScroll is wired
rg "onScroll" --type js -C 3 | head -80Repository: usebruno/bruno
Length of output: 3773
🏁 Script executed:
#!/bin/bash
# Search for QueryResultPreview pattern
fd -e js -e jsx | xargs grep -l "QueryResultPreview" 2>/dev/null | head -5Repository: usebruno/bruno
Length of output: 212
🏁 Script executed:
#!/bin/bash
# Get QueryResultPreview onScroll implementation
cat -n "packages/bruno-app/src/components/ResponsePane/QueryResult/QueryResultPreview/index.js" | grep -A 10 "const onScroll"Repository: usebruno/bruno
Length of output: 356
🏁 Script executed:
#!/bin/bash
# Check CodeEditor implementation for scroll event binding
cat -n "packages/bruno-app/src/components/CodeEditor/index.js" | head -200Repository: usebruno/bruno
Length of output: 8321
🏁 Script executed:
#!/bin/bash
# Search for getScrollInfo usage in codebase
rg "getScrollInfo" --type jsRepository: usebruno/bruno
Length of output: 557
🏁 Script executed:
#!/bin/bash
# Continue reading CodeEditor to find scroll event binding
cat -n "packages/bruno-app/src/components/CodeEditor/index.js" | sed -n '200,350p'Repository: usebruno/bruno
Length of output: 6300
🏁 Script executed:
#!/bin/bash
# Search for where scroll event is registered on editor
rg "\.on\('scroll" --type js -A 3Repository: usebruno/bruno
Length of output: 743
🏁 Script executed:
#!/bin/bash
# Look for CodeMirror scroll event documentation or patterns
rg "scroll" "packages/bruno-app/src/components/CodeEditor/index.js" -C 2Repository: usebruno/bruno
Length of output: 617
🏁 Script executed:
#!/bin/bash
# Verify CodeMirror 5 API: check if getScrollInfo is consistently used
rg "getScrollInfo" --type js -B 2 -A 2 | head -40Repository: usebruno/bruno
Length of output: 2304
Use the public getScrollInfo().top API instead of the internal editor.doc.scrollTop property.
editor.getScrollInfo() is the documented CodeMirror 5 API that returns {left, top, width, height, clientWidth, clientHeight}. The editor.doc.scrollTop property is internal and undocumented. The codebase already uses getScrollInfo() elsewhere (linkAware.js, brunoVarInfo.js), so adopt the same pattern here for consistency and future maintainability.
Proposed fix
const onScroll = (editor) => {
dispatch(
updateRequestBodyScrollPosition({
uid: focusedTab.uid,
- scrollY: editor.doc.scrollTop
+ scrollY: editor.getScrollInfo().top
})
);
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-app/src/components/RequestPane/RequestBody/index.js` around
lines 38 - 45, The onScroll handler is reading the editor's internal scrollTop
via editor.doc.scrollTop; change it to use the public API
editor.getScrollInfo().top (e.g., pass editor.getScrollInfo().top as the scrollY
value when dispatching updateRequestBodyScrollPosition for focusedTab.uid) so it
matches other usages (linkAware.js, brunoVarInfo.js) and avoids relying on
undocumented internals.
| const onScroll = (editor) => { | ||
| dispatch( | ||
| updateRequestBodyScrollPosition({ | ||
| uid: focusedTab.uid, |
There was a problem hiding this comment.
focusedTab is not null-guarded here, unlike line 75.
Line 75 correctly uses focusedTab?.requestBodyScrollPosition, but line 41 accesses focusedTab.uid unconditionally. If focusedTab resolves to undefined (e.g., during a race where activeTabUid is null while the component is still mounted), this will throw.
🛡️ Proposed fix
const onScroll = (editor) => {
+ if (!focusedTab) return;
dispatch(
updateRequestBodyScrollPosition({
uid: focusedTab.uid,
scrollY: editor.getScrollInfo().top
})
);
};📝 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.
| uid: focusedTab.uid, | |
| const onScroll = (editor) => { | |
| if (!focusedTab) return; | |
| dispatch( | |
| updateRequestBodyScrollPosition({ | |
| uid: focusedTab.uid, | |
| scrollY: editor.getScrollInfo().top | |
| }) | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-app/src/components/RequestPane/RequestBody/index.js` at line
41, The object construction uses focusedTab.uid without a null guard which can
throw if focusedTab is undefined; change that usage to safely access the
property (e.g., use focusedTab?.uid or provide a default like null/undefined)
where the object is created (look for the object literal containing "uid:
focusedTab.uid") so it mirrors the safe access pattern used elsewhere
(focusedTab?.requestBodyScrollPosition).
2 participants
Fixes: #7249
Description
When editing large request bodies (JSON/XML/text/sparql), switching to another tab (params, headers, auth, etc.) or different request and back would reset the editor scroll position to the top.
Fix by persisting the scroll position to Redux on editor unmount (via onScroll prop) and restoring it on mount (via initialScroll), mirroring the existing scroll restoration pattern in QueryResultPreview.
Contribution Checklist:
Summary by CodeRabbit